Allow setting of LD_LIBRARY_PATH for compilers

[hegner]

As mentioned in issue #332 this adds the possiblity to specify an LD_LIBRARY_PATH in compilers.yaml

gcc@4.4.7:
    cc  = /usr/bin/gcc
    cxx = /usr/bin/g++
    f77 = /usr/bin/gfortran
    fc  = /usr/bin/gfortran
    ld_library_path = /what/ever/you/need

If considered useful, I will clean up and document.

[davydden]

@luca-heltai ping.

[tgamblin]

@hegner: This looks great. Some comments:

1. The compilers.yaml format will change slightly in the next couple weeks, to include module-loaded compilers, when Features/newarch #561 is merged. That adds a couple new sections under each compiler definition.
2. Could this be modified to use the same syntax as feature : customization of modules from configuration files #744 inside an environment: section under each compiler in compilers.yaml? That way we could add more easily add things later, b/c the environment modifications would be separate from cc, cxx, etc. Example of what I'm thinking:

gcc@4.4.7:
    cc:  /usr/bin/gcc
    cxx: /usr/bin/g++
    f77: /usr/bin/gfortran
    fc:  /usr/bin/gfortran
    environment:
        prepend:
            LD_LIBRARY_PATH: /what/ever/you/need

FYI: #561 will actually allow us to specifically specific modules to load to use a particular compiler, so it will probably solve this problem for most users. However, I think it's useful to allow adding arbitrary environment changes to the compilers, too. The above syntax uses @alalazo's module customization syntax to modify the environment here and in modules.

What do you think?

[hegner]

@tgamblin - no prob. I will do that and update the docs accordingly. I would propose a two stage procedure here:

1. Change the config syntax as you propose for supporting LD_LIBRARY_PATH only, clean up and merge soon. Seems a semi-blocker to many people.
2. Wait for merge of Features/newarch #561 and then mirror all the environment settings that are relevant from feature : customization of modules from configuration files #744. Would have lower priority for me unless an important use case pops up.

Do you agree?

[tgamblin]

@hegner: sounds good to me. I suspect for phase 1 you should be able to reuse some of @alalazo's Python code very easily. However, in cc, there is no bash implementation of the general env setting code (@alalazo's code generates modules for that), so adding code just for LD_LIBRARY_PATH seems reasonable to me.

[tgamblin]

@hegner: thinking about it, you may want to throw DYLD_LIBRARY_PATH in there as well, to cover Mac OS X.

[hegner]

@tgamblin - same parameter name in the yaml, but I already set both LD and DYLD from it. Can make it cleaner.

[alalazo]

@hegner @tgamblin I don't know if I am overlooking something, but I don't get what is the mechanism that prevents the second unset of LD_LIBRARY_PATH in lib/spack/env/cc...

[hegner]

@alalazo - yapp. need to get that fixed again. Was a quick re-surrection.

[alalazo]

I was wondering : if we want to maintain the same level of 'security' in unsetting these variables, we could

1. set SPACK_ prefixed variables in python code (e.g. SPACK_LD_LIBRARY_PATH)
2. if those are present resurrect the corresponding variable in lib/spack/env/cc after they have just been unset

What do you think?

[hegner]

Sounds good.

[hegner]

@alalazo - then it would look like what I just pushed. I'll address the better config syntax proposal of @tgamblin, the doc and flake8 over weekend.

[luca-heltai]

I personally like this solution. Nice PR!

I had to hack around a lot of things, but with this, everything should work for my setting...

[hegner]

Feature complete for now. One can set LD_LIBRARY_PATH or DYLD_LIBRARY_PATH via

    gcc@4.9.3:
        cc: /opt/gcc/bin/gcc
        c++: /opt/gcc/bin/g++
        f77: /opt/gcc/bin/gfortran
        fc: /opt/gcc/bin/gfortran
        environment:
            set:
                LD_LIBRARY_PATH : /opt/gcc/lib

Implementation wise I make the assumption that I only do a set on environment variables. Which variables can be set is kept dynamic and purely limited by the schema definition of the configuration. In case other variables may be needed, intervention is limited to exactly one place.

[adamjstewart]

Has this been tested with multiple environment variables? What about an LD_LIBRARY_PATH that contains a colon separated list of paths?

[hegner]

@adamjstewart - yap. You can do that with any variable - given it is allowed by the config schema. For now only LD_LIBRARY_PATH and DYLD_LIBRARY_PATH are allowed via lib/spack/spack/config:179. Checked for colon separated variables as well. I had that in my use case and was the first thing I tested 😄

If you want to verify in a poor man's way that things are actually set, you can just add some fake paths

  environment:
    set:
      LD_LIBRARY_PATH: I/should:definitely/see
      DYLD_LIBRARY_PATH: this/output

to your compiler config and add an echo in the compiler wrapper

--- a/lib/spack/env/cc
+++ b/lib/spack/env/cc
@@ -248,6 +248,8 @@ for varname in "${varnames[@]}"; do
   export $varname=${!spack_varname}
 done

+echo $LD_LIBRARY_PATH
+echo $DYLD_LIBRARY_PATH
 #
 # Filter '.' and Spack environment directories out of PATH so that
 # this script doesn't just call itself

Then any build will have its .spack/build.log flodded with

I/should:definitely/see
this/output

[adamjstewart]

I like this implementation a lot, but I can't help but wonder if it could be made more generic. The Intel compilers in particular make use of a lot of different environment variables (sourced from compilervars.sh). The NAG compiler requires the NAG_KUSARI_FILE variable to be set in order to use it. Yes, all of these variables could be set by first loading a module. But it would be way cooler to be able to set any necessary variables in compilers.yaml. Is there some language limitation that prevents this?

[hegner]

@adamjstewart - I limited it to these two paths to prevent feature creep. Implementation wise the difference is something < 10 lines of code to allow setting any variable. Just a change on the config schema. I hope @tgamblin would be fine with that change. If yes, it's a matter of a coffee break to code and test.

[hegner]

@adamjstewart see this commit: b5f89c3 : 8 lines including docs.

[KineticTheory]

I found another hiccup to this approach. While setting LD_LIBRAR_PATH=$SPACK_LD_LIBRARY_PATH in spack's compile wrapper works for building code, it doesn't account for a configure process that tries to run locally generated binaries (e.g.: cmake). To build cmake, I needed to add this:

diff --git a/var/spack/repos/builtin/packages/cmake/package.py b/var/spack/repos/builtin/packages/cmake/package.py
index 7b2a125..0c88d3c 100644
--- a/var/spack/repos/builtin/packages/cmake/package.py
+++ b/var/spack/repos/builtin/packages/cmake/package.py
@@ -23,6 +23,8 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 from spack import *
+import os
+

 class Cmake(Package):
     """A cross-platform, open-source build system. CMake is a family of
@@ -70,6 +72,10 @@ class Cmake(Package):
         # Consistency check
         self.validate(spec)

+        if os.environ.get('SPACK_LD_LIBRARY_PATH') != "None":
+            os.environ['LD_LIBRARY_PATH'] \
+                = os.environ.get('SPACK_LD_LIBRARY_PATH')
+
         # configure, build, install:
         options = ['--prefix=%s' % prefix]
         options.append('--parallel=%s' % str(make_jobs))

Can anyone think of a way to transform this hack into a cleaner implementation?

[alalazo]

@KineticTheory You're right : we should set LD_LIBRARY_PATH (for configure time) and SPACK_LD_LIBRARY_PATH (for compile time) to the same value in python code.

[hegner]

@KineticTheory thanks for testing! You are right. I broke that part when I started prefixing things with SPACK_ to protect things within the compiler wrapper. I now provide both as correctly proposed by @alalazo. Can you check in your setup again?

[hegner]

@davydden - not yet. will address that soon. Sorry for the delay.

[citibeth]

Should this PR be closed now that we can load modules for a compile? Or are there still use cases?

[KineticTheory]

@citibeth Loading the compiler module via compilers.yaml doesn't help me with %intel@16.0.3. I have to run install with --dirty to get anything to build. However, if the dirty option is the path forward, then I think we can close this issue.

compilers.yaml

- compiler:
    modules:
      - intel/16.0.3
    operating_system: redhat6
    paths:
      cc: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/icc
      cxx: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/icpc
      f77: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/ifort
      fc: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/ifort
    spec: intel@16.0.3

Test 1 (default)

$ spack install -v ncurses
...
tic: error while loading shared libraries: libimf.so: cannot open shared object file: No such file or directory

Test 2 (dirty)

$ spack install -v --dirty ncurses
...
==> Successfully installed ncurses

[citibeth]

I've had the same experience.

I think the dirty option is the path forward; asking users to sanitize
their path for Spack isn't so bad. And for naive users with no-module
compilers, that is not even necessary.

Forcing the user to write --dirty is NOT the path forward. We need a way
to configure this. I proposed using an env var in #1625 and am looking for
feedback. I think the right approach is a config file somewhere. But I
don't know where, or how to set that up.

-- Elizabeth

#1625

On Mon, Aug 29, 2016 at 10:17 PM, Kelly Thompson notifications@github.com
wrote:

@citibeth https://github.com/citibeth Loading the compiler module via
compilers.yaml doesn't help me with %intel@16.0.3. I have to run install
with --dirty to get anything to build. However, if the dirty option is
the path forward, then I think we can close this issue.

compilers.yaml

• compiler:
  modules:
  • intel/16.0.3
    operating_system: redhat6
    paths:
    cc: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/icc
    cxx: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/icpc
    f77: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/ifort
    fc: /yellow/usr/projects/hpcsoft/toss2/common/intel-clusterstudio/2016.3.067/compilers_and_libraries_2016/linux/bin/intel64/ifort
    spec: intel@16.0.3

Test 1 (default)

$ spack install -v ncurses
...
tic: error while loading shared libraries: libimf.so: cannot open shared object file: No such file or directory

Test 2 (dirty)

$ spack install -v --dirty ncurses
...
==> Successfully installed ncurses

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#943 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd-g_g8ElDdpZWkuZT1Il4GHzwdVrks5qk5LPgaJpZM4IdWEB
.

[davydden]

Should this PR be closed now that we can load modules for a compile? Or are there still use cases?

My understanding that loading modules will also fix issues experienced by @luca-heltai here https://groups.google.com/d/topic/spack/I7CQgmwHmLI/discussion

[citibeth]

I'm forced to use a particularly evil compiler that was built by hand. Not only do you need to load a module to RUN it, you also have to load the same module to run anything built by it. In practice, that means that --dirty is required and the module loading is redundant (for this compiler).

For moderately evil compilers, I think the module loading feature in Spack is nice.

I think this thread can be closed.

[tgamblin]

This thread shouldn't be closed. Allowing compilers (even ones that don't have modules) to run in a clean environment is still a priority, I just haven't gotten to it. I actually view this issue as a pretty high priority. It's a barrier to entry for a number of users at LANL, for instance, and requiring --dirty is a workaround as the name implies.

[citibeth]

I suppose the suggested improvement would address some of the issues raised in this thread, and is worth doing for that reason.

It will not help me with my evil compiler because build processes at times will need to run code produced by my compiler. The LD_LIBRARY_PATH needs to be in effect not just while running the compiler, but all the time. Using --dirty is not great, but I don't see an obviously better solution to my problem.

[tgamblin]

Using compilers that RPATH properly is another issue. But the mechanism for this would also allow us to add compiler runtime paths that Spack should add to RPATH of the generated executable, which would help you with that. We would just add another property in compilers.yaml for runtime paths.

[citibeth]

If spack can fix my evil compiler so I no longer need to load modules to
run its output, that would definitely be an improvement.

On Wed, Sep 21, 2016 at 1:22 PM, Todd Gamblin notifications@github.com
wrote:

Using compilers that RPATH properly is another issue. But the mechanism
for this would also allow us to add compiler runtime paths that Spack
should add to RPATH of the generated executable, which would help you
with that. We would just add another property in compilers.yaml for
runtime paths.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#943 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd-hU83UPR10UtdGOLy4bj9ENKM8tks5qsWe-gaJpZM4IdWEB
.

[scheibelp]

Hi folks, I was asked to take a crack at updating this to be compatible with current Spack. The results are at #2275. That should likely be closed and that work integrated here but for now I've opened it as a separate PR.

One question I've got: why is this done with coordination between build_environment.set_build_environment_variables and the compiler wrapper, vs. just setting 'env' in build_environment.set_build_environment_variables? If the function were used it seems like other types of modifications like prepending to LD_LIBRARY_PATH would be possible.

Thanks,
Peter

[scheibelp]

We would just add another property in compilers.yaml for runtime paths.

@tgamblin Perhaps a foolish question but do you mean add a variable in compilers.yaml to be used to decide how to set up PATH for a package when its binaries are run? I suppose I'm not clear how that would be achieved in general. Module files currently allow setting/prepending/etc. for specific packages, but without module files I'm not clear on how that is generally done.

[alalazo]

@scheibelp

One question I've got: why is this done with coordination between build_environment.set_build_environment_variables and the compiler wrapper, vs. just setting 'env' in build_environment.set_build_environment_variables? If the function were used it seems like other types of modifications like prepending to LD_LIBRARY_PATH would be possible.

If I remember well the motivation is that until not so long ago LD_LIBRARY_PATH and friends were unset both in python code AND in the compiler wrapper.

Using just env will work now because we changed this behavior in #2072

[hegner]

@alalazo - that's correct. This PR pre-dates a few improvements
@scheibelp - thanks for updating this PR

[tgamblin]

Perhaps a foolish question but do you mean add a variable in compilers.yaml to be used to decide how to set up PATH for a package when its binaries are run? I suppose I'm not clear how that would be achieved in general. Module files currently allow setting/prepending/etc. for specific packages, but without module files I'm not clear on how that is generally done.

It's been a while but I think I was suggesting we could also have a section in compilers.yaml that would allow sites to force the compiler to RPATH the compiler's runtime libs. So, if you had another setting in compilers.yaml:

compilers:
    - compiler:
        environment:
            # ...
        extra_rpaths:
            - /path/to/some/compiler/runtime/directory
            - /path/to/some/other/compiler/runtime/directory

Some compilers fails to RPATH in their own runtime directory and binaries break when the generated executables pick up conflicting system libraries. This would force generated code to use the runtime libs of the compiler they were built with. cc would then need to add the RPATHs.

[citibeth]

Yes! This is what we need for compilers like the ones I'm forced to use!

On Tue, Nov 8, 2016 at 10:57 AM, Todd Gamblin notifications@github.com
wrote:

@tgamblin https://github.com/tgamblin Perhaps a foolish question but do
you mean add a variable in compilers.yaml to be used to decide how to set
up PATH for a package when its binaries are run? I suppose I'm not clear
how that would be achieved in general. Module files currently allow
setting/prepending/etc. for specific packages, but without module files I'm
not clear on how that is generally done.

It's been a while but I think I was suggesting we could also have a
section in compilers.yaml that would allow sites to force the compiler to
RPATH the compiler's runtime libs. So, if you had another setting in
compilers.yaml:

compilers:
- compiler:
environment: # ...
extra_rpaths:
- /path/to/some/compiler/runtime/directory
- /path/to/some/other/compiler/runtime/directory

Some compilers fails to RPATH in their own runtime directory and binaries
break when the generated executables pick up conflicting system libraries.
This would force generated code to use the runtime libs of the compiler
they were built with.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#943 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd8frRbkXT3EAp8GRkl2HxIEZafERks5q8Jv4gaJpZM4IdWEB
.

[tgamblin]

@scheibelp: want to take a stab at the above tweaks?

[scheibelp]

@scheibelp: want to take a stab at the above tweaks?

Yes I can do that. Should be ready later today

[tgamblin]

Ported to #2279 and merged. Thanks @hegner