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

SSHManager with csh shell #32690

Closed
DcfGH opened this issue Jul 26, 2019 · 3 comments
Closed

SSHManager with csh shell #32690

DcfGH opened this issue Jul 26, 2019 · 3 comments
Labels
parallelism Parallel or distributed computation

Comments

@DcfGH
Copy link

DcfGH commented Jul 26, 2019 •
edited by mbauman
Loading

Dear Julia team,

Adding workers using SSHManager (so workers are not local) gives me troubles if user default shell is not bash. Using ssh with multi lines command returns an error in csh (need a back slash for multi lines)

I tried a small change in stdlib/Distributed/src/managers.jl
from (lines 187 up to 190)

    cmds = """
        cd -- $(shell_escape_posixly(dir))
        $(isempty(tval) ? "" : "export JULIA_WORKER_TIMEOUT=$(shell_escape_posixly(tval))")
        $(shell_escape_posixly(exename)) $(shell_escape_posixly(exeflags))"""
to (one line with 2 commands separated by ";")
    cmds = """cd -- $(shell_escape_posixly(dir)) ; $(isempty(tval) ? "" : "export JULIA_WORKER_TIMEOUT=$(shell_escape_posixly(tval))") $(shell_escape_posixly(exename)) $(shell_escape_posixly(exeflags))"""

It seems to work in both cases : csh and bash.

Best regards
@mbauman mbauman added the parallelism Parallel or distributed computation label Jul 26, 2019
@vchuravy vchuravy changed the title Distributed.addprocs SSHManager with csh shell Jul 29, 2019
@StefanKarpinski
Copy link
Sponsor Member

Could you make a pull request with that change?

@DcfGH
Copy link
Author

DcfGH commented Aug 2, 2019

Hi,
Pull request is 32765
2 tests are not successful : buildbot/tester_macos64 and buildbot/tester_win64
I hope problems do not come from this change
Best regards

@FelipeLema FelipeLema mentioned this issue Sep 8, 2020
Closed
@FelipeLema FelipeLema mentioned this issue Sep 24, 2020
Closed
mgkuhn added a commit to mgkuhn/julia that referenced this issue Jul 6, 2021
@mgkuhn
Distributed: add C shell support (fixes JuliaLang#32690)
1748fc7 
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

mgkuhn commented Jul 6, 2021 •
edited
Loading

The change proposed here does not solve the problem that C shells and POSIX shells have different syntax and mutually incompatible quoting rules. The workaround suggested above will break if the strings passed on contain shell meta-characters, such as single or double quotes. PR #41485 adds instead proper C shell support to SSHManager via option shell=:csh.

mgkuhn added a commit to mgkuhn/julia that referenced this issue Jul 7, 2021
@mgkuhn
Distributed: add C shell support (fixes JuliaLang#32690)
ae3fb5c 
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.
Keno pushed a commit that referenced this issue Jun 5, 2024
@mgkuhn
Distributed: add C shell support (fixes #32690) (#41485)
5019ded 
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
Assignees
No one assigned
Labels
parallelism Parallel or distributed computation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@StefanKarpinski @mbauman @mgkuhn @DcfGH