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.

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

job.sh: run as bash only #3093

Merged
merged 2 commits into from
Apr 9, 2019
Merged

Conversation

oliver-sanders
Copy link
Member

Follow-up to #3088, use bash shebang for the job.sh file.

@oliver-sanders oliver-sanders added this to the cylc-8.0a1 milestone Apr 8, 2019
@oliver-sanders oliver-sanders self-assigned this Apr 8, 2019
@oliver-sanders oliver-sanders requested a review from hjoliver April 8, 2019 09:10
@dwsutherland
Copy link
Member

Guess I can remove that “shell” info from the job fields in the UI feed..

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Apr 8, 2019

Ooh, good point! See 881ad17.

Copy link
Member

@dwsutherland dwsutherland left a comment

Choose a reason for hiding this comment

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

LGTM at a glance 😁

Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

Change looks good!

Should we update the docs too?

diff --git a/doc/src/appendices/suiterc-config-ref.rst b/doc/src/appendices/suiterc-config-ref.rst
index 91385e60a..58d31d61f 100644
--- a/doc/src/appendices/suiterc-config-ref.rst
+++ b/doc/src/appendices/suiterc-config-ref.rst
@@ -1572,18 +1572,6 @@ job file path.
 - *example*: ``llsubmit \%(job)s``
 
 
-.. _JobSubShell:
-
-[runtime] ``->`` [[\_\_NAME\_\_]] ``->`` [[[job]]] ``->`` shell
-'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-Location of the bash shell invoked on job hosts to interpret task job scripts,
-if not installed in the standard location.
-
-- *type*: string
-- *root default*: ``/bin/bash``
-
-
 .. _JobSubRefRetries:
 
 [runtime] ``->`` [[\_\_NAME\_\_]] ``->`` [[[job]]] ``->`` submission retry delays
@@ -2015,7 +2003,7 @@ parser is concerned these are just normal configuration items).
 - *type*: string
 - *default*: (none)
 - *legal values*: depends to some extent on the task job
-  submission shell (:ref:`JobSubShell`).
+  submission shell
 - *examples*, for the bash shell:
 
   - ``FOO = $HOME/bar/baz``

@kinow
Copy link
Member

kinow commented Apr 8, 2019

Just kicked Travis to see if the build passes now 👍

@hjoliver
Copy link
Member

hjoliver commented Apr 9, 2019

Should we update the docs too? ...

So I see @oliver-sanders latest commit removes the remaining ability to configure the location of the bash interpreter, if 'not in the standard location' (left in when I removed ksh support a few days ago) ... in which case, yes the associated user-guide docs should be removed too.

So long as we can rely on bash always being located at /bin/bash. Presumably we can, in which case that's fine ... but just checking!

@hjoliver
Copy link
Member

hjoliver commented Apr 9, 2019

Note genuine test failure due to removal of the shell config item.

@oliver-sanders
Copy link
Member Author

BTW: Happy to revert the last commit if desired.

So long as we can rely on bash always being located at /bin/bash. Presumably we can, in which case that's fine ... but just checking!

It hadn't occurred to me that that might be a problem. I'm not sure why this would happen, installing multiple bash versions?

Should we update the docs too?

Yes! About time we auto-generated this...

@matthewrmshin matthewrmshin merged commit d2e649b into cylc:master Apr 9, 2019
@oliver-sanders oliver-sanders deleted the shellcheck-job.sh branch April 9, 2019 13:57
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.

5 participants