Mitigation for GitVersion bug when no =reference is given#36159
Merged
haampie merged 3 commits intospack:developfrom Mar 17, 2023
Merged
Mitigation for GitVersion bug when no =reference is given#36159haampie merged 3 commits intospack:developfrom
=reference is given#36159haampie merged 3 commits intospack:developfrom
Conversation
They are semantically equivalent for concrete versions, but the GitVersion.intersects implementation is buggy
fixes spack#36134 This commit works around the issue in spack#36134, by using GitVersion.satisfies instead of GitVersion.intersects There are still underlying issues when trying to infer the "reference version" when no explicit one is given, but: 1. They are not reproducible with our synthetic repo 2. They occur only when the `git.<xxx>` form of Git version is used Here we just work around the user facing issue and ensure the tests are correct with our synthetic repository.
spackbot-app
bot
added
core
alalazo
commented
Mar 16, 2023
| # Also, "possible versions" contain only concrete versions, so satisfies is appropriate | ||
| allowed_versions = [ | ||
| v for v in sorted(self.possible_versions[pkg_name]) if v.intersects(versions) | ||
| v for v in sorted(self.possible_versions[pkg_name]) if v.satisfies(versions) |
Member
Author
There was a problem hiding this comment.
To be clear, in this point where v.concrete is True using satisfies or intersects is conceptually the same. GitVersion.intersects has a bug that requires some time to be fixed, changing the version being called seems not ot incur in #36134
Member
Author
There was a problem hiding this comment.
@scheibelp fyi, I think you touched the same in #35057
haampie
approved these changes
Mar 17, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #36134
This commit works around the issue in #36134, by using
GitVersion.satisfiesinstead ofGitVersion.intersects.There are still underlying issues when trying to infer the "reference version" when no explicit one is given, but:
git.<xxx>form of Git version is usedHere we just work around the user facing issue and ensure the tests are correct with our synthetic repository.