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.

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

Simplify logic for defining the nowarn macro. #305

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

dholladay00
Copy link
Collaborator

@dholladay00 dholladay00 commented Oct 4, 2023

PR Summary

I am unable to install singularity-eos from spack. This may be a combination of new ports-of-call and overly complicated logic for defining the SG_PIF_NOWARN macro. This simplifies that preprocessor logic that I believe was overly complex.

If this passes tests, I think we should merge it quickly @Yurlungur @jhp-lanl @rbberger

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by using the make format command after configuring with cmake.
  • Document any new features, update documentation for changes made.
  • Make sure the copyright notice on any files you modified is up to date.
  • After creating a pull request, note it in the CHANGELOG.md file
  • If preparing for a new release, update the version in cmake.

Copy link
Collaborator

@jhp-lanl jhp-lanl left a comment

Choose a reason for hiding this comment

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

I don't fully understand the problem this is fixing, but this does seem simpler and more targeted.

@annapiegra even if this gets merged, do you have opinions?

@jhp-lanl jhp-lanl merged commit 790cba7 into main Oct 5, 2023
4 checks passed
@jhp-lanl jhp-lanl deleted the dholladay00/fix_sg_pif_nowarn branch October 5, 2023 18:16
@jhp-lanl
Copy link
Collaborator

jhp-lanl commented Oct 5, 2023

For posterity, another explanation from @dholladay00 :

There exists a bug in how that macro is defined in which it won't get defined in all cases and it must be defined in all cases or else it won't compile. This PR fixes said bug. In my case, I observed this bug when trying to install via spack so that I could work on getting singularity up to date with newer poc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants