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.

Sign up for GitHub

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

Include rustc version in the user agent, if rustc is available #9987

Merged
merged 4 commits into from
May 21, 2021
Merged

Include rustc version in the user agent, if rustc is available #9987

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f787788
Include rustc version in the user agent, if rustc is available
alex May 17, 2021
722ba2d
Added a new fragment
alex May 17, 2021
248b678
Added comments
alex May 17, 2021
9ab448d
Added a timeout to invoking rustc
alex May 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/pip/_internal/network/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import mimetypes
import os
import platform
import shutil
import subprocess
import sys
import urllib.parse
import warnings
Expand Down Expand Up @@ -163,6 +165,21 @@ def user_agent():
if setuptools_dist is not None:
data["setuptools_version"] = str(setuptools_dist.version)

if shutil.which("rustc") is not None:
# If for any reason `rustc --version` fails, silently ignore it
try:
rustc_output = subprocess.check_output(
["rustc", "--version"], stderr=subprocess.STDOUT
)
except Exception:
pass
else:
if rustc_output.startswith(b"rustc "):
# The format of `rustc --version` is:
# `b'rustc 1.52.1 (9bc8c42bb 2021-05-09)\n'`
# We extract just the middle (1.52.1) part
data["rustc_version"] = rustc_output.split(b" ")[1].decode()

# Use None rather than False so as not to give the impression that
# pip knows it is not being run under CI. Rather, it is a null or
# inconclusive result. Also, we include some value rather than no
Expand Down
6 changes: 4 additions & 2 deletions tests/unit/test_req_file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,13 +598,14 @@ def make_var(name):
with open(tmpdir.joinpath('req1.txt'), 'w') as fp:
fp.write(template.format(*map(make_var, env_vars)))

session = PipSession()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this file around PipSession seem unrelated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not -- PipSession now checks for an env var (specifically PATHEXT on Windows), and these were being constructed inside a monkey-patch of getenv (which was intended to test parse_requirements).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Can you add a comment explaining this, as it seems like a very subtle interaction that would be incredibly hard to diagnose if someone later decided to omit the extra variable as part of a tidy-up.

Actually, does that mean we're doing the subprocess call every time we construct a PipSession? Would it not be better to just calculate the rust version once, as it's not as if it's going to change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

And yes, we do it once per PipSession. While we construct a few of them in tests, I think in pip itself, it's only ever constructed once, so I'm not sure it's a big deal?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably not. Test suite times are dominated by running pip over and over, so a couple of extra rustc runs isn't going to harm anything.

with patch('pip._internal.req.req_file.os.getenv') as getenv:
getenv.side_effect = lambda n: env_vars[n]

reqs = list(parse_reqfile(
tmpdir.joinpath('req1.txt'),
finder=finder,
session=PipSession()
session=session
))

assert len(reqs) == 1, \
Expand All @@ -623,13 +624,14 @@ def test_expand_missing_env_variables(self, tmpdir, finder):
with open(tmpdir.joinpath('req1.txt'), 'w') as fp:
fp.write(req_url)

session = PipSession()
with patch('pip._internal.req.req_file.os.getenv') as getenv:
getenv.return_value = ''

reqs = list(parse_reqfile(
tmpdir.joinpath('req1.txt'),
finder=finder,
session=PipSession()
session=session
))

assert len(reqs) == 1, \
Expand Down