Skip to content

Conversation

@Micket
Copy link
Contributor

@Micket Micket commented Mar 19, 2024

fixes #970

As discussed, I think this would be a greatly useful feature for anyone struggling to

Rather than directly dropping users into the shell when things fail, just writing these temp files allow some advantages, primarily:

  1. No need to break the flow in EB, so no need to limit this to tty sessions only or introducing any options, we can just always do this since it's cheap.
  2. Allows users to revisit any of the commands (want to experiment with the configure step before going back to attempt another "make -j"? trivial with this approach)

Maybe we want to be able to limit when we generate this? Maybe, we do run a lot of shell cmds so i guess many of these are not worth ever really revisiting.

This is completely untested yet, just my somewhat formed ideas.

@Micket Micket marked this pull request as draft March 19, 2024 23:44
@Micket Micket added the EasyBuild-5.0 EasyBuild 5.0 label Mar 19, 2024
@Micket Micket added this to the 5.0 milestone Mar 19, 2024
@Micket Micket force-pushed the cmdlog branch 3 times, most recently from 5c65182 to 50b98c5 Compare April 2, 2024 22:22
@Micket Micket marked this pull request as ready for review April 2, 2024 22:36
@Micket
Copy link
Contributor Author

Micket commented Apr 2, 2024

I think this is good. I haven't actually tested using it yet (need to set up EB5 testing environment)

@Micket
Copy link
Contributor Author

Micket commented Apr 7, 2024

I had to make some adjustments due to how exported variables, history and rcfiles work with bash. But it was all for the better.

Now it generates a big env.sh file with the entire environment (exported)

export CC=gcc
export CXX=g++
export LMOD_CMD=/usr/share/lmod/lmod/libexec/lmod
...

and a cmd.sh file that sources this environment into an interactive shell + change directory + helpful text + command history + custom prompt

#!/usr/bin/env bash

cd "/dev/shm/Bison/3.8.2/system-system/bison-3.8.2"
EB_SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
echo Shell for the command: './configure --prefix=/apps/Test2/software/Bison/3.8.2  --build=x86_64-pc-linux-gnu  --host=x86_64-pc-linux-gnu'
echo Use command history, exit to stop
bash --rcfile <(cat $EB_SCRIPT_DIR/env.sh; echo 'PS1="eb-shell> "'; echo history -s './configure --prefix=/apps/Test2/software/Bison/3.8.2  --build=x86_64-pc-linux-gnu  --host=x86_64-pc-linux-gnu')

I've actually tested so that it works and i already love it

@Micket
Copy link
Contributor Author

Micket commented Apr 10, 2024

All that remains here is how to display the existence of these additional files to the user.

Currently, one would see:

  >> running shell command:
        ./configure --prefix=/apps/Test2/software/Bison/3.8.2  --build=x86_64-pc-linux-gnu  --host=x86_64-pc-linux-gnu
        [started at: 2024-04-07 19:48:51]
        [working dir: /dev/shm/Bison/3.8.2/system-system/bison-3.8.2]
        [output saved to /dev/shm/eb-08gn7ssh/run-shell-cmd-output/configure-rdh8fxpj/out.txt]

and I could add more lines like

        [environment saved to /dev/shm/eb-08gn7ssh/run-shell-cmd-output/configure-rdh8fxpj/env.sh]
        [command environment saved to /dev/shm/eb-08gn7ssh/run-shell-cmd-output/configure-rdh8fxpj/cmd.sh]

or we could consider just pointing to the directory:

        [environment and output in: /dev/shm/eb-08gn7ssh/run-shell-cmd/configure-rdh8fxpj/]

@Micket
Copy link
Contributor Author

Micket commented Apr 11, 2024

I moved my PS1 and history to the cmd.env file instead, allowing it to be used as an rcfile directly.

This simplifies the code but more importantly also avoid trouble with echo removing quotes around strings, so, this is now robust against quotes and spaces in the cmdstr.

@boegel boegel changed the title Log shell cmds to temp file which allow users to revisit them create env.sh and cmd.sh helper scripts in run_shell_cmd May 30, 2024
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

@Micket Please take a look at the suggestions in Micket#3

…l_cmd + enhance test to verify they're working as intended
Micket and others added 2 commits May 30, 2024 14:29
minor tweaks to dumping of `env.sh` + `run.sh` helper scripts in `run_shell_cmd` + enhance test to verify they're working as intended
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

@Micket small fix in case custom env is used in run_shell_cmd (which should be very rare, but may happen):

boegel and others added 2 commits May 31, 2024 09:13
fix handling of specified command environment in `create_cmd_scripts` + add test for env option in `run_shell_cmd`
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

I have some ideas to improve this further, but those can be implemented in a follow-up PR

@boegel boegel merged commit a032ebf into easybuilders:5.0.x Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants