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

Distributed: add C shell support (fixes #32690) #41485

Merged
merged 1 commit into from
Jul 9, 2021

Conversation

mgkuhn
Copy link
Contributor

@mgkuhn mgkuhn commented Jul 6, 2021

The Unix C Shell (csh, tcsh) remains in common use as a login shell in some communities. It is not compatible with POSIX shells (bash, dash, ksh, zsh, etc.) and requires different syntax and escaping. This patch adds the function Base.shell_escape_csh(), a C shell equivalent of Base.shell_escape_posixly(), along with a new option :csh for the shell argument of Distributed.addprocs(), for use if the login shell on worker accounts is a C shell.

closes #32765
closes #37501
closes #41285

@mgkuhn mgkuhn requested a review from vtjnash July 6, 2021 18:35
@mgkuhn mgkuhn added the parallelism Parallel or distributed computation label Jul 6, 2021
base/shell.jl Outdated Show resolved Hide resolved
base/shell.jl Outdated Show resolved Hide resolved
test/spawn.jl Outdated Show resolved Hide resolved
The Unix C Shell (csh, tcsh) remains in common use as a login shell in
some communities. It is not compatible with POSIX shells (bash, dash,
ksh, zsh, etc.) and requires different syntax and escaping. This patch
adds the function `Base.shell_escape_csh()`, a C shell equivalent of
`Base.shell_escape_posixly()`, along with a new option `:csh` for the
`shell` argument of `Distributed.addprocs()`, for use if the login
shell on worker accounts is a C shell.
@mgkuhn
Copy link
Contributor Author

mgkuhn commented Jul 7, 2021

@JeffBezanson Thanks, I've now applied all your suggestions.

@JeffBezanson JeffBezanson merged commit 156b0be into JuliaLang:master Jul 9, 2021
@fredrikekre
Copy link
Member

Seems NEWS-worthy?

@fredrikekre fredrikekre added needs compat annotation Add !!! compat "Julia x.y" to the docstring needs news A NEWS entry is required for this change labels Jul 10, 2021
@mgkuhn
Copy link
Contributor Author

mgkuhn commented Jul 10, 2021

@fredrikekre Yes. I'm planning a few more SSHManager related PRs in the coming days (e.g., tidy up shell=:posix to not call sh, IPv6-only support, possibly even shell=:powershell support – once I have grokked their slightly baffling syntax spec), and will then roll that NEWS+compat item into one of these. (The problem with including a NEWS update into a PR early on is that these often cause merge conflicts quite quickly, so I've become a bit reluctant to do it from the beginning.)

@mgkuhn mgkuhn removed the needs news A NEWS entry is required for this change label Jul 28, 2022
@mgkuhn mgkuhn deleted the csh branch July 28, 2022 10:27
Keno pushed a commit that referenced this pull request Jun 5, 2024
The Unix C Shell (csh, tcsh) remains in common use as a login shell in
some communities. It is not compatible with POSIX shells (bash, dash,
ksh, zsh, etc.) and requires different syntax and escaping. This patch
adds the function `Base.shell_escape_csh()`, a C shell equivalent of
`Base.shell_escape_posixly()`, along with a new option `:csh` for the
`shell` argument of `Distributed.addprocs()`, for use if the login
shell on worker accounts is a C shell.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs compat annotation Add !!! compat "Julia x.y" to the docstring parallelism Parallel or distributed computation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants