Override package directives by danielsjensen1 · Pull Request #48947 · spack/spack

danielsjensen1 · 2025-02-08T01:32:37Z

This merge request adds support for removing directives when desired. This is especially useful when inheriting a package because often the child doesn't have the same versions, dependencies, conflicts, etc. In the past I've used hacks to get around this as shown below. I tried to follow the style in the surrounding code but am open to suggestions if anyone thinks it should look or function differently.

Example of old hack:
The Xyce package has the following dependency.

depends_on("[email protected]:develop", when="@7.8.0:")

I can change this dependency in a child class using the following two lines

depends_on("trilinos", when="@7.8.0:") # Hack to override the base class dependency
depends_on("trilinos@14", when="@7.8.0") # This is the correct dependency

In this case the Trilinos dependency is incorrect and I should (and will eventually) fix the parent package. The proposed code in this merge request would change the lines above to

drop_depends_on("trilinos", when="@7.8.0:")
depends_on("trilinos@14", when="@7.8.0")

If this is acceptable and on the right track then I would be open to adding support for deleting other directives; I've just found that the versions, depends_on, and conflicts directives are the most important for me.

Still need to parameterize the unit tests.

Generalized the _create_test_repo test fixture to be parameterized.

haampie · 2025-02-08T11:37:14Z

This is similar to #35045 and currently discussed again. The idea was not to do verbatim mappings of when=... but also allow e.g. drop("depends_on", ..., when="@4:") if the parent class has depends_on("...") unconditionally (or when="@:" really).

danielsjensen1 · 2025-02-10T18:05:30Z

This is similar to #35045 and currently discussed again. The idea was not to do verbatim mappings of when=... but also allow e.g. drop("depends_on", ..., when="@4:") if the parent class has depends_on("...") unconditionally (or when="@:" really).

I think I understand this use case but just want to clarify before I modify my code. In my current implementation this is achievable via two statements but I could modify it to just be a single function if that is the desired functionality.

class Parent(Package):
    depends_on("mpi")

class Child(Parent):
    # Proposal with one function call
    remove_depends_on("mpi", when="@4.0:")
    # Is currently equivalent to two function calls
    remove_depends_on("mpi")
    depends_on("mpi", when="@:3.9")

haampie · 2025-02-12T13:31:15Z

It's not very ergonomic when there are multiple depends_on statements with different constraints and different when conditions on the same package. You'd have to remove all, and then add all-but-one back to make a change. Or you'd have to remove a specific one, and update every time there's a small change in the package you inherit from.

danielsjensen1 · 2025-02-18T18:11:20Z

I'll take a crack at implementing this to avoid the multiple depends_on statements. Would it make more sense to change the function call to something like depends_on("mpi", when="@:3.9", override=True)? That way we wouldn't have to introduce any new directives and it would be a bit easier to figure out what is going on. (Personally, I find it a bit mind bending to have to do all of the negation in my head with the remove_depends_on("mpi", when="@4.0:") call.)

Started modifying remove_conflict to use the complement methods.

Added support for partially removing conflicts based on ranges.

danielsjensen1 · 2025-02-24T19:42:21Z

I added support for partially removing directives as suggested. The following now works

class X(Package):
    conflicts("mpi", when="@3:")
    remove_conflict("mpi", when="@5:")
    # This is equivalent to conflicts("mpi", when="@3:4")

I need to clean things up a bit and apply the same support to the other directives. @haampie Does this satisfy the feature you were suggesting earlier? It would probably be good to have someone familiar with the versioning code check how I implemented the complement method as well. Also, is there a strong preference for using the drop("conflict", ...) terminology versus remove_conflict?

psakievich · 2025-02-27T15:35:43Z

@tgamblin will you update on how this is related to #35045?

* develop: (752 commits) mesa: add v23.3.3 and use py-packaging while python>=3.12 (spack#49121) gcc: add v15.1.0 (spack#50212) draco: add v7.20.0 (spack#49996) sgpp: update dependencies and variants (spack#49384) input_analysis.py: fix conditional requirements (spack#50194) boost: add 1.88.0 (spack#50158) mapl: add v2.55.1 (spack#50201) mepo: add v2.3.2 (spack#50202) py-repligit: add v0.1.1 (spack#50204) [package updates] Bump version of cp2k and sirius (spack#50141) petsc4py: update ldshared.patch for v3.20.1, and skip for v3.23.1+ (spack#50170) namd: add v3.0.1 (spack#50192) geomodel: depend on c (spack#49781) CompilerAdaptor: add support for opt_flags/debug_flags (spack#50126) Add ls alias to spack {compiler, external} (spack#50189) covfie: depend on c (spack#50190) lua-sol2: add v3.5.0 (spack#49970) crtm-fix: fix directory logic (spack#50172) py-build: add v1.2.2 (spack#50148) py-pillow: fix build (spack#50177) ...

Started turning remove_directive into a class.

Started parametrizing tests for removal directives.

tgamblin · 2025-06-06T09:42:34Z

lib/spack/spack/directives.py

+def drop_version(ver: Union[str, int]):
+    """Try to remove a specific version from a package."""


Can this be made into drop_versions(), and can it accept a VersionRange or a VersionList?

Ideally this would use satisfies to figure out what to drop so that I can drop ranges of versions, e.g., given:

class Gcc(Package): version("14.2.0") version("9.5.0") version("8.5.0") version("8.3.0") version("5.4.0") version("5.2.0") version("4.9.3") version("4.9.2") version("4.9.1")

You could write:

drop_versions("=4.9.2,5.4.0:8,14")

and get:

class MyrGcc(Package): version("9.5.0") version("5.2.0") version("4.9.3") version("4.9.1")

Can that work?

Yes, I'm modifying it right now to do that. I'm currently adding support for DropPatch and DropVersion but have had to generalize the code a bit because of their different function signatures. Not a big deal but I probably won't finish until next week. The good news is that the generalization should make it apply to all of the directives.

I have drop_patch working now and will do drop_version(s) next week. I've had some time to think about your request for drop_versions. I'm still open to doing the drop_versions modification but have realized that some of the directives like drop_patch accept a lot of arguments so they probably won't be compatible with the version range idea. Even drop_version will have a similar issue when it supports the when argument. I've just been doing [drop_version(ver) for ver in ["1.3", "1.1"]] to achieve something similar. Anyways, I'll update everyone when this is ready for another review.

…n1/spack into override-package-directives

becker33

We do not have a general Spec.complement() method, and I think that without one it is not possible to implement the when argument in this PR correctly. I see 3 things we can do about this:

Remove the when option for now, and make it future work for when we have a Spec.complement capability
Implement a general Spec.complement capability
Leave this as-is and document (very loudly) that the when clauses for drop* directives only accept versions.
Replace the when option with an except option (eliminating the need for a complement operation).

I think (2) is infeasible -- the spec syntax does not lend itself well to a complement capability. It's not clear what the complement of +foo is for a spec that only defines that variant on some subset of versions (presumably it's the union of ~foo and the complement of the versions on which foo is defined?), and it's even less clear what the complement of cflags=-O3 is (I think it's any setting such that -O3 is not in the cflags?) and neither of those are things we can currently express in a Spec object.

I'm open to option (3), but I understand why some maintainers won't be (because it is a deviation from what when accepts in every other directive, it will be hard for users to keep the mental model.

@danielsjensen1 do you have strong feelings between options (1), (3), and (4)? Is the when option necessary for the use case that's motivating you to work on this?

becker33 · 2025-09-18T12:00:12Z

lib/spack/spack/directives.py

+        filtered = {}
+        for when, directives in directive_dict.items():
+            for directive_entry in self.iterate_directives(directives):
+                if self.get_directive(directive_entry) == removal_directive:


I think this would be clearer if we combine the get_directive and make_directive methods into a single method entry_matches_removal. Then this line becomes

Suggested change

if self.get_directive(directive_entry) == removal_directive:

if self.entry_matches_removal(directive_entry):

and the default implementation for that method could be return self.get_directive(directive_entry) == self.make_directive(pkg), but it gives us more room to rename those methods to something easier to follow as well.

becker33 · 2025-09-18T12:02:29Z

lib/spack/spack/directives.py

+    def add_to_filtered(self, filtered, when, directive_entry):
+        filtered.setdefault(when, []).append(directive_entry)


It would be great if we could refactor the add_to_filtered methods for each directive type to share code with how the entries are added to the dictionary in the directive itself. It seems fragile to have two different ways of registering a conflict / dependency / etc. in a dictionary implemented separately in two different code paths.

becker33 · 2025-09-18T12:06:11Z

lib/spack/spack/directives.py

+    def iterate_directives(self, directives):
+        return directives.items()
+
+    def get_directive(self, directive_entry):


Reading this, I'm curious about the following case:

class Foo(Package): depends_on("mpi@1:") class Bar(Foo): drop_depends_on("mpi")

It appears that in this case bar would still depend on mpi@1, is that correct?

We should consider whether it's better to have spec satisfaction as the matching function for these cases. It seems more natural that way, although it does make it impossible to wipe out a general constraint without also wiping the more specific constraints.

Yes, you are correct that this will still depend on mpi@1 with the current code. I'll work on changing it to do spec comparison.

becker33 · 2025-09-18T12:09:08Z

lib/spack/spack/directives.py

  * ``version``
  * ``requires``
  * ``redistribute``
+  * ``drop_all_conflicts``


Once we are happy with the rest of this PR, we should look at extending it to extends, provides, variant, and maybe even resource. We should also definitely add a drop_all_patches directive, but that's trivial to add.

danielsjensen1 · 2025-09-20T03:59:00Z

Thanks @becker33 for looking at this and your feedback. My main motivation for this work was to override some conflicts, depends_on, and version directives for packages like Xyce and Trilinos. I hadn't even considered the fact that we would need a complement method for specs. In fact, I didn't even think about getting the complement for versions until @haampie suggested it. I just have several use cases where I will always need to override parts of a community package but want to avoid copying and pasting. I finally have some time to work on this again and will respond to your suggestions over the next few days.

# Conflicts: # lib/spack/spack/directives.py # lib/spack/spack/package.py # lib/spack/spack/test/directives.py

becker33 · 2025-12-08T19:02:53Z

@danielsjensen1 we talked about this in the Spack team meeting today.

We were unanimous that option (3) above is the best option, and we should just document that it only works for versions and throw an error if the when clause on a drop directive has non-version components.
We should do the spec comparison change we discussed above
We should apply this to all the directives that we can

Where do you stand on coming back around to this? Can we make it a priority for Spack 1.2?

danielsjensen1 · 2025-12-10T19:21:25Z

Thanks @becker33 for helping make the decision. I'll start cleaning things up and make this a priority for Spack 1.2.

psakievich · 2026-02-04T23:30:55Z

@danielsjensen1 any updates on this? I'm hungry to use it...

tgamblin · 2026-03-17T20:33:55Z

@danielsjensen1 are you going to have time to work on this? I would like to get it in for 1.2, as I think this is a really valuable feature, but I realize you have other stuff going on! Can we help?

danielsjensen1 · 2026-03-18T21:42:30Z

I'm working on this again now. The _get_execution_plan function now sorts the directives (directives_involved: {'drop_all_versions', 'drop_version', 'version'} so drop_version is running before version. I'm figuring out the best way to fix or work around that.

danielsjensen1 added 3 commits February 6, 2025 18:25

Added remove_all_versions and remove_versions directives.

dce92ca

Still need to parameterize the unit tests.

Added conflicts and depends_on removal functions with unit tests.

4795d73

Generalized the _create_test_repo test fixture to be parameterized.

Delete dictionary key when list is empty.

c37122d

spackbot-app bot added conflicts core PR affects Spack core functionality dependencies directives new-version tests General test capability(ies) utilities labels Feb 8, 2025

Merge branch 'develop' into override-package-directives

f17d47c

danielsjensen1 mentioned this pull request Feb 8, 2025

Add a new drop directive for extending packages #35045

Closed

4 tasks

Added complement methods for VersionList and ClosedOpenRange.

a9d4346

Started modifying remove_conflict to use the complement methods.

spackbot-app bot added the versions label Feb 22, 2025

danielsjensen1 added 2 commits February 24, 2025 12:32

Removed debug statements.

79abddb

Added support for partially removing conflicts based on ranges.

Merge branch 'develop' into override-package-directives

e51f02b

danielsjensen1 added 3 commits February 24, 2025 14:15

Fix mypy error.

5a2db98

Merge branch 'develop' into override-package-directives

71ab573

Problem with black autoformatting removing a comma.

f63028e

danielsjensen1 and others added 5 commits April 26, 2025 09:48

Started abstracting out the dependency removal.

59026c3

Lots of refactoring and generalization.

f9675eb

Started turning remove_directive into a class.

More generalization.

6c86e3c

Added support to remove requirements.

193f917

Started parametrizing tests for removal directives.

psakievich requested a review from haampie June 6, 2025 03:38

tgamblin requested changes Jun 6, 2025

View reviewed changes

psakievich assigned psakievich and danielsjensen1 Jun 6, 2025

tgamblin modified the milestones: v1.0.0, v1.1.0 Jun 6, 2025

danielsjensen1 added 3 commits June 13, 2025 18:32

Added support for dropping patches.

2361382

Merge branch 'override-package-directives' of github.com:danielsjense…

e21eb98

…n1/spack into override-package-directives

Merge branch 'develop' into override-package-directives

73695c8

spackbot-app bot added the patch label Jun 14, 2025

Fixed broken call to patch.

3bea9b9

becker33 requested changes Sep 18, 2025

View reviewed changes

Merge branch 'develop' into override-package-directives

b76a484

# Conflicts: # lib/spack/spack/directives.py # lib/spack/spack/package.py # lib/spack/spack/test/directives.py

psakievich added this to Sandia-Spack-Work Oct 8, 2025

psakievich moved this to In Progress in Sandia-Spack-Work Oct 8, 2025

danielsjensen1 added 2 commits October 8, 2025 18:49

Switch drop directive tests to use mock_packages.

29edeff

Style fixes.

b7c0420

tgamblin modified the milestones: v1.1.0, v1.2.0 Nov 3, 2025

danielsjensen1 added 2 commits November 12, 2025 13:53

Make drop_requires singular.

9bcd0c8

Merge branch 'develop' into override-package-directives

76db39f

danielsjensen1 added 2 commits March 18, 2026 16:43

Merge branch 'develop' into override-package-directives

90847f7

Hack to fix sorting of directives.

6276cc2

github-actions bot added the unit-tests label Mar 18, 2026

		def drop_version(ver: Union[str, int]):
		"""Try to remove a specific version from a package."""

	if self.get_directive(directive_entry) == removal_directive:
	if self.entry_matches_removal(directive_entry):

		def add_to_filtered(self, filtered, when, directive_entry):
		filtered.setdefault(when, []).append(directive_entry)

Conversation

danielsjensen1 commented Feb 8, 2025 • edited by tgamblin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haampie commented Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielsjensen1 commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haampie commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielsjensen1 commented Feb 18, 2025

Uh oh!

danielsjensen1 commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psakievich commented Feb 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

becker33 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielsjensen1 commented Sep 20, 2025

Uh oh!

becker33 commented Dec 8, 2025

Uh oh!

danielsjensen1 commented Dec 10, 2025

Uh oh!

psakievich commented Feb 4, 2026

Uh oh!

tgamblin commented Mar 17, 2026

Uh oh!

danielsjensen1 commented Mar 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danielsjensen1 commented Feb 8, 2025 •

edited by tgamblin

Loading

haampie commented Feb 8, 2025 •

edited

Loading

danielsjensen1 commented Feb 10, 2025 •

edited

Loading

haampie commented Feb 12, 2025 •

edited

Loading

danielsjensen1 commented Feb 24, 2025 •

edited

Loading