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

Issue 32690 : SSHManager with csh shell #32765

Closed
wants to merge 1 commit into from
Closed

Conversation

DcfGH
Copy link

@DcfGH DcfGH commented Aug 2, 2019

Dear Julia team,

A pull request from issue 32690
Best regards

@kshyatt kshyatt added the parallelism Parallel or distributed computation label Oct 15, 2019
@kshyatt
Copy link
Contributor

kshyatt commented Oct 15, 2019

@amitmurthy do you want to review this?

@ViralBShah
Copy link
Member

@tanmaykm Can you review and merge this? I don't fully understand the intricacies of csh, but it looks like a small change.

cd -- $(shell_escape_posixly(dir))
$(isempty(tval) ? "" : "export JULIA_WORKER_TIMEOUT=$(shell_escape_posixly(tval))")
$(shell_escape_posixly(exename)) $(shell_escape_posixly(exeflags))"""
cmds = """cd -- $(shell_escape_posixly(dir)) ; $(isempty(tval) ? "" : "export JULIA_WORKER_TIMEOUT=$(shell_escape_posixly(tval))") $(shell_escape_posixly(exename)) $(shell_escape_posixly(exeflags))"""
Copy link
Member

Choose a reason for hiding this comment

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

If multiple lines were merged, shouldn't there be a command separator after all the lines? I don't see one after the second line.

@tanmaykm
Copy link
Member

Since this is quite old, it will be good to rebase this once with master.

We probably do not run our CI tests with csh. So we may be only testing for regressions here and not the actual fix. Is there any way to do that?

@FelipeLema
Copy link

fixes #32690

@FelipeLema
Copy link

FelipeLema commented Sep 8, 2020

@tanmaykm you can do something like sudo usermod --shell=/usr/bin/csh travis (asuming I got the username right).

However, that will make all shells spawned with csh instead of default bash, which is likely (?) to have consequences midway through the test run.

Is there a proper place where we can set-and-unset csh as shell in .travis.yml? I was thinking "before remote process tests", is this possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parallelism Parallel or distributed computation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants