Skip to content

asp.py: do not mutate spec.name#51821

Merged
alalazo merged 2 commits intodevelopfrom
hs/fix/asp-no-spec-mutation
Jan 12, 2026
Merged

asp.py: do not mutate spec.name#51821
alalazo merged 2 commits intodevelopfrom
hs/fix/asp-no-spec-mutation

Conversation

@haampie
Copy link
Copy Markdown
Member

@haampie haampie commented Jan 8, 2026

Mutating Spec.name during concretization invalidates hash values used in dictionary keys and causes non-local side effects for shared immutable specs (ref #51819). This leads to subtle bugs like these:

spec = Spec("+foo")
specs = {spec}
spec.name = "example"
assert spec in list(specs) # OK
assert spec in specs # AssertionError

and

empty_spec = Spec()  # shared, immutable singleton, used as "unconditional spec"
spec = empty_spec  # reference instead of copy, intentional, non-local
spec.name = "foo"  # as part of _condition_clauses
Spec("example").intersects(empty_spec)  # false; somewhere deep down the call chain from _condition_clauses

This commit removes the need for mutation by passing the package name as an explicit argument through the ASP generation logic. This was already the status quo for def _condition_clauses, the same pattern is simply propagated down the function call tree.

  • Removed the named_spec context manager used for temporary renaming.
  • Updated TransformFunction signature to (name, spec, facts).
  • Added a name fallback argument to spec_clauses, spec_versions, and target_ranges.
  • Updated _get_condition_id and _condition_clauses to use explicit names for caching and fact generation.
  • Updated runtimes.py to use the name argument in spec_clauses instead of modifying the spec directly.

@haampie haampie force-pushed the hs/fix/asp-no-spec-mutation branch 7 times, most recently from 21b30c1 to c2ea7b5 Compare January 8, 2026 11:25
This was referenced Jan 8, 2026
@haampie haampie force-pushed the hs/fix/asp-no-spec-mutation branch from c2ea7b5 to 99e5b3f Compare January 9, 2026 09:16
In asp.py and runtimes.py specs are mutated. Most of the "when" specs
are used as keys in dictionaries in PackageBase classes. Mutating their
name means invalidating their hash, leading to subtle issues like

```
spec = Spec("+foo")
specs = {spec}
spec.name = "example"
assert spec in specs # raises
```

This commit fixes that by passing `name` as a function argument.

Signed-off-by: Harmen Stoppels <[email protected]>
Signed-off-by: Harmen Stoppels <[email protected]>
@haampie haampie force-pushed the hs/fix/asp-no-spec-mutation branch from e13a4e2 to 126fb76 Compare January 9, 2026 12:47
Copy link
Copy Markdown
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like much having one more argument to many functions, but since this is needed for #51819 and it's internal API that we can refactor later I'd leave these style concerns for later.

@alalazo alalazo self-assigned this Jan 12, 2026
@alalazo alalazo merged commit a31f840 into develop Jan 12, 2026
34 checks passed
@alalazo alalazo deleted the hs/fix/asp-no-spec-mutation branch January 12, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants