Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

scons: fix signature for install_args #34481

Merged
merged 1 commit into from
Dec 13, 2022
Merged

scons: fix signature for install_args #34481

merged 1 commit into from
Dec 13, 2022

Conversation

alalazo
Copy link
Member

@alalazo alalazo commented Dec 12, 2022

Hopefully the last issue falling out from #30738. @amd-toolchain-support This should fix the build of amdlibm.

@spackbot-app spackbot-app bot added build-systems core PR affects Spack core functionality labels Dec 12, 2022
tldahlgren
tldahlgren reviewed Dec 12, 2022
View reviewed changes
Copy link
Contributor

@tldahlgren tldahlgren left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Should MavenPackage (e.g., hadoop-xrootd) and fpm be changed for install_args and build_args to pass the same arguments for consistency?

@alalazo
Copy link
Member Author

alalazo commented Dec 12, 2022

No for both. The point is exactly that - I did a breaking change to enforce consistency of the *_args methods in Scons and in a few other build systems early when writing #30738 (the vast majority accepts just self). Later we decided to be 100% backward compatible, including these inconsistencies, but I overlooked reverting some lines.

@alalazo
Copy link
Member Author

alalazo commented Dec 12, 2022

For reference, the method that have a different signature from the others of the same kind are the ones listed in the legacy_long_methods attributes in build system classes.

@tldahlgren
Copy link
Contributor

Okay. Thanks for the clarification.

adamjstewart
adamjstewart approved these changes Dec 13, 2022
View reviewed changes
Copy link
Member

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

This reminds me, we should get back to our discussion of whether functions should be self, self, spec, prefix, self, pkg, spec, prefix, or some combination of these.

@adamjstewart adamjstewart self-assigned this Dec 13, 2022
@adamjstewart adamjstewart added this to the v0.19.1 milestone Dec 13, 2022
@alalazo alalazo mentioned this pull request Dec 13, 2022
Merged
20 tasks
@alalazo
Copy link
Member Author

alalazo commented Dec 13, 2022

@spackbot run pipeline

@spackbot-app
Copy link

spackbot-app bot commented Dec 13, 2022

I've started that pipeline for you!

@alalazo alalazo merged commit f9cfc2f into spack:develop Dec 13, 2022
@alalazo alalazo deleted the bugfixes/scons_install_args branch December 13, 2022 11:21
@dylex dylex mentioned this pull request Dec 13, 2022
Open
charmoniumQ pushed a commit to charmoniumQ/spack that referenced this pull request Dec 13, 2022
@alalazo @charmoniumQ
scons: fix signature for install_args (spack#34481)
68c031a
benkirk pushed a commit to benkirk/spack that referenced this pull request Dec 16, 2022
@alalazo @benkirk
scons: fix signature for install_args (spack#34481)
2e648a0
haampie pushed a commit that referenced this pull request Dec 22, 2022
@alalazo @haampie
scons: fix signature for install_args (#34481)
9a575cf
haampie pushed a commit that referenced this pull request Dec 23, 2022
@alalazo @haampie
scons: fix signature for install_args (#34481)
e2f30a0
haampie pushed a commit that referenced this pull request Jan 18, 2023
@alalazo @haampie
scons: fix signature for install_args (#34481)
595d654
haampie pushed a commit that referenced this pull request Jan 24, 2023
@alalazo @haampie
scons: fix signature for install_args (#34481)
39955c2
haampie pushed a commit that referenced this pull request Feb 7, 2023
@alalazo @haampie
scons: fix signature for install_args (#34481)
c7ad46c
alalazo added a commit that referenced this pull request Feb 7, 2023
@alalazo
scons: fix signature for install_args (#34481)
8d45404
amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023
@alalazo
scons: fix signature for install_args (spack#34481)
bca371f
tygoetsch pushed a commit to lanl-preteam/spack that referenced this pull request Dec 9, 2023
apply backwards compatibility fix for scons.py mentioned in spack issue
efc3605 
spack#34481 so amdlibm will build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tldahlgren tldahlgren tldahlgren left review comments

@adamjstewart adamjstewart adamjstewart approved these changes

Assignees

@adamjstewart adamjstewart

Labels
build-systems core PR affects Spack core functionality
Projects
None yet
Milestone
v0.19.1
Development

Successfully merging this pull request may close these issues.
3 participants
@alalazo @tldahlgren @adamjstewart