Conversation
| (building project X doesn't need project Y's build dependencies). | ||
| * **"link"**: the project is linked to by the project. The package will be | ||
| added to the current package's ``rpath``. | ||
| * **"run"**: the project is used by the project at runtime. The package will |
There was a problem hiding this comment.
"the project is used by the project" is possibly the most confusing statement I've ever seen.
| this because uninstalling the dependency would break the package. | ||
| this because uninstalling the dependency would break the package. Another | ||
| consequence of this is that ``build``-only dependencies do not affect the | ||
| hash of the package. The same is true for ``test`` dependencies. |
There was a problem hiding this comment.
Thanks for clarifying this point. I didn't realize this behavior was available.
There was a problem hiding this comment.
For build dependencies this might change in the future, so not sure if we should state it in the docs.
There was a problem hiding this comment.
My understanding was that in #3768 we decided the best idea would be to add an include deptype for things that affect the hash but can be uninstalled after installation.
Anyway, based on how long #3768 has been sitting without any changes, I would rather document the current behavior than document some future behavior.
There was a problem hiding this comment.
My understanding was that in #3768 we decided the best idea would be to add an include deptype for things that affect the hash but can be uninstalled after installation.
@adamjstewart The dependency model discussed in #3768 is outdated in the sense that:
- A
builddependency might affect how a package binary is built, so it should affect the hash since we use that for reproducibility (think of acmakethat changes some compilation defaults across versions) - There are other aspects to be taken into consideration, for instance if a dependency is unified over a DAG or not (one can build 10 different nodes of a DAG with 10 different
cmake, but likely can't use different versions of the same C++ template library in a single DAG)
I concur that what is documented here is the current implementation and I'm fine with these changes overall. Can we maybe add a note or warning box to state that we might change how build deps affect the hash in the future if extending some aspect of the dependency model require it?
There was a problem hiding this comment.
- I don't want to think about this 😦 . I agree it could be problematic, but having the build dep affect the hash will cause a lot of reinstalls. I guess we'll probably wait until the new concretizer for that so there are fewer rebuilds.
- I would argue that such a C++ template library should be a type=link or type=include dependency, but I don't know as much about C++ so maybe I'm wrong.
I'll wait for one more reviewer to suggest adding a note about how these might change in the future. I'm trying to be as detailed as possible without confusing the reader. I assume if we change the behavior of any of these deptypes, we will update any packages that need to be updated accordingly. So it's not as important (to me at least) to warn users of future behavior in the Packaging Guide.
There was a problem hiding this comment.
I guess we'll probably wait until the new concretizer for that so there are fewer rebuilds.
That's in fact the first use case I have in mind. Fine with me to wait for other reviewers and see what they think.
I'll wait for one more reviewer to suggest adding a note about how these might change in the future. I'm trying to be as detailed as possible without confusing the reader.
Just to be explicit on my intent and avoid misunderstanding: I'm not proposing to document now how things will change, but just suggesting to add a brief note to users to say that the behavior of build deps might still change in the future.
There was a problem hiding this comment.
I think documenting the current behavior and changing the documentation as needed is the proper course of action.
Spack has had a "test" dep type for years, but this hasn't been documented until now. Thanks for bringing this to my attention @KineticTheory.
As for the other dep types, users are constantly confused on their meaning, so let's make this more clear while we're at it.
Closes #3768