Skip to content

Commit 767b050

Browse files
f0rmigaf0rmiga
authored
fix: checked-in requirements imports generated requirements (#1053)
* fix: checked-in requirements imports generated requirements Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: shutil.copy instead of shutil.copyfile This allows copying from one filesystem to another, as the `os.rename` (used by copyfile) doesn't work with multiple filesystems. Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: patch os.replace to use shutil.copy Same as the previous commit. Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: runfiles.Rlocation requires paths to be normalized Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: drop rules_python from import This is not compatible with bzlmod. Importing python.runfiles works for both ways. Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: remove unnecessary runfiles Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * doc: why os.replace = shutil.copy Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: allow the test to still be remote cacheable Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * doc: why shutil.copy Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * doc: add missing punctuation Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: remove unnecessary _fix_up_requirements_in_path Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * test: make sure the locked requirements is updated Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: copy requirements back into src tree if needed Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: make sure windows uses forward slashes Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> --------- Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
1 parent 0051393 commit 767b050

File tree

6 files changed

+76
-55
lines changed

6 files changed

+76
-55
lines changed

‎.bazelci/presubmit.yml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,21 +289,49 @@ tasks:
289289
name: compile_pip_requirements integration tests on Ubuntu
290290
working_directory: tests/compile_pip_requirements
291291
platform: ubuntu2004
292+
shell_commands:
293+
# Make a change to the locked requirements and then assert that //:requirements.update does the
294+
# right thing.
295+
- "echo '' > requirements_lock.txt"
296+
- "! git diff --exit-code"
297+
- "bazel run //:requirements.update"
298+
- "git diff --exit-code"
292299
integration_test_compile_pip_requirements_debian:
293300
<<: *reusable_build_test_all
294301
name: compile_pip_requirements integration tests on Debian
295302
working_directory: tests/compile_pip_requirements
296303
platform: debian11
304+
shell_commands:
305+
# Make a change to the locked requirements and then assert that //:requirements.update does the
306+
# right thing.
307+
- "echo '' > requirements_lock.txt"
308+
- "! git diff --exit-code"
309+
- "bazel run //:requirements.update"
310+
- "git diff --exit-code"
297311
integration_test_compile_pip_requirements_macos:
298312
<<: *reusable_build_test_all
299313
name: compile_pip_requirements integration tests on macOS
300314
working_directory: tests/compile_pip_requirements
301315
platform: macos
316+
shell_commands:
317+
# Make a change to the locked requirements and then assert that //:requirements.update does the
318+
# right thing.
319+
- "echo '' > requirements_lock.txt"
320+
- "! git diff --exit-code"
321+
- "bazel run //:requirements.update"
322+
- "git diff --exit-code"
302323
integration_test_compile_pip_requirements_windows:
303324
<<: *reusable_build_test_all
304325
name: compile_pip_requirements integration tests on Windows
305326
working_directory: tests/compile_pip_requirements
306327
platform: windows
328+
shell_commands:
329+
# Make a change to the locked requirements and then assert that //:requirements.update does the
330+
# right thing.
331+
- "echo '' > requirements_lock.txt"
332+
- "! git diff --exit-code"
333+
- "bazel run //:requirements.update"
334+
- "git diff --exit-code"
307335

308336
integration_test_pip_repository_entry_points_ubuntu:
309337
<<: *reusable_build_test_all

‎python/pip_install/requirements.bzl‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ def compile_pip_requirements(
103103

104104
tags = tags or []
105105
tags.append("requires-network")
106+
tags.append("no-remote-exec")
107+
tags.append("no-sandbox")
106108
attrs = {
107109
"args": args,
108110
"data": data,

‎python/pip_install/tools/dependency_resolver/dependency_resolver.py‎

Lines changed: 43 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,39 @@
1414

1515
"Set defaults for the pip-compile command to run it under Bazel"
1616

17+
import atexit
1718
import os
18-
import re
19+
import shutil
1920
import sys
2021
from pathlib import Path
21-
from shutil import copyfile
2222

23+
import piptools.writer as piptools_writer
2324
from piptools.scripts.compile import cli
2425

26+
# Replace the os.replace function with shutil.copy to work around os.replace not being able to
27+
# replace or move files across filesystems.
28+
os.replace = shutil.copy
29+
30+
# Next, we override the annotation_style_split and annotation_style_line functions to replace the
31+
# backslashes in the paths with forward slashes. This is so that we can have the same requirements
32+
# file on Windows and Unix-like.
33+
original_annotation_style_split = piptools_writer.annotation_style_split
34+
original_annotation_style_line = piptools_writer.annotation_style_line
35+
36+
37+
def annotation_style_split(required_by) -> str:
38+
required_by = set([v.replace("\\", "/") for v in required_by])
39+
return original_annotation_style_split(required_by)
40+
41+
42+
def annotation_style_line(required_by) -> str:
43+
required_by = set([v.replace("\\", "/") for v in required_by])
44+
return original_annotation_style_line(required_by)
45+
46+
47+
piptools_writer.annotation_style_split = annotation_style_split
48+
piptools_writer.annotation_style_line = annotation_style_line
49+
2550

2651
def _select_golden_requirements_file(
2752
requirements_txt, requirements_linux, requirements_darwin, requirements_windows
@@ -41,19 +66,6 @@ def _select_golden_requirements_file(
4166
return requirements_txt
4267

4368

44-
def _fix_up_requirements_in_path(absolute_prefix, output_file):
45-
"""Fix up references to the input file inside of the generated requirements file.
46-
47-
We don't want fully resolved, absolute paths in the generated requirements file.
48-
The paths could differ for every invocation. Replace them with a predictable path.
49-
"""
50-
output_file = Path(output_file)
51-
contents = output_file.read_text()
52-
contents = contents.replace(absolute_prefix, "")
53-
contents = re.sub(r"\\(?!(\n|\r\n))", "/", contents)
54-
output_file.write_text(contents)
55-
56-
5769
if __name__ == "__main__":
5870
if len(sys.argv) < 4:
5971
print(
@@ -75,7 +87,6 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file):
7587
# absolute prefixes in the locked requirements output file.
7688
requirements_in_path = Path(requirements_in)
7789
resolved_requirements_in = str(requirements_in_path.resolve())
78-
absolute_prefix = resolved_requirements_in[: -len(str(requirements_in_path))]
7990

8091
# Before loading click, set the locale for its parser.
8192
# If it leaks through to the system setting, it may fail:
@@ -86,7 +97,7 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file):
8697
os.environ["LANG"] = "C.UTF-8"
8798

8899
UPDATE = True
89-
# Detect if we are running under `bazel test`
100+
# Detect if we are running under `bazel test`.
90101
if "TEST_TMPDIR" in os.environ:
91102
UPDATE = False
92103
# pip-compile wants the cache files to be writeable, but if we point
@@ -95,31 +106,13 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file):
95106
# In theory this makes the test more hermetic as well.
96107
sys.argv.append("--cache-dir")
97108
sys.argv.append(os.environ["TEST_TMPDIR"])
98-
# Make a copy for pip-compile to read and mutate
109+
# Make a copy for pip-compile to read and mutate.
99110
requirements_out = os.path.join(
100111
os.environ["TEST_TMPDIR"], os.path.basename(requirements_txt) + ".out"
101112
)
102-
copyfile(requirements_txt, requirements_out)
103-
104-
elif "BUILD_WORKSPACE_DIRECTORY" in os.environ:
105-
# This value, populated when running under `bazel run`, is a path to the
106-
# "root of the workspace where the build was run."
107-
# This matches up with the values passed in via the macro using the 'rootpath' Make variable,
108-
# which for source files provides a path "relative to your workspace root."
109-
#
110-
# Changing to the WORKSPACE root avoids 'file not found' errors when the `.update` target is run
111-
# from different directories within the WORKSPACE.
112-
os.chdir(os.environ["BUILD_WORKSPACE_DIRECTORY"])
113-
else:
114-
err_msg = (
115-
"Expected to find BUILD_WORKSPACE_DIRECTORY (running under `bazel run`) or "
116-
"TEST_TMPDIR (running under `bazel test`) in environment."
117-
)
118-
print(
119-
err_msg,
120-
file=sys.stderr,
121-
)
122-
sys.exit(1)
113+
# Those two files won't necessarily be on the same filesystem, so we can't use os.replace
114+
# or shutil.copyfile, as they will fail with OSError: [Errno 18] Invalid cross-device link.
115+
shutil.copy(requirements_txt, requirements_out)
123116

124117
update_command = os.getenv("CUSTOM_COMPILE_COMMAND") or "bazel run %s" % (
125118
update_target_label,
@@ -137,12 +130,17 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file):
137130

138131
if UPDATE:
139132
print("Updating " + requirements_txt)
140-
try:
141-
cli()
142-
except SystemExit as e:
143-
if e.code == 0:
144-
_fix_up_requirements_in_path(absolute_prefix, requirements_txt)
145-
raise
133+
if "BUILD_WORKSPACE_DIRECTORY" in os.environ:
134+
workspace = os.environ["BUILD_WORKSPACE_DIRECTORY"]
135+
requirements_txt_tree = os.path.join(workspace, requirements_txt)
136+
# In most cases, requirements_txt will be a symlink to the real file in the source tree.
137+
# If symlinks are not enabled (e.g. on Windows), then requirements_txt will be a copy,
138+
# and we should copy the updated requirements back to the source tree.
139+
if not os.path.samefile(requirements_txt, requirements_txt_tree):
140+
atexit.register(
141+
lambda: shutil.copy(requirements_txt, requirements_txt_tree)
142+
)
143+
cli()
146144
else:
147145
# cli will exit(0) on success
148146
try:
@@ -160,7 +158,6 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file):
160158
)
161159
sys.exit(1)
162160
elif e.code == 0:
163-
_fix_up_requirements_in_path(absolute_prefix, requirements_out)
164161
golden_filename = _select_golden_requirements_file(
165162
requirements_txt,
166163
requirements_linux,

‎tests/compile_pip_requirements/BUILD.bazel‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@ EOF
2222
compile_pip_requirements(
2323
name = "requirements",
2424
data = [
25+
"requirements.in",
2526
"requirements_extra.in",
2627
],
2728
extra_args = [
2829
"--allow-unsafe",
2930
"--resolver=backtracking",
3031
],
31-
requirements_in = "requirements.in",
32+
requirements_in = "requirements.txt",
3233
requirements_txt = "requirements_lock.txt",
3334
)

‎tests/compile_pip_requirements/requirements.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-r requirements.in

‎tools/publish/BUILD.bazel‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,4 @@ compile_pip_requirements(
44
name = "requirements",
55
requirements_darwin = "requirements_darwin.txt",
66
requirements_windows = "requirements_windows.txt",
7-
# This fails on RBE right now, and we don't need coverage there:
8-
# WARNING: Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None))
9-
# after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.HTTPSConnection object at 0x7f3784e08110>:
10-
# Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/twine/
11-
#
12-
# ERROR: Could not find a version that satisfies the requirement twine==4.0.2
13-
# (from -r tools/publish/requirements.in (line 1)) (from versions: none)
14-
tags = ["no-remote-exec"],
157
)

0 commit comments

Comments
 (0)