Summary: | [JSC] Set ssh keepalive in run-jsc-stress-tests | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Angelos Oikonomopoulos <angelos> | ||||||
Component: | New Bugs | Assignee: | Angelos Oikonomopoulos <angelos> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aperez, Hironori.Fujii, jbedard, webkit-bug-importer, ysuzuki | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Angelos Oikonomopoulos
2022-02-22 03:50:43 PST
Created attachment 452852 [details]
Patch
Stopped https://ews-build.webkit.org/#/builders/70/builds/1022, the 3 failures it flagged are already known. Comment on attachment 452852 [details] Patch The new option added and moving the always used SSH options into a common array LGTM, but there is one cleanup I would like to see done as part of this patch =) View in context: https://bugs.webkit.org/attachment.cgi?id=452852&action=review > Tools/Scripts/run-jsc-stress-tests:2560 > + IO.popen("ssh #{SSH_OPTIONS_DEFAULT.join(" ")} -p #{remoteHost.port}" + (remoteHost.identity_file_path ? " -i #{remoteHost.identity_file_path}" : "") + " #{remoteHost.user}@#{remoteHost.host} '#{cmd}'", "r") { While doing modifications here, could we change this to pass the array of command+arguments to IO.popen instead of using a command string that will be passed through the shell? The “ssh” invocation itself does not need to go through the local shell — the remote command (“cmd” here) will still run in a shell remotely because that's how SSH behaves. Created attachment 452961 [details]
Patch
(In reply to Adrian Perez from comment #3) > While doing modifications here, could we change this to pass the array of > command+arguments to IO.popen instead of using a command string that will > be passed through the shell? The “ssh” invocation itself does not need to > go through the local shell — the remote command (“cmd” here) will still > run in a shell remotely because that's how SSH behaves. Sure thing! While here I also changed the next snippet to not open-code a read loop. Comment on attachment 452961 [details] Patch r=me, thanks! View in context: https://bugs.webkit.org/attachment.cgi?id=452961&action=review > Tools/Scripts/run-jsc-stress-tests:2566 > + result = inp.read Not sure if placebo effect, but this does look more readable to me, as a bonus :-} Committed r290369 (247685@main): <https://commits.webkit.org/247685@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452961 [details]. |