This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Run `chmod +x` on executables when testing via SSH
ClosedPublic

Authored by broadwaylamb on Oct 18 2019, 8:10 AM.

Details

Summary

When running libc++ tests on a remote machine via SSH, we can encounter
a 'Permission denied' error.

Fix this with plain old 'chmod +x <executable>'.

Diff Detail

Event Timeline

broadwaylamb created this revision.Oct 18 2019, 8:10 AM
Herald added a project: Restricted Project. · View Herald Transcript

Execute chmod +x without creating a separate connection

ldionne added inline comments.Oct 18 2019, 10:33 AM
libcxx/utils/libcxx/test/executor.py
139

Do I understand correctly: the problem is that files are created on Windows and then scp'd to Linux, but there is no executable bit set on Windows so the files arrive on Linux as not executable?

142

You're assuming that we have chmod on the system, but what if we're instead remotely executing binaries compiled on Linux for Windows (not sure that's even a thing, but you get the point)?

ldionne requested changes to this revision.Oct 18 2019, 10:33 AM

Requesting changes so it shows up properly in my review queue.

This revision now requires changes to proceed.Oct 18 2019, 10:33 AM
broadwaylamb marked 2 inline comments as done.Oct 18 2019, 11:10 AM
broadwaylamb added inline comments.
libcxx/utils/libcxx/test/executor.py
139

Yes, exactly.

142

In D68275 I made Executor depend on the target info object, so we can ask if the target system is not Windows, and only then set the executable bit.

But if there are better solutions, I'm ready to investigate them.

broadwaylamb marked an inline comment as done.Oct 24 2019, 8:07 AM
broadwaylamb added inline comments.
libcxx/utils/libcxx/test/executor.py
147–149

Found a bug in this line: in the _execute_command_remote method we actually prepend the env command to set the execution environment. However, because of this patch, the env command is only applied to chmod.

I'm going to send a fix.

Use export instead of env for setting execution environment in SSHExecutor.

This is safe, because the exported environment can't outlive the SSH session, which itself is alive only within the _execute_command_remote method.

ldionne accepted this revision.Oct 24 2019, 3:43 PM

LGTM with the requested change.

libcxx/utils/libcxx/test/executor.py
203

This seems clearer to me:

if export_cmd:
  remote_cmd = ' '.join(export_cmd) + ' && ' + remote_cmd
This revision is now accepted and ready to land.Oct 24 2019, 3:43 PM

Use a simpler condition syntax in executor.py

broadwaylamb marked 2 inline comments as done.Oct 25 2019, 7:05 AM

@ldionne if everything is good, can we land this?

libcxx/utils/libcxx/test/executor.py
203

Oh. Of course. Updated accordingly.

broadwaylamb marked an inline comment as done.Oct 25 2019, 7:06 AM
This revision was automatically updated to reflect the committed changes.