Avoid re-concretizing if the spec is already concrete#20192
Avoid re-concretizing if the spec is already concrete#20192scottwittenburg wants to merge 1 commit intospack:developfrom
Conversation
|
@haampie For me, this fixed the issue we were digging into this evening. Hopefully it doesn't cause a bunch of test failures 😉 |
|
Looks good, but needs a test |
|
Which issue is this PR solving? Is that with the new concretizer or the original one? |
|
#20191, I used to do |
| if not spec.concrete: | ||
| spec.concretize() |
There was a problem hiding this comment.
I don't know the full context, but this seems to be covering up a bug. My reasoning is that concretization should reach a fixed point i.e. for a concrete spec spec.concretize() should be a no-op.
There was a problem hiding this comment.
This method could have been called with a spec made from a .spec.yaml fetched from an arbitrary binary mirror, and in that case, isn't it true that re-concretization could change it completely?
However, it looks like #20196 is a better way to prevent this behavior from happening in any possible code path.
|
Seems like the hash mismatch is gone when adding but I still think it shouldn't reconcretize a concrete spec. |
This fixes a problem we could see where asking spack to check a buildcache using a hash resulted in re-concretization and subsequent checking of a buildcache with a different hash.
E.g.:
Notice how spack actually looked for
/2b2hcaqrather than/yrnbhxf, which was what we asked it to look for.