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

Explicitly run perl for OpenSSL Configure #44131

Merged
merged 1 commit into from
Sep 15, 2017
Merged

Conversation

smaeul
Copy link
Contributor

@smaeul smaeul commented Aug 28, 2017

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@Mark-Simulacrum
Copy link
Member

We'd want to also update the sanity checks and add perl to the configuration if we're going to do this. Sanity checks are here: https://github.com/rust-lang/rust/blob/master/src/bootstrap/sanity.rs#L124 and configuration will need to be updated both in the loader (look at how nodejs is done) and config.toml.example in the root will also need to be updated.

r? @Mark-Simulacrum

@arielb1 arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 29, 2017
@smaeul
Copy link
Contributor Author

smaeul commented Sep 2, 2017

I'm not sure why any of that is necessary (although it might be nice to have). The status quo is that perl is not configurable, and nobody has complained about that. This does not change which perl is called. This is just replicating what the existing code does already at the beginning of the Configure script:

:
eval 'exec perl -S $0 ${1+"$@"}'
    if $running_under_some_shell;

The only reason this change is necessary at all is because the script is designed for decades-old UNIX systems without #! (from man perlrun, under -S):

To start up sh rather than csh, some systems may have to replace the "#!" line with a line containing just a colon, which will be politely ignored by Perl.

-S is removed because it was already a no-op, as we ran script by its full path.

If you want, I can replace Command::new("perl"); with Command::new("sh");, since the file is just as much a "shell script" as "Perl script", but I thought it would be better to avoid the gratuitous indirection.

@Mark-Simulacrum
Copy link
Member

Sure, if you don't want to implement that I'm happy to r+ now. Could you file an issue for the steps I propose though? It'd be a nice to have, at least.

@aidanhs
Copy link
Member

aidanhs commented Sep 7, 2017

@bors r=Mark-Simulacrum rollup

@bors
Copy link
Contributor

bors commented Sep 7, 2017

📌 Commit adfebed has been approved by Mark-Simulacrum

@aidanhs aidanhs added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 7, 2017
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 8, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 9, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 9, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 11, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 11, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 13, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 14, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 15, 2017
Explicitly run perl for OpenSSL Configure

OpenSSL's Configure script is missing a shebang. On some platforms,
execve falls back to execution with the shell. Some other platforms,
like musl, will fail with an exec format error. Avoid this by calling
perl explicitly (since it's a perl script).
bors added a commit that referenced this pull request Sep 15, 2017
@bors bors merged commit adfebed into rust-lang:master Sep 15, 2017
@smaeul smaeul deleted the openssl-perl branch October 17, 2017 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants