Skip to content

update compiler config with bootstrapped compiler when already installed#16221

Merged
tldahlgren merged 4 commits intodevelopfrom
bugfix/bootstrap-compiler-already-installed
Apr 22, 2020
Merged

update compiler config with bootstrapped compiler when already installed#16221
tldahlgren merged 4 commits intodevelopfrom
bugfix/bootstrap-compiler-already-installed

Conversation

@becker33
Copy link
Copy Markdown
Member

When config:install_missing_compilers is True, spack install foo %compiler_not_available works if the compiler is not installed, but fails if the compiler is already installed.

This PR adds logic to update the compiler config upon finding that a needed compiler is already installed, in addition to doing so after installing it if it were not already installed.

@becker33 becker33 added the bugfix Something wasn't working, here's a fix label Apr 21, 2020
@tldahlgren tldahlgren self-requested a review April 21, 2020 22:45
Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

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

Reviewed the code in context and see why this should've been added.

LGTM. It just needs a unit test.

@becker33 becker33 force-pushed the bugfix/bootstrap-compiler-already-installed branch from a0162c0 to 4d361c8 Compare April 22, 2020 19:12
@becker33
Copy link
Copy Markdown
Member Author

@tldahlgren test added and ready to go.

Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

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

LGTM and new tests run for me.

@tldahlgren tldahlgren merged commit ec23e4f into develop Apr 22, 2020
@tldahlgren
Copy link
Copy Markdown
Contributor

Thank you for your contribution.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants