Skip to content

spec parser: ensure toolchains are expanded to different objects#51731

Merged
becker33 merged 1 commit intospack:developfrom
alalazo:bugfix/toolchain-expansion-same-id
Dec 12, 2025
Merged

spec parser: ensure toolchains are expanded to different objects#51731
becker33 merged 1 commit intospack:developfrom
alalazo:bugfix/toolchain-expansion-same-id

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Dec 10, 2025

fixes #51606

If:

foo %gnu ^bar %gnu

has %gnu expanding to the same objects in memory, it may happen that some facts for the literal spec are not emitted correctly.

fixes spack#51606

If "foo %gnu ^bar %gnu" has %gnu expanding to the same
object in memory, it may happen that some facts for the
literal spec are not emitted correctly.

Signed-off-by: Massimiliano Culpo <[email protected]>
@becker33
Copy link
Copy Markdown
Member

Actually, quick question -- wouldn't it be more robust to fix this by changing the seen set in asp.py to track edges instead of nodes?

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Dec 12, 2025

wouldn't it be more robust to fix this by changing the seen set in asp.py to track edges instead of nodes?

Hmm, not really. We use that branch to descend into sub-graphs which is a per node decision. Probably the most robust solution is to fix the TODO in #51727 but that is much bigger than this PR.

@becker33 becker33 merged commit a60742f into spack:develop Dec 12, 2025
56 of 59 checks passed
@becker33
Copy link
Copy Markdown
Member

I'm not convinced "every node is a separate object" is a good assumption in abstract specs, even without this bug. But that can be a separate conversation.

@alalazo alalazo deleted the bugfix/toolchain-expansion-same-id branch December 12, 2025 18:32
becker33 pushed a commit that referenced this pull request Jan 10, 2026
)

fixes #51606

If "foo %gnu ^bar %gnu" has %gnu expanding to the same
object in memory, it may happen that some facts for the
literal spec are not emitted correctly.

Signed-off-by: Massimiliano Culpo <[email protected]>
@becker33 becker33 mentioned this pull request Jan 10, 2026
2 tasks
becker33 pushed a commit that referenced this pull request Jan 11, 2026
)

fixes #51606

If "foo %gnu ^bar %gnu" has %gnu expanding to the same
object in memory, it may happen that some facts for the
literal spec are not emitted correctly.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 15, 2026
)

fixes #51606

If "foo %gnu ^bar %gnu" has %gnu expanding to the same
object in memory, it may happen that some facts for the
literal spec are not emitted correctly.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
…ck#51731)

fixes spack#51606

If "foo %gnu ^bar %gnu" has %gnu expanding to the same
object in memory, it may happen that some facts for the
literal spec are not emitted correctly.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 22, 2026
)

fixes #51606

If "foo %gnu ^bar %gnu" has %gnu expanding to the same
object in memory, it may happen that some facts for the
literal spec are not emitted correctly.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 26, 2026
)

fixes #51606

If "foo %gnu ^bar %gnu" has %gnu expanding to the same
object in memory, it may happen that some facts for the
literal spec are not emitted correctly.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
@becker33
Copy link
Copy Markdown
Member

becker33 commented Feb 2, 2026

This PR does not apply cleanly to releases/v1.0 and will be dropped from v1.0.3

@becker33 becker33 removed the v1.0.3 PRs to backport for v1.0.3 label Feb 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something wasn't working, here's a fix v1.1.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Specifying a toolchain more than once in the same root spec doesn't work correctly if it contains unconditional dependencies

2 participants