Skip to content

z21_epic_main.sh: block propagation of $1 - #430

Open
veprbl wants to merge 1 commit into
masterfrom
pr/thisepic_no_geom
Open

veprbl wants to merge 1 commit into
masterfrom
pr/thisepic_no_geom

Conversation

@veprbl

@veprbl veprbl commented Sep 20, 2026

Copy link
Copy Markdown
Member

Currently a simple

singularity exec /cvmfs/eic.opensciencegrid.org/singularity/eicweb/eic_xl:26.07.1-stable eicrecon test.edm4hep.root

fails with

ERROR: file: /opt/software/linux-x86_64_v2/epic-26.07.1-uzwreubimqduvctpoysj5wmbj6ay5emu/share/epic/eicrecon.xml does not exist!
Check that your DETECTOR_PATH and DETECTOR_CONFIG environment variables are set correctly.

i.e. eicrecon is passed as $1 and is used by entrypoint.sh as the command to exec and by thisepic.sh to set DETECTOR_CONFIG.

This will source thisepic.sh in such a way that no detector config is set.

Currently a simple
```
singularity exec /cvmfs/eic.opensciencegrid.org/singularity/eicweb/eic_xl:26.07.1-stable eicrecon test.edm4hep.root
```
fails with
```
ERROR: file: /opt/software/linux-x86_64_v2/epic-26.07.1-uzwreubimqduvctpoysj5wmbj6ay5emu/share/epic/eicrecon.xml does not exist!
Check that your DETECTOR_PATH and DETECTOR_CONFIG environment variables are set correctly.
```
i.e. eicrecon is passed as $1 and is used by entrypoint.sh as the command to exec and by thisepic.sh to set 
DETECTOR_CONFIG.

This will source thisepic.sh in such a way that no detector config is set.
@github-actions

Copy link
Copy Markdown

Capybara summary for PR 430

@veprbl

veprbl commented Sep 20, 2026

Copy link
Copy Markdown
Member Author

Tested using singularity exec docker://ghcr.io/eic/eic_xl:unstable-mr-430-nightly eicrecon /dev/null

@veprbl
veprbl marked this pull request as ready for review September 20, 2026 20:38
Copilot AI lite review requested due to automatic review settings September 20, 2026 20:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The wrapper correctly isolates positional parameters while preserving existing detector initialization behavior.

Review effort: Lite
Findings: None

What changed in this PR

Prevents container command arguments from being interpreted as detector configuration when sourcing thisepic.sh.

Changes:

  • Adds a wrapper that clears positional arguments before sourcing.
  • Uses the wrapper for automatic EPIC environment initialization.
File Description
containers/​eic/​profile.d/​z21_epic_main.sh Prevents $1 propagation into thisepic.sh.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@wdconinc

Copy link
Copy Markdown
Contributor

How does this change behavior of echo command | eic-shell or eic-shell -- command when args are present?

@veprbl

veprbl commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

How does this change behavior of echo command | eic-shell or eic-shell -- command when args are present?

Don't see a problem:

% echo ps | singularity exec docker://ghcr.io/eic/eic_xl:unstable-mr-430-nightly eic-shell sh
INFO:    Using cached SIF image
    PID TTY          TIME CMD
1064804 pts/10   00:00:00 starter
1064846 pts/10   00:00:00 bash
1064871 pts/10   00:00:00 squashfuse_ll
1064929 pts/10   00:00:00 bash
1064931 pts/10   00:00:00 ps

This branch was successfully deployed

1 active deployment
github-pages 09a75b37 Deployed Sep 20, 2026 by veprbl via deploy-artifacts-page #4152
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.

3 participants