Skip to content

Fix issues for LD_LIBRARY_PATH via EESSI module #114

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix issues for LD_LIBRARY_PATH via EESSI module #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d7c62c3
Fix issues for locale, lesspipe and LD_LIBRARY_PATH in EESSI module
ocaisa Oct 23, 2025
ff7192f
Update 2023.06.lua
ocaisa Oct 23, 2025
e09c480
Print every character to see what is going wrong
ocaisa Oct 23, 2025
b87ff12
Typo fix
ocaisa Oct 23, 2025
627be08
Explicitly set a locale to avoid dealing with Lmod behaviour around L…
ocaisa Oct 23, 2025
8ebad08
Don't fiddle with locale for now
ocaisa Oct 23, 2025
a8382c7
Also remove locale setting in CI
ocaisa Oct 23, 2025
5f2c89b
Update 2023.06.lua
ocaisa Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 43 additions & 4 deletions .github/workflows/tests_eessi_module.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ jobs:
# EESSI_DEBUG_INIT/EESSI_ARCHDETECT_OPTIONS only relevant for Lmod init
unset EESSI_DEBUG_INIT
# Store all relevant environment variables
env | grep -E '(^EESSI_|^LMOD_RC|^LMOD_PACKAGE_PATH|^MODULEPATH)' | grep -v EESSI_ARCHDETECT_OPTIONS | sort > "${moduleoutfile}"
env | grep -E '(^EESSI_|^LMOD_RC|^LMOD_PACKAGE_PATH|^MODULEPATH)' | grep -v EESSI_ARCHDETECT_OPTIONS | grep -v EESSI_DEFAULT_HOST_LD_LIBRARY_PATH | sort > "${moduleoutfile}"
module unload EESSI/${{matrix.EESSI_VERSION}}

# We should only have two EESSI_* variables defined (which set the overrides)
Expand Down Expand Up @@ -212,14 +212,14 @@ jobs:
module unuse .github/workflows/modules
module avail

# Store the initial environment (ignoring Lmod tables)
env | grep -v _ModuleTable | sort > "${initial_env_file}"
# Store the initial environment (ignoring Lmod tables and internal stack varibles)
env | grep -v _ModuleTable | grep -v __LMOD_STACK | sort > "${initial_env_file}"

# Do (and undo) loading the EESSI module
CPU_ARCH=$(./init/eessi_archdetect.sh -a cpupath)
module load EESSI/${{matrix.EESSI_VERSION}}
module unload EESSI/${{matrix.EESSI_VERSION}}
env | grep -v _ModuleTable | sort > "${module_cycled_file}"
env | grep -v _ModuleTable | grep -v __LMOD_STACK |sort > "${module_cycled_file}"

# Now compare the two results (do not expose the files, as they contain the full environment!)
if (diff "${initial_env_file}" "${module_cycled_file}" > /dev/null); then
Expand Down Expand Up @@ -315,3 +315,42 @@ jobs:
module load "EESSI/${{matrix.EESSI_VERSION}}"
GREP_PATTERN="Lmod is automatically replacing \"EESSI/${{matrix.EESSI_VERSION}}\" with \"${LOCAL_STACK_NAME}/${LOCAL_STACK_VERSION}\"."
module load "${LOCAL_STACK_NAME}/${LOCAL_STACK_VERSION}" |& grep "${GREP_PATTERN}"

check_ld_library_path_eessi_module:
runs-on: ubuntu-24.04
strategy:
fail-fast: false
matrix:
EESSI_VERSION:
- '2023.06'
- '2025.06'

steps:
- name: Check out software-layer repository
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- name: Mount EESSI CernVM-FS repository
uses: cvmfs-contrib/github-action-cvmfs@55899ca74cf78ab874bdf47f5a804e47c198743c # v4.0
with:
cvmfs_config_package: https://github.com/EESSI/filesystem-layer/releases/download/latest/cvmfs-config-eessi_latest_all.deb
cvmfs_http_proxy: DIRECT
cvmfs_repositories: software.eessi.io

- name: Make sure we are filtering LD_LIBRARY_PATH if necessary
run: |
# Initialise Lmod
. /cvmfs/software.eessi.io/versions/${{matrix.EESSI_VERSION}}/compat/linux/$(uname -m)/usr/share/Lmod/init/bash

# Make sure we are using the EESSI module file from the repository
export MODULEPATH=init/modules

# Create LD_LIBRARY_PATH to test with
export FAKE_PATH=/path/does/not/exist
export INITIAL_LD_LIBRARY_PATH="/usr/lib:$FAKE_PATH"
export LD_LIBRARY_PATH="$INITIAL_LD_LIBRARY_PATH"
module load "EESSI/${{matrix.EESSI_VERSION}}"
# Check we have a sanitised LD_LIBRARY_PATH
[ "$LD_LIBRARY_PATH" = "$FAKE_PATH" ] || (echo "LD_LIBRARY_PATH is not $FAKE_PATH but $LD_LIBRARY_PATH" && exit 1)
# Make sure things are back as we expect afterwards
module purge
[ "$LD_LIBRARY_PATH" = "$INITIAL_LD_LIBRARY_PATH" ] || (echo "LD_LIBRARY_PATH is not $INITIAL_LD_LIBRARY_PATH but $LD_LIBRARY_PATH" && exit 1)
16 changes: 16 additions & 0 deletions init/modules/EESSI/2023.06.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,22 @@ if os.getenv("EESSI_MODULE_STICKY") then
load_message = load_message .. " (requires '--force' option to unload or purge)"
end

-- Filter system paths from LD_LIBRARY_PATH
-- Needs to be reversible so first make a copy
append_path ("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH", os.getenv("LD_LIBRARY_PATH") or "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an append_path? Should't it just overwrite? What if at the start EESSI_DEFAULT_HOST_LD_LIBRARY_PATH=/foo and LD_LIBRARY_PATH=/bar. I think what you want is that after unloading LD_LIBRARY_PATH=/bar, but with the current setup, it would be LD_LIBRARY_PATH=/foo:/bar no?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the Lmod documentation it explicitly says to use append_path when initialising a path-like variable (which we are doing here), and indeed if you don't it doesn't work properly (I tried).

I think you are not quite following what is happening here:

  • We make a copy of the initial value of the original LD_LIBRARY_PATH
  • We sanitize the copy
  • We use pushenv to set LD_LIBRARY_PATH to the sanitized value.
  • Using pushenv means the approach is reversible. If we had worked directly on LD_LIBRARY_PATH unloading would not restore the original environment (the path would remain sanitized)

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it help to do more extensive commenting?

Copy link
Member Author

@ocaisa ocaisa Oct 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I see your point about EESSI_DEFAULT_HOST_LD_LIBRARY_PATH, but it is not expected that it exists. Maybe the right way to imply that is by naming it_EESSI_DEFAULT_HOST_LD_LIBRARY_PATH_ and verifying it is not set (os.getenv(_EESSI_DEFAULT_HOST_LD_LIBRARY_PATH_)should be nil)

-- on unload the variable will no longer exist
if mode() == "load" then
-- remove any standard paths that can interfere with the compat layer
remove_path ("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH", "/lib")
remove_path ("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH", "/lib64")
remove_path ("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH", "/usr/lib")
remove_path ("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH", "/usr/lib64")
remove_path ("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH", "/usr/local/lib")
remove_path ("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH", "/usr/local/lib64")
Comment on lines +218 to +224
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused, didn't you intend to remove from LD_LIBRARY_PATH here? I.e. now, you are essentially leaving LD_LIBRARY_PATH unsanitized, while you're cleaning EESSI_DEFAULT_HOST_LD_LIBRARY_PATH (which I think you meant to keep as a the original value so you can restore later)

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, I see that your test passes, I just don't understand how... I don't think it should pass with the current code...

Copy link
Member Author

Choose a reason for hiding this comment

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

We make a copy of LD_LIBRARY_PATH, clean it up, and then use push_env to update LD_LIBRARY_PATH. This means after loading the module the LD_LIBRARY_PATH has the sanitised value. On unload, the original value get's restored

end
-- now we can use pushenv to retain/restore the original value
pushenv ("LD_LIBRARY_PATH", os.getenv("EESSI_DEFAULT_HOST_LD_LIBRARY_PATH") or "")

if mode() == "load" then
LmodMessage(load_message)
end