-
Notifications
You must be signed in to change notification settings - Fork 284
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
required changes for toolchain which replaces toolkit + cleanups/enhancements #7
Conversation
…constants are being used (they are defined at runtime by EasyBuild)
opts = { | ||
'mpicc': "%s %s" % (os.getenv('MPICC'), os.getenv('CFLAGS')), | ||
'mpif77': "%s %s" % (os.getenv('MPIF77'), os.getenv('FFLAGS')), | ||
'f77': os.getenv('F77'), | ||
'cc': os.getenv('CC'), | ||
'builddir': os.getcwd(), | ||
'base': base, | ||
'mpilib': mpilib | ||
'mpidir': os.path.dirname(os.getenv('MPI_LIB_DIR')), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at some point we will have to specify what the best way is to do this: through environment or through toolchain instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with best i meant "recommended"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel environment variables are easier, no API for people to get familiar with, they just need to know which environment variable to use. It also allows us to make changes to toolchain, if needed.
The biggest downside is that something else may be setting the environment variables as well, outside of EasyBuild, I guess.
Is there currently an easy way to access stuff like CC
, MPICC
and things like CFLAGS
and MPI_LIB_DIR
via toolchain
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't understand the question. str(self.toolchain.variables['CC'])
? you could write a tiny function (get_variable
) to do just that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize it was that simple. :) But it does make a lot of sense in retrospect.
Having a get_variable
function makes sense, it hides the guts of toolchain, which is a good thing (because then we can change it without affecting outsiders).
So... Should we change this everywhere? That's going to be a fun exercise... Hard to test if we got them all too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ask feedback and make the preferred method known. actual work can be done later and in seperate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See easybuilders/easybuild-framework#298 for a discussion on this.
…, cxxflags and fflags (Trilinos easyblock)
required changes for toolchain which replaces toolkit + cleanups/enhancements
parse output of 'g++ --print-search-dirs' in Python
style fixes for SuiteSparse easyblock
add back missing import for LooseVersion in MCR easyblock
prepare release notes for eb302
…pi-support don't use -static for AmberTools + fix sanity check
use self.version rather than 'latest' symlink to point to correct subdirectory in impi build dir
No description provided.