Split embedded eic-shell heredocs into template files, assembled via Jinja2#27
Split embedded eic-shell heredocs into template files, assembled via Jinja2#27
Conversation
…I publish workflow Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the install.sh launcher generation by moving the embedded Singularity/Docker eic-shell scripts into dedicated template files and using envsubst for substitution, with a CI workflow to publish a self-contained assembled installer for curl | bash usage.
Changes:
- Extract
eic-shelllaunchers intoeic-shell.singularityandeic-shell.dockertemplates, generated viaenvsubstfrominstall.sh. - Add
assemble.pyto inline templates back into a standaloneinstall.shfor distribution. - Add a GitHub Pages publish workflow that assembles and deploys
install.shon pushes tomain.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| install.sh | Uses envsubst + $SCRIPT_DIR templates to generate the eic-shell launcher. |
| eic-shell.singularity | New Singularity launcher template to be substituted at install time. |
| eic-shell.docker | New Docker launcher template (including the macOS X11 logic) to be substituted at install time. |
| assemble.py | Generates a standalone install.sh by replacing template reads with inline heredocs. |
| .github/workflows/publish.yml | Builds and deploys the assembled install.sh to GitHub Pages. |
| .gitignore | Ignores Python __pycache__/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
eic-shell.singularity
Outdated
| if [ ! -z ${UPGRADE} ]; then | ||
| echo "Upgrading eic-shell..." | ||
| if [ -z "$DISABLE_CVMFS_USAGE" -a -d /cvmfs/singularity.opensciencegrid.org/${ORGANIZATION}/${CONTAINER}:${VERSION} ]; then | ||
| echo "" | ||
| echo "Note: You cannot manually update the container as you are using the CVMFS version." |
eic-shell.singularity
Outdated
|
|
||
| export EIC_SHELL_PREFIX=${EICS_PREFIX}/local | ||
| export SINGULARITY_BINDPATH=${EICS_BINDPATH} | ||
| ${SINGULARITY:-${EICS_SINGULARITY}} exec ${SINGULARITY_OPTIONS:-} ${SIF:-${EICS_SIF}} eic-shell $@ |
eic-shell.docker
Outdated
| if [ ! -z ${UPGRADE} ]; then | ||
| echo "Upgrading eic-shell..." | ||
| docker pull ${EICS_IMG} || exit 1 | ||
| echo "eic-shell upgrade successful" | ||
| exit 0 |
eic-shell.docker
Outdated
| dispnum=`ps -e |grep Xquartz | grep listen | grep -v xinit |awk '{print $5}'` | ||
| XSTUFF="-e DISPLAY=host.docker.internal${dispnum} -v /tmp/.X11-unix:/tmp/.X11-unix" | ||
| fi | ||
| docker run ${EICS_PLATFORM_FLAG} ${EICS_MOUNT} $XSTUFF -w=$PWD -it --rm -e EIC_SHELL_PREFIX=${EICS_PREFIX}/local ${EICS_IMG} eic-shell $@ |
install.sh
Outdated
| envsubst '${EICS_ORGANIZATION} ${EICS_CONTAINER} ${EICS_TMPDIR} ${EICS_VERSION} ${EICS_PREFIX} ${EICS_DISABLE_CVMFS_USAGE} ${EICS_SINGULARITY} ${EICS_BINDPATH} ${EICS_SIF}' < "$SCRIPT_DIR/eic-shell.singularity" > eic-shell | ||
|
|
eic-shell.singularity
Outdated
| if [ ! -z ${DISABLE_CVMFS_USAGE} ]; then | ||
| FLAGS="${FLAGS} --no-cvmfs" | ||
| fi | ||
| curl -L https://github.com/eic/eic-shell/raw/main/install.sh \ |
| echo " -u,--upgrade Upgrade the container to the latest version" | ||
| echo " -n,--no-cvmfs Disable check for local CVMFS when updating. (D: enabled)" | ||
| echo " -o,--organization Organization (D: $ORGANIZATION) (requires cvmfs)" | ||
| echo " -c,--container Container family (D: $CONTAINER) (requires cvmfs)" | ||
| echo " -v,--version Version to install (D: $VERSION) (requires cvmfs)" |
eic-shell.docker
Outdated
| if [ "$(uname -s)" = 'Darwin' ] && [ ! ${NOX} ]; then | ||
| nolisten=`defaults find nolisten_tcp | grep nolisten | head -n 1 | awk '{print $3}'|cut -b 1 ` | ||
| [[ $nolisten -ne 0 ]] && echo "For X support: In XQuartz settings --> Security --> enable \"Allow connections from network clients\" and restart (should be only once)." | ||
| xhost +localhost | ||
| dispnum=`ps -e |grep Xquartz | grep listen | grep -v xinit |awk '{print $5}'` |
| install_sh = re.sub( | ||
| r'(?P<indent>[ \t]*)envsubst \'(?P<variables>[^\']+)\'' | ||
| r' < "\$SCRIPT_DIR/(?P<template>[^"]+)" > (?P<output>\S+)', | ||
| inline_template, | ||
| install_sh, | ||
| ) | ||
|
|
install.sh
Outdated
| export EICS_ORGANIZATION="$ORGANIZATION" EICS_CONTAINER="$CONTAINER" EICS_TMPDIR="$TMPDIR" \ | ||
| EICS_VERSION="$VERSION" EICS_PREFIX="$PREFIX" EICS_DISABLE_CVMFS_USAGE="${DISABLE_CVMFS_USAGE}" \ | ||
| EICS_SINGULARITY="$SINGULARITY" EICS_BINDPATH="$BINDPATH" EICS_SIF="$SIF" | ||
| envsubst '${EICS_ORGANIZATION} ${EICS_CONTAINER} ${EICS_TMPDIR} ${EICS_VERSION} ${EICS_PREFIX} ${EICS_DISABLE_CVMFS_USAGE} ${EICS_SINGULARITY} ${EICS_BINDPATH} ${EICS_SIF}' < "$SCRIPT_DIR/eic-shell.singularity" > eic-shell |
❌ Deploy Preview for kaleidoscopic-syrniki-0cfb8a failed.
|
.github/workflows/publish.yml
Outdated
There was a problem hiding this comment.
@copilot Target branch calls this file deploy.yml. Instead of creating a new file, integrate into the target branch's deploy.yml workflow.
There was a problem hiding this comment.
Done in 42c57e1 — integrated the "Assemble standalone install.sh" step into the existing deploy.yml and removed publish.yml.
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
wdconinc
left a comment
There was a problem hiding this comment.
@copilot I don't like the envsubst replacement in assembly.py; it's hacky. Consider if a templating engine like jinja2 would be better here. The target is two goals:
- a way to run the install.sh script locally for testing,
- a way to provide a combined install.sh script that does not depend on external files.
The goals would be best accomplished by allowing the embedded shell scripts to be treated as shell scripts in the repository, not as embedded heredocs (the main motivation for this change).
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Done in c199be7. Here's what changed: Template files are now proper shell scripts —
|
install.shembedded two large heredocs (~180 lines total) for generating the Singularity and Dockereic-shelllaunchers inline, making the script bloated and the templates hard to maintain independently.Changes
Template files
eic-shell.singularity— Singularity launcher extracted frominstall_singularity()eic-shell.docker— Docker launcher extracted frominstall_docker(), including the macOS X11 block as natural shell code gated by a runtime$(uname -s)checkBoth files are proper standalone shell scripts using
${VAR:=default}bash defaults for all install-time variables, so they can be run or linted directly (set the relevant env vars to override the defaults for testing). There are no${EICS_*}pseudo-variables.Launcher generation in
install.shInstall-time values are baked into the generated
eic-shellscript in two steps:Assembly for distribution
assemble.py— Usesjinja2.Environment(loader=FileSystemLoader(...))to load each template file cleanly, then replaces thetail -n +2 "$SCRIPT_DIR/TEMPLATE" >> OUTPUTcalls with equivalent inline heredocs, producing a self-containedinstall.shsuitable forcurl … | bashdistribution. A post-assembly validation raises an error if any$SCRIPT_DIR/reference survives.CI deploy workflow
.github/workflows/deploy.yml— Extended (from the existing workflow) to installjinja2and runassemble.pybefore building the Jekyll site, so the assembled standaloneinstall.shis published to GitHub Pages on every push tomain.Original prompt
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.