-
Notifications
You must be signed in to change notification settings - Fork 93
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
[JENKINS-55946] always force an interpreter #89
base: master
Are you sure you want to change the base?
Conversation
Pipeline jobs including an sh step are including a shebang on the underlying script. Such shebang caused the script to be mode changed to executable. These "executables", when written on a mounted filesystem with the noexec flag, become non-executable (causing a not obvious failure). Replacing the bare executable script with a call to the runner defined at the shebang, avoids this situation
Thanks for the PR @witokondoria! We try to track nontrivial changes through Jira issues, so would you be able to open an issue on https://issues.jenkins-ci.org/secure/Dashboard.jspa with component Is the scenario you mention something that can be reproduced in a test using a Docker container? For example, we previously had errors on Alpine, and so added some Alpine-specific tests (e.g. runOnAlpineDocker uses this Dockerfile so we don't accidentally regress the behavior). Given that seemingly minor changes in this plugin can have surprising effects on some platforms, it is best to add tests whenever possible to avoid regressions. |
src/main/java/org/jenkinsci/plugins/durabletask/BourneShellScript.java
Outdated
Show resolved
Hide resolved
I have created the requested ticket (and assigned to you)
tmp/noexecmount would be the workspace folder while hopefully, the bind mount wont fiddle with the noexec flag |
The CI failure is because |
FWIW I was a little concerned that shebang lines like |
Maybe, but should be investigated before merging. |
Sounds like that should be made into a test case, since it is not immediately obvious from reading the code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is safe.
Will try to include such tests |
@witokondoria Do you think you will have any time to look into adding some tests? |
See JENKINS-55946.
Pipeline jobs including an sh step are including a shebang on the underlying script. Such shebang caused the script to be mode changed to executable. These "executables", when written on a mounted filesystem with the noexec flag, become non-executable (causing a not obvious failure).
Replacing the bare executable script with a call to the runner defined at the shebang, avoids this situation