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

Update rustup-init to detect tls1.2 for curl 7.73+ (Fixes #2603) #2604

Merged
merged 1 commit into from
Dec 13, 2020

Conversation

apnorton
Copy link
Contributor

This fixes #2603. In curl 7.73.0, the help format was changed so as to only show a summary when calling curl --help, while the old menu can be accessed as curl --help all. This change updates the check_help_for function to detect if the command being checked contains the curl 7.73.0+ language indicating the use of --help all.

If this language is present, check_help_for will insert the all category after the --help flag, allowing the query to work. If the language is not present, then it will insert an empty string following the --help flag, maintaining the same behavior with pre-7.73 versions of curl.

Side Note: This is my first contribution to the rustup repository and I've tried to follow the contribution guidelines to the best of my ability; however, it seems that much of that document is relevant mainly for actual code changes to rustup itself, not to the init script. I still ran the test suite just in case one of those tests covered the init script somehow. Please let me know if there's anything else I need to do for this contribution.

This fixes rust-lang#2603.  In curl 7.73.0, the help format was changed so as to
only show a summary when calling `curl --help`, while the old menu can
be accessed as `curl --help all`.  This change updates the
`check_help_for` function to detect if the command being checked
contains the curl 7.73.0+ language indicating the use of `--help all`.

If this language is present, `check_help_for` will insert the `all`
category after the `--help` flag, allowing the query to work.  If the
language is not present, then it will insert an empty string following
the `--help` flag, maintaining the same behavior with pre-7.73 versions
of curl.
@kinnison
Copy link
Contributor

We actually use the script in our CI, so it's tested in that way, thank you for trying though.

The change looks sane to me, though obviously we lack anything running that curl version to test it on. I assume you've also tested it against a system which exhibits the problem? if not, could you do so?

@apnorton
Copy link
Contributor Author

I assume you've also tested it against a system which exhibits the problem? if not, could you do so?

I have, yes. I'm currently on a system with curl version 7.73.0 (x86_64-pc-linux-gnu) and have just now uninstalled rustup ($ rustup self uninstall) and reinstalled it from the script in this PR just to be doubly-sure; no warnings were shown and rustup/cargo/rustc are all working fine.

@apnorton
Copy link
Contributor Author

apnorton commented Dec 11, 2020

Since I'm guessing that trusting some rando (me) on the internet saying "works on my machine!" probably isn't a very comfy level of confidence for merging a PR, I went ahead and wrote up a dockerfile so you can try it on your machine (edit: pay no attention to the duplicate apk instruction to install openssl 😬):

FROM alpine:3.7
RUN apk add --no-cache alpine-sdk bash openssl openssl-dev
RUN apk add gcc openssl

RUN curl https://raw.githubusercontent.com/rust-lang/rustup/01fd3a19774e97839f6b3c52075a491ea9479abd/rustup-init.sh -o /tmp/rustup-init.sh  && chmod a+x /tmp/rustup-init.sh

RUN curl https://curl.se/download/curl-7.74.0.tar.gz | tar xz -C /tmp
RUN mkdir /tmp/curlinstall && \
    cd /tmp/curl-7.74.0 && \
    ./configure --with-ssl --prefix /tmp/curlinstall && \
    make && \
    make install

ENV PATH="/tmp/curlinstall/bin:${PATH}"

ENTRYPOINT ["/bin/bash"]

(This takes a hot second to build because it's building the latest curl from source; it also downloads the rustup-init from this PR.)

Then you can build/run/test it with:

$ docker build -t "curl-image" .
$ docker run -it curl-image:latest 
bash-4.4# curl --version
curl 7.74.0 (x86_64-pc-linux-musl) libcurl/7.74.0 OpenSSL/1.0.2t zlib/1.2.11
Release-Date: 2020-12-09
Protocols: dict file ftp ftps gopher http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets
bash-4.4# /tmp/rustup-init.sh 
info: downloading installer

Welcome to Rust!
<SNIP>
bash-4.4# ~/.cargo/bin/cargo --version
cargo 1.48.0 (65cbdd2dc 2020-10-14)

@kinnison
Copy link
Contributor

Thank you for that effort. My only other question is whether this will affect wget based systems? Can you also check that? While I can run docker, I'm not entirely au-fait with making my own so it may be easier for you to do that.

@apnorton
Copy link
Contributor Author

Good check! Here's another dockerfile that doesn't include curl at all, and instead installs wget:

FROM alpine:3.7
RUN apk add --no-cache bash wget openssl openssl-dev gcc libgcc

RUN wget https://raw.githubusercontent.com/rust-lang/rustup/01fd3a19774e97839f6b3c52075a491ea9479abd/rustup-init.sh -O /tmp/rustup-init.sh  && chmod a+x /tmp/rustup-init.sh

ENTRYPOINT ["/bin/bash"]

Installing rustup the same way inside the docker image, but this time with a wget-only system (can also check that the curl isn't found):

$ docker build -t wget-only .
<SNIP>
$ docker run -it wget-only
bash-4.4# curl --version
bash: curl: command not found
bash-4.4# /tmp/rustup-init.sh 
info: downloading installer
--2020-12-12 19:33:37--  https://static.rust-lang.org/rustup/dist/x86_64-unknown-linux-musl/rustup-init
Resolving static.rust-lang.org... 13.32.204.23, 13.32.204.13, 13.32.204.53, ...
Connecting to static.rust-lang.org|13.32.204.23|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 12444912 (12M) [binary/octet-stream]
Saving to: '/tmp/tmp.MaLLLi/rustup-init'
<VERY LONG SNIP>
info: default toolchain set to 'stable-x86_64-unknown-linux-musl'

  stable-x86_64-unknown-linux-musl installed - rustc 1.48.0 (7eac88abb 2020-11-16)


Rust is installed now. Great!

To get started you need Cargo's bin directory ($HOME/.cargo/bin) in your PATH
environment variable. Next time you log in this will be done
automatically.

To configure your current shell, run:
source $HOME/.cargo/env
bash-4.4# source ~/.cargo/env
bash-4.4# rustc --version
rustc 1.48.0 (7eac88abb 2020-11-16)
bash-4.4# cargo --version
cargo 1.48.0 (65cbdd2dc 2020-10-14)

@kinnison
Copy link
Contributor

Fantastic, thank you so much. This looks good to merge.

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

Successfully merging this pull request may close these issues.

Installer detection of TLS 1.2 support fails for curl 7.73.0+
2 participants