[JSC] Set ssh keepalive in run-jsc-stress-tests
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].
<rdar://problem/89351769>