Bugfix: package requirements with git commits#35057
Conversation
…erate associated facts to indicate that those versions are valid
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@spackbot fix style |
|
Let me see if I can fix that for you! |
This comment was marked as resolved.
This comment was marked as resolved.
|
I've addressed the direct review comments but haven't fully resolved my original questions. I've iterated on those at: |
…quivalence) - passes in this branch but not on develop
|
I also added fd58f2b to demonstrate that without these changes you also cannot do |
…record these individually and (b) define an equivalence between them. Problem: the spec *after* concretization does not record the equivalence
…transferring properties from command line specs)
…hash=version; as a result, there are no special considerations to document
…the wrong place but ill relocate it later
| # In this case, neither supplies a version equivalence with "=" | ||
| # and the commit strings are equal | ||
| return True | ||
| elif other.is_ref: |
There was a problem hiding this comment.
@alalazo @tgamblin I think this implements the desired semantics and I added test_git_user_supplied_reference_satisfaction to confirm that (I didn't see a test covering these in test/versions.py). I don't think this will conflict with #35681 (that doesn't change satisfies, and furthermore satisfies can be more strict than intersects).
…, update test that was expecting failure
… (not just intersect)
|
Pipeline here is only failing because a power runner could not be found. Everything else is passing, and |
| self.idx = idx | ||
| self.origin = origin | ||
|
|
||
| def __eq__(self, other): |
There was a problem hiding this comment.
Do we really need all this complexity? Is this still necessary when @y gets parsed as VersionRange(y, y) and @=y as Version(y). imho this looks very messy
| return False | ||
| is_git = lambda v: isinstance(v, spack.version.GitVersion) | ||
| if is_git(self.version) != is_git(other.version): | ||
| # GitVersion(hash=x) and Version(y) compare equal if x == y, but |
There was a problem hiding this comment.
Why do they compare equal in the first place if they're different things.
|
I think this broke |
* Specs that define 'new' versions in the require: section need to generate associated facts to indicate that those versions are valid. * add test to verify success with unknown versions.
…35057)" (spack#36341)" This reverts commit 4dc9d9f.
- [x] Specs that define 'new' versions in the require: section need to generate associated facts to indicate that those versions are valid.
- [x] add test to verify success with unknown versions.
- [x] remove unneeded check that was leading to extra complexity and test
failures (at this point, all `hash=version` does not require listing out that version
in `packages.yaml`)
- [x] unique index for origin (dont reuse 0)
Co-authored-by: Peter Josef Scheibel <scheibel1@llnl.gov>
Specs which define 'new' versions in the require: section need to generate associated facts to indicate that those versions are valid.
(Update: since addressed, although pending further discussion)
There's a couple conceptual issues at the moment (noted as TODOs in the code).