Skip to content

compiler flags: fix mixed flags from cli and yaml#34218

Merged
tgamblin merged 1 commit intodevelopfrom
bugfix/mixed-compiler-flags-cli-yaml
Dec 7, 2022
Merged

compiler flags: fix mixed flags from cli and yaml#34218
tgamblin merged 1 commit intodevelopfrom
bugfix/mixed-compiler-flags-cli-yaml

Conversation

@becker33
Copy link
Copy Markdown
Member

@becker33 becker33 commented Nov 30, 2022

Fixes #34206
Fixes #27736

The concretizer logic to properly order flags returned from the logic program had a bug that prevented merging constraints from both compilers.yaml and CLI compiler flags.

@spackbot-app spackbot-app bot added core PR affects Spack core functionality tests General test capability(ies) labels Nov 30, 2022
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Nov 30, 2022

@vkallesh does this work for you?

@lukebroskop lukebroskop mentioned this pull request Nov 30, 2022
Closed
3 tasks
@tgamblin tgamblin merged commit 194f9a9 into develop Dec 7, 2022
@tgamblin tgamblin deleted the bugfix/mixed-compiler-flags-cli-yaml branch December 7, 2022 00:32
@msimberg msimberg mentioned this pull request Dec 7, 2022
Closed
3 tasks
@haampie
Copy link
Copy Markdown
Member

haampie commented Jan 20, 2023 •
edited
Loading

This is still broken:

spack:
  concretizer:
    unify: true
  specs:
  - python cflags==-a
  - curl cflags==-b
  File "/home/user/spack/lib/spack/spack/solver/asp.py", line 2315, in reorder_flags
    assert set(compiler_flags) == set(ordered_compiler_flags), msg
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: {'-a'} does not equal {'-b', '-a'}

amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023
@becker33
compiler flags: fix mixed flags from cli and yaml (spack#34218)
a8e0470
@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 28, 2023

develop (a72021f, blue) vs. PR (194f9a9, orange)

radiuss_develop.csv
radiuss_34218.csv
radiuss.txt

No substantial differences, as expected:

totals

@jonasjucker jonasjucker mentioned this pull request Feb 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgamblin tgamblin tgamblin approved these changes

Assignees

No one assigned

Labels

core PR affects Spack core functionality tests General test capability(ies)

Projects

None yet

Milestone

No milestone

4 participants

@becker33 @tgamblin @haampie @alalazo