Skip to content

Commit ca2a4cc

Browse files
committed
Rename constraints param of DefaultIsolatedEnv.install
This is a vestige from a previous version where the constraints.txt file was incorrectly interpreted as a requirement list.
1 parent 3e7a445 commit ca2a4cc

5 files changed

Lines changed: 50 additions & 119 deletions

File tree

docs/changelog/1160.removal.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Remove ``constraints`` parameter of ``env.DefaultIsolatedEnv.install`` and replace with ``constraints_txt_path``. - by
2+
:user:`layday`

src/build/__main__.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -218,17 +218,7 @@ def _bootstrap_build_env(
218218
with DefaultIsolatedEnv(installer=installer, path=env_dir) as env:
219219
builder = ProjectBuilder.from_isolated_env(env, srcdir, runner=runner)
220220

221-
install = env.install
222-
if dependency_constraints_txt:
223-
with open(dependency_constraints_txt, encoding='utf-8') as dependency_constraints_file:
224-
constraints_text = dependency_constraints_file.read()
225-
if constraints_text.strip():
226-
# Passed through as a single element instead of re-parsed into lines, so a requirement can never
227-
# be split from its `--hash` continuation lines (as produced by `pip-compile --generate-hashes`)
228-
# and silently lose its hash check. env.py's installer backends reconstruct the original file via
229-
# `'\n'.join(constraints)` (see `_PipInstaller`/`_UvInstaller.install_dependencies`), a no-op here
230-
# since there is only one element.
231-
install = partial(install, constraints=(constraints_text,))
221+
install = partial(env.install, constraints_txt_path=dependency_constraints_txt)
232222

233223
# first install the build dependencies
234224
install(builder.build_system_requires, _fresh=True)

src/build/env.py

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
else:
6060
from typing import Self
6161

62+
from . import _types
63+
6264
class _DistArgs(typing.TypedDict, total=False):
6365
path: list[str]
6466

@@ -211,7 +213,7 @@ def installed_versions(self, requirements: Collection[str]) -> dict[str, str]:
211213
def install(
212214
self,
213215
requirements: Collection[str],
214-
constraints: Collection[str] = (),
216+
constraints_txt_path: _types.StrPath | None = None,
215217
*,
216218
_fresh: bool = False, # Used internally by CLI to support preset PYTHONPATH
217219
) -> None:
@@ -230,7 +232,7 @@ def install(
230232
'Installing packages in isolated environment:\n' + '\n'.join(f'- {r}' for r in sorted(requirements)),
231233
kind=('step',),
232234
)
233-
self._env_backend.install_dependencies(requirements, constraints, _fresh=_fresh)
235+
self._env_backend.install_dependencies(requirements, constraints_txt_path, _fresh=_fresh)
234236

235237

236238
def _canonical_requirement_name(requirement: str) -> str | None:
@@ -250,7 +252,7 @@ def create(self, path: str) -> None: ...
250252
def install_dependencies(
251253
self,
252254
requirements: Collection[str],
253-
constraints: Collection[str],
255+
constraints_txt_path: _types.StrPath | None,
254256
*,
255257
_fresh: bool = False,
256258
) -> None: ...
@@ -395,7 +397,7 @@ def create(self, path: str) -> None:
395397
def install_dependencies(
396398
self,
397399
requirements: Collection[str],
398-
constraints: Collection[str],
400+
constraints_txt_path: _types.StrPath | None,
399401
*,
400402
_fresh: bool = False,
401403
) -> None:
@@ -423,14 +425,8 @@ def install_dependencies(
423425

424426
cmd += ['-r', os.path.abspath(requirement_file.name)]
425427

426-
if constraints:
427-
with tempfile.NamedTemporaryFile(
428-
'w', prefix='build-constraints-', suffix='.txt', delete=False, encoding='utf-8'
429-
) as constraint_file:
430-
constraint_file.write('\n'.join(constraints))
431-
exit_stack.callback(functools.partial(os.unlink, constraint_file.name))
432-
433-
cmd += ['-c', os.path.abspath(constraint_file.name)]
428+
if constraints_txt_path:
429+
cmd += ['-c', str(constraints_txt_path)]
434430

435431
run_subprocess(cmd, env=_pip_env())
436432

@@ -464,32 +460,25 @@ def create(self, path: str) -> None:
464460
def install_dependencies( # pragma: no cover -- uv tests are skipped on PyPy, covered on CPython
465461
self,
466462
requirements: Collection[str],
467-
constraints: Collection[str],
463+
constraints_txt_path: _types.StrPath | None,
468464
*,
469465
_fresh: bool = False,
470466
) -> None:
471-
with contextlib.ExitStack() as exit_stack:
472-
cmd = [self._uv_bin, 'pip']
473-
474-
if (verbosity := _ctx.verbosity) > 1:
475-
cmd += [f'-{"v" * min(2, verbosity - 1)}']
467+
cmd = [self._uv_bin, 'pip']
476468

477-
cmd += ['install', *requirements, '--python', self.python_executable]
469+
if (verbosity := _ctx.verbosity) > 1:
470+
cmd += [f'-{"v" * min(2, verbosity - 1)}']
478471

479-
if constraints:
480-
with tempfile.NamedTemporaryFile(
481-
'w', prefix='build-constraints-', suffix='.txt', delete=False, encoding='utf-8'
482-
) as constraint_file:
483-
constraint_file.write('\n'.join(constraints))
484-
exit_stack.callback(functools.partial(os.unlink, constraint_file.name))
472+
cmd += ['install', *requirements, '--python', self.python_executable]
485473

486-
cmd += ['-c', os.path.abspath(constraint_file.name)]
474+
if constraints_txt_path:
475+
cmd += ['-c', str(constraints_txt_path)]
487476

488-
env = {k: v for k, v in os.environ.items() if k != 'PYTHONPATH'}
489-
env['VIRTUAL_ENV'] = self._env_path
490-
if 'UV_KEYRING_PROVIDER' not in os.environ and _has_keyring_cli():
491-
env['UV_KEYRING_PROVIDER'] = 'subprocess'
492-
run_subprocess(cmd, env=env)
477+
env = {k: v for k, v in os.environ.items() if k != 'PYTHONPATH'}
478+
env['VIRTUAL_ENV'] = self._env_path
479+
if 'UV_KEYRING_PROVIDER' not in os.environ and _has_keyring_cli():
480+
env['UV_KEYRING_PROVIDER'] = 'subprocess'
481+
run_subprocess(cmd, env=env)
493482

494483
@property
495484
def display_name(self) -> str:

tests/test_env.py

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -286,21 +286,27 @@ def test_install_short_circuits(
286286

287287

288288
@pytest.mark.parametrize('verbosity', range(3))
289-
@pytest.mark.parametrize('constraints', [[], ['foo']])
289+
@pytest.mark.parametrize('constraints_txt', ['', 'foo'])
290290
@pytest.mark.parametrize('fresh', [False, True])
291291
@pytest.mark.usefixtures('local_pip')
292292
def test_default_impl_install_cmd_well_formed(
293+
tmp_path: Path,
293294
mocker: pytest_mock.MockerFixture,
294295
verbosity: int,
295-
constraints: list[str],
296+
constraints_txt: str,
296297
fresh: bool,
297298
) -> None:
298299
mocker.patch.object(_ctx, 'verbosity', verbosity)
299300

301+
constraints_txt_path = None
302+
if constraints_txt:
303+
constraints_txt_path = tmp_path.joinpath('constraints.txt')
304+
constraints_txt_path.write_text(constraints_txt, encoding='utf-8')
305+
300306
with build.env.DefaultIsolatedEnv() as env:
301307
run_subprocess = mocker.patch('build.env.run_subprocess')
302308

303-
env.install(['some', 'requirements'], constraints, _fresh=fresh)
309+
env.install(['some', 'requirements'], constraints_txt_path, _fresh=fresh)
304310

305311
run_subprocess.assert_called_once_with(
306312
[
@@ -316,29 +322,35 @@ def test_default_impl_install_cmd_well_formed(
316322
'--no-input',
317323
'-r',
318324
mocker.ANY,
319-
*(['-c', mocker.ANY] if constraints else []),
325+
*(['-c', str(constraints_txt_path)] if constraints_txt_path else []),
320326
],
321327
env=mocker.ANY,
322328
)
323329

324330

325331
@pytest.mark.parametrize('verbosity', range(3))
326-
@pytest.mark.parametrize('constraints', [[], ['foo']])
332+
@pytest.mark.parametrize('constraints_txt', ['', 'foo'])
327333
@pytest.mark.parametrize('fresh', [False, True])
328334
@pytest.mark.skipif(IS_PYPY, reason='uv cannot find PyPy executable')
329335
@pytest.mark.skipif(MISSING_UV, reason='uv executable not found')
330336
def test_uv_impl_install_cmd_well_formed( # pragma: no cover -- uv tests are skipped on PyPy, covered on CPython
337+
tmp_path: Path,
331338
mocker: pytest_mock.MockerFixture,
332339
verbosity: int,
333-
constraints: list[str],
340+
constraints_txt: str,
334341
fresh: bool,
335342
) -> None:
336343
mocker.patch.object(_ctx, 'verbosity', verbosity)
337344

345+
constraints_txt_path = None
346+
if constraints_txt:
347+
constraints_txt_path = tmp_path.joinpath('constraints.txt')
348+
constraints_txt_path.write_text(constraints_txt, encoding='utf-8')
349+
338350
with build.env.DefaultIsolatedEnv(installer='uv') as env:
339351
run_subprocess = mocker.patch('build.env.run_subprocess')
340352

341-
env.install(['some', 'requirements'], constraints, _fresh=fresh)
353+
env.install(['some', 'requirements'], constraints_txt_path, _fresh=fresh)
342354

343355
run_subprocess.assert_called_once_with(
344356
[
@@ -350,7 +362,7 @@ def test_uv_impl_install_cmd_well_formed( # pragma: no cover -- uv tests are sk
350362
'requirements',
351363
'--python',
352364
mocker.ANY,
353-
*(['-c', mocker.ANY] if constraints else []),
365+
*(['-c', str(constraints_txt_path)] if constraints_txt_path else []),
354366
],
355367
env=mocker.ANY,
356368
)
@@ -360,7 +372,7 @@ def test_uv_impl_install_cmd_well_formed( # pragma: no cover -- uv tests are sk
360372

361373
@pytest.mark.usefixtures('local_pip')
362374
def test_default_impl_install_files_line_endings_not_doubled(mocker: pytest_mock.MockerFixture) -> None:
363-
# The requirements/constraints files are opened in text mode, which translates every
375+
# The requirements files are opened in text mode, which translates every
364376
# '\n' written to os.linesep -- '\r\n' on disk is correct on Windows. Joining with
365377
# os.linesep first (instead of '\n') would double-translate there, turning '\r\n' into
366378
# '\r\r\n'. Read back as bytes, before the files are deleted by the
@@ -372,38 +384,13 @@ def fake_run_subprocess(cmd: list[str], **_kwargs: object) -> None:
372384
for arg in args:
373385
if arg == '-r':
374386
written['requirements'] = Path(next(args)).read_bytes()
375-
elif arg == '-c':
376-
written['constraints'] = Path(next(args)).read_bytes()
377387

378388
with build.env.DefaultIsolatedEnv() as env:
379389
mocker.patch('build.env.run_subprocess', side_effect=fake_run_subprocess)
380-
env.install(['some', 'requirements'], ['a-constraint', 'b-constraint'])
390+
env.install(['some', 'requirements'])
381391

382392
assert b'\r\r' not in written['requirements']
383393
assert written['requirements'].splitlines() == [b'some', b'requirements']
384-
assert b'\r\r' not in written['constraints']
385-
assert written['constraints'].splitlines() == [b'a-constraint', b'b-constraint']
386-
387-
388-
@pytest.mark.skipif(IS_PYPY, reason='uv cannot find PyPy executable')
389-
@pytest.mark.skipif(MISSING_UV, reason='uv executable not found')
390-
def test_uv_impl_install_files_line_endings_not_doubled( # pragma: no cover -- skipped on PyPy, covered on CPython
391-
mocker: pytest_mock.MockerFixture,
392-
) -> None:
393-
written: dict[str, bytes] = {}
394-
395-
def fake_run_subprocess(cmd: list[str], **_kwargs: object) -> None:
396-
args = iter(cmd)
397-
for arg in args:
398-
if arg == '-c':
399-
written['constraints'] = Path(next(args)).read_bytes()
400-
401-
with build.env.DefaultIsolatedEnv(installer='uv') as env:
402-
mocker.patch('build.env.run_subprocess', side_effect=fake_run_subprocess)
403-
env.install(['some', 'requirements'], ['a-constraint', 'b-constraint'])
404-
405-
assert b'\r\r' not in written['constraints']
406-
assert written['constraints'].splitlines() == [b'a-constraint', b'b-constraint']
407394

408395

409396
@pytest.mark.usefixtures('local_pip')

tests/test_main.py

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,10 @@ def test_build_isolated(mocker: pytest_mock.MockerFixture, package_test_flit: st
219219

220220
build.__main__.build_package(package_test_flit, '.', ['sdist'])
221221

222-
install.assert_any_call({'flit_core >=2,<4'}, _fresh=True)
222+
install.assert_any_call({'flit_core >=2,<4'}, constraints_txt_path=None, _fresh=True)
223223

224224
required_cmd.assert_called_with('sdist', None)
225-
install.assert_any_call({'dep1', 'dep2'})
225+
install.assert_any_call({'dep1', 'dep2'}, constraints_txt_path=None)
226226

227227
build_cmd.assert_called_with('sdist', '.', None)
228228

@@ -455,44 +455,7 @@ def test_build_package_with_constraints(
455455
with pytest.raises(build.BuildBackendException, match=re.escape("Backend 'flit_core.buildapi' is not available.")):
456456
build.__main__.build_package(package_test_flit, tmp_path, ['wheel'], dependency_constraints_txt=constraints_txt_path)
457457

458-
install.assert_any_call({'flit_core >=2,<4'}, constraints=('flit-core==12.34\nfoo==wot\n',), _fresh=True)
459-
460-
461-
@pytest.mark.isolated
462-
def test_build_package_with_constraints_passes_file_through_unmodified(
463-
mocker: pytest_mock.MockerFixture, tmp_path: pathlib.Path, package_test_flit: str
464-
) -> None:
465-
# As produced by e.g. `pip-compile --generate-hashes`: a requirement and its --hash options wrapped onto
466-
# continuation lines. Regression test for the requirement/hash pair being split apart when re-parsed into
467-
# individual lines - the file content must reach the installer byte-for-byte instead.
468-
install = mocker.patch('build.env.DefaultIsolatedEnv.install')
469-
470-
constraints_text = (
471-
'flit-core==12.34 \\\n --hash=sha256:aaaa \\\n --hash=sha256:bbbb\n # via test\nfoo==wot \\\n'
472-
' --hash=sha256:cccc\n'
473-
)
474-
constraints_txt_path = tmp_path.joinpath('constraints.txt')
475-
constraints_txt_path.write_text(constraints_text, encoding='utf-8')
476-
477-
with pytest.raises(build.BuildBackendException, match=re.escape("Backend 'flit_core.buildapi' is not available.")):
478-
build.__main__.build_package(package_test_flit, tmp_path, ['wheel'], dependency_constraints_txt=constraints_txt_path)
479-
480-
install.assert_any_call({'flit_core >=2,<4'}, constraints=(constraints_text,), _fresh=True)
481-
482-
483-
@pytest.mark.isolated
484-
def test_build_package_with_empty_constraints_txt(
485-
mocker: pytest_mock.MockerFixture, tmp_path: pathlib.Path, package_test_flit: str
486-
) -> None:
487-
install = mocker.patch('build.env.DefaultIsolatedEnv.install')
488-
489-
constraints_txt_path = tmp_path.joinpath('constraints.txt')
490-
constraints_txt_path.write_text(' \n\n', encoding='utf-8')
491-
492-
with pytest.raises(build.BuildBackendException, match=re.escape("Backend 'flit_core.buildapi' is not available.")):
493-
build.__main__.build_package(package_test_flit, tmp_path, ['wheel'], dependency_constraints_txt=constraints_txt_path)
494-
495-
install.assert_any_call({'flit_core >=2,<4'}, _fresh=True)
458+
install.assert_any_call({'flit_core >=2,<4'}, constraints_txt_path=constraints_txt_path, _fresh=True)
496459

497460

498461
@pytest.mark.pypy3323bug
@@ -954,8 +917,8 @@ def test_bootstrap_build_env_logs_versions(mocker: pytest_mock.MockerFixture) ->
954917
) as result:
955918
assert result is builder
956919

957-
env.install.assert_any_call({'setuptools'}, _fresh=True)
958-
env.install.assert_any_call({'wheel'})
920+
env.install.assert_any_call({'setuptools'}, constraints_txt_path=None, _fresh=True)
921+
env.install.assert_any_call({'wheel'}, constraints_txt_path=None)
959922
log_versions.assert_called_once_with(env, {'setuptools', 'wheel'})
960923

961924

0 commit comments

Comments
 (0)