Skip to content
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

Jump to bottom

Shell script wrappers #586

Merged
merged 10 commits into from
Sep 5, 2022
Merged

Shell script wrappers #586

merged 10 commits into from
Sep 5, 2022

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Aug 31, 2022

This will close #565

@vsoch
proof of concept for shell wrapper!
4ae2e0b 
I still need to do the container command, exec, run, and the two inspects,
and then the same for docker. This is a WIP - the proof of concept seems to
be okay so far!

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
adding reminder of singularity wrappers
e5d90cb 
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
adding docker wrapper scripts and tests
156fcb9 
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
formatting
056aef4 
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
singularity exec/run should not have -s shell
2e68e13 
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch vsoch mentioned this pull request Aug 31, 2022
Closed
@muffato
Copy link
Contributor

muffato commented Sep 4, 2022

Hi. I'm facing an issue when testing this. For a certain software, I've defined a custom wrapper script for one of its aliases. Very much like vanessa/salad except that it doesn't work :/
Somehow, the file that is being created has a single line, which contains the full path (with no line return) of the template it was supposed to load and substitute. Any idea what could be causing that ? I can't trivially share it because it's not an open-source software, and one has to sign the licence agreement and build their own container.

@muffato
Copy link
Contributor

muffato commented Sep 4, 2022

Oh, the issue is also on the main branch ! it must come from one of the recent PRs, then

@vsoch
Copy link
Member Author

vsoch commented Sep 4, 2022

Can you give me the command to reproduce?

@vsoch
Copy link
Member Author

vsoch commented Sep 4, 2022

I have a feeling a script name is being provided as the variable instead of the template somewhere. If I can reproduce I can get it into this PR and fix asap!

@muffato muffato mentioned this pull request Sep 4, 2022
Closed
@muffato
Copy link
Contributor

muffato commented Sep 4, 2022

I could clearly reproduce the issue on vanessa/salad, cf #588

@vsoch
Copy link
Member Author

vsoch commented Sep 4, 2022

Taking a look now.

@vsoch
ensure template is fully loaded
8029a6a 
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
fixing bug that filesystem wrapper returns the filename instead of th…
41a3991 
…e content!

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
update test: load_wrapper_script should have equivalent behavior rega…
d4395c9 
…rdless!

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
muffato
muffato approved these changes Sep 5, 2022
View reviewed changes
Copy link
Contributor

@muffato muffato left a comment

Choose a reason for hiding this comment

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

set_shell_function is a Lua thing, right ? I can't see any reference to it in the rest of docker.tcl and singularity.tcl

shpc/main/modules/templates/singularity.tcl Outdated Show resolved Hide resolved
shpc/main/modules/templates/docker.tcl Outdated Show resolved Hide resolved
vsoch and others added 2 commits September 4, 2022 19:11
@vsoch @muffato
Update shpc/main/modules/templates/singularity.tcl
ce4b232 
Co-authored-by: Matthieu Muffato <mm49@sanger.ac.uk>
@vsoch @muffato
Update shpc/main/modules/templates/docker.tcl
77a50aa 
Co-authored-by: Matthieu Muffato <mm49@sanger.ac.uk>
@vsoch vsoch merged commit 3b164e0 into main Sep 5, 2022
@vsoch vsoch deleted the shell-script-wrappers branch September 5, 2022 02:31
@muffato muffato mentioned this pull request Sep 8, 2022
Open
@vsoch vsoch mentioned this pull request Feb 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muffato muffato muffato approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generating shell scripts instead of functions for wrappers without specifying on the container.yaml
2 participants
@vsoch @muffato