Skip to content

Change default build type to empty string#6970

Closed
becker33 wants to merge 2 commits intodevelopfrom
default-cmake-build-type-empty
Closed

Change default build type to empty string#6970
becker33 wants to merge 2 commits intodevelopfrom
default-cmake-build-type-empty

Conversation

@becker33
Copy link
Copy Markdown
Member

For compatibility with compiler flags and similarity to CMake outside of Spack

Resolves #6961. For more detail, see the issue #6961.


try:
build_type = pkg.spec.variants['build_type'].value
build_type = pkg.spec.variants['build_type'].value or ''
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I had no idea you could do this in Python!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@scheibelp just showed it to me on a recent PR, it's great for keeping None checks clean!

@scheibelp
Copy link
Copy Markdown
Member

Would it be worth adding a tty.warning somewhere in CMakePackage if build_type is set, there are compiler flags, and the flag handler uses the build system? If a package overrides build_type and sets their own default, they may be confused why the flags suddenly aren't set. IMO flags_to_build_system_args would be an appropriate place.

@becker33
Copy link
Copy Markdown
Member Author

@scheibelp if we were to do that we would need to do it for all flag handlers, the bad behavior appears under all flag_handlers, it's about setting incompatible flags, in whatever way they are set.

I don't think we would want to set that warning from the build environment, because then it would be written to the build log not the terminal. I'm not sure where before that we could set it though, without seeing it every time we concretize a spec, which I think would be overkill. I don't think it's a bad idea, but I'm not sure where we could reasonably set that message.

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Aug 18, 2020

Closing as stale (similarly to the recently closed #5265)

@alalazo alalazo closed this Aug 18, 2020
@alalazo alalazo deleted the default-cmake-build-type-empty branch August 18, 2020 15:55
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.

Problems with cmake build_type variant

4 participants