Skip to content

unify: when_possible and unify: true -- Bugfix for error in 37438#37681

Merged
tgamblin merged 3 commits intodevelopfrom
bugfix/error-in-37438
May 16, 2023
Merged

unify: when_possible and unify: true -- Bugfix for error in 37438#37681
tgamblin merged 3 commits intodevelopfrom
bugfix/error-in-37438

Conversation

@becker33
Copy link
Copy Markdown
Member

@becker33 becker33 commented May 15, 2023

Two bugs came in from #37438

  1. unify: when_possible was broken, because of an incorrect assertion. abstract/concrete spec pairs were compared against the results that were in the process of being computed, rather than against the previous results.
  2. unify: true had an ordering bug that could mix the association between abstract and concrete specs

1 is resolved by creating a lookup from old concrete specs to old abstract specs, and we use that to associate the "new" concrete specs that happen to be the old ones with their abstract specs (since those are stripped out for concretization
2 is resolved by combining the new and old abstract as lists instead of combining them as sets. This is important because set() | set() does not make any ordering promises, even though set ordering is otherwise guaranteed in python@3.7:

becker33 added 2 commits May 15, 2023 09:50
37438 had a bug in which the results of concretization were checked against
the new environment, rather than against the previous environment, to ensure
existing specs were not changed.
@spackbot-app spackbot-app bot added core PR affects Spack core functionality environments labels May 15, 2023
@becker33 becker33 added this to the v0.20.0 milestone May 15, 2023
@tgamblin tgamblin enabled auto-merge (squash) May 15, 2023 21:12
@tgamblin tgamblin merged commit 3765a5f into develop May 16, 2023
@tgamblin tgamblin deleted the bugfix/error-in-37438 branch May 16, 2023 05:08
alalazo pushed a commit that referenced this pull request May 16, 2023
…7681)

Two bugs came in from #37438

1. `unify: when_possible` was broken, because of an incorrect assertion. abstract/concrete
   spec pairs were compared against the results that were in the process of being computed,
   rather than against the previous results.
2. `unify: true` had an ordering bug that could mix the association between abstract and
   concrete specs

- [x] 1 is resolved by creating a lookup from old concrete specs to old abstract specs,
      and we use that to associate the "new" concrete specs that happen to be the old
      ones with their abstract specs (since those are stripped out for concretization
- [x] 2 is resolved by combining the new and old abstract as lists instead of combining
      them as sets. This is important because `set() | set()` does not make any ordering
      promises, even though set ordering is otherwise guaranteed in `python@3.7:`
@alalazo alalazo mentioned this pull request May 16, 2023
14 tasks
alalazo pushed a commit that referenced this pull request May 16, 2023
…7681)

Two bugs came in from #37438

1. `unify: when_possible` was broken, because of an incorrect assertion. abstract/concrete
   spec pairs were compared against the results that were in the process of being computed,
   rather than against the previous results.
2. `unify: true` had an ordering bug that could mix the association between abstract and
   concrete specs

- [x] 1 is resolved by creating a lookup from old concrete specs to old abstract specs,
      and we use that to associate the "new" concrete specs that happen to be the old
      ones with their abstract specs (since those are stripped out for concretization
- [x] 2 is resolved by combining the new and old abstract as lists instead of combining
      them as sets. This is important because `set() | set()` does not make any ordering
      promises, even though set ordering is otherwise guaranteed in `python@3.7:`
haampie pushed a commit that referenced this pull request May 16, 2023
…7681)

Two bugs came in from #37438

1. `unify: when_possible` was broken, because of an incorrect assertion. abstract/concrete
   spec pairs were compared against the results that were in the process of being computed,
   rather than against the previous results.
2. `unify: true` had an ordering bug that could mix the association between abstract and
   concrete specs

- [x] 1 is resolved by creating a lookup from old concrete specs to old abstract specs,
      and we use that to associate the "new" concrete specs that happen to be the old
      ones with their abstract specs (since those are stripped out for concretization
- [x] 2 is resolved by combining the new and old abstract as lists instead of combining
      them as sets. This is important because `set() | set()` does not make any ordering
      promises, even though set ordering is otherwise guaranteed in `python@3.7:`
alalazo pushed a commit that referenced this pull request May 18, 2023
…7681)

Two bugs came in from #37438

1. `unify: when_possible` was broken, because of an incorrect assertion. abstract/concrete
   spec pairs were compared against the results that were in the process of being computed,
   rather than against the previous results.
2. `unify: true` had an ordering bug that could mix the association between abstract and
   concrete specs

- [x] 1 is resolved by creating a lookup from old concrete specs to old abstract specs,
      and we use that to associate the "new" concrete specs that happen to be the old
      ones with their abstract specs (since those are stripped out for concretization
- [x] 2 is resolved by combining the new and old abstract as lists instead of combining
      them as sets. This is important because `set() | set()` does not make any ordering
      promises, even though set ordering is otherwise guaranteed in `python@3.7:`
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
…ack#37681)

Two bugs came in from spack#37438

1. `unify: when_possible` was broken, because of an incorrect assertion. abstract/concrete
   spec pairs were compared against the results that were in the process of being computed,
   rather than against the previous results.
2. `unify: true` had an ordering bug that could mix the association between abstract and
   concrete specs

- [x] 1 is resolved by creating a lookup from old concrete specs to old abstract specs,
      and we use that to associate the "new" concrete specs that happen to be the old
      ones with their abstract specs (since those are stripped out for concretization
- [x] 2 is resolved by combining the new and old abstract as lists instead of combining
      them as sets. This is important because `set() | set()` does not make any ordering
      promises, even though set ordering is otherwise guaranteed in `python@3.7:`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core PR affects Spack core functionality environments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants