pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.32k stars 636 forks source link

poetry_requirement defined by relative path generates a lockfile with absolute path #21287

Open jasondamour opened 2 months ago

jasondamour commented 2 months ago

Describe the bug pyproject.toml:

[tool.poetry.dependencies]
snorkel = {path="./ext/dist/snorkel-0.9.9-py3-none-any.whl"}

BUILD:

poetry_requirements(name="poetry", resolve="x")

pants.lock:

// This lockfile was autogenerated by Pants. To regenerate, run:
//
//    pants generate-lockfiles --resolve=x
//
// --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
// {
//   "version": 3,
//   "valid_for_interpreter_constraints": [
//     "CPython==3.10.6"
//   ],
//   "generated_with_requirements": [
//     "snorkel@ file:///Users/jasondamour/Workspace/x/x/x/x/x/ext/dist/snorkel-0.9.9-py3-none-any.whl",
// }
// --- END PANTS LOCKFILE METADATA ---

Pants version Which version of Pants are you using? 2.21

OS Generate lock on macos

Additional info This lockfile is not valid on machine besides the lockfile generator host.

jsirois commented 2 months ago

I'll note pex handles this with --path-mapping:

:; pex3 lock create -h | grep -A16 path-mapping
                        [--path-mapping PATH_MAPPINGS] [-r FILE or URL]
                        [--constraints FILE or URL] [--project DIR]
                        [--exclude EXCLUDED] [--override OVERRIDDEN]
                        [--python PYTHON] [--python-path PYTHON_PATH]
                        [--interpreter-constraint INTERPRETER_CONSTRAINT]
                        [--platform PLATFORMS]
                        [--complete-platform COMPLETE_PLATFORMS]
                        [--manylinux [ASSUME_MANYLINUX]]
                        [--resolve-local-platforms]
                        [--resolver-version {pip-legacy-resolver,pip-2020-resolver}]
                        [--pip-version {latest,vendored,20.3.4-patched,22.2.2,22.3,22.3.1,23.0,23.0.1,23.1,23.1.1,23.1.2,23.2,23.3.1,23.3.2,24.0,24.1,24.1.1,24.1.2,24.2}]
                        [--allow-pip-version-fallback]
                        [--extra-pip-requirement EXTRA_PIP_REQUIREMENTS]
                        [--use-pip-config] [--pypi] [-f PATH/URL] [-i URL]
                        [--retries RETRIES] [--timeout SECS] [--proxy PROXY]
                        [--cert PATH] [--client-cert PATH]
                        [--cache-ttl DEPRECATED] [-H DEPRECATED] [--pre]
--
  --path-mapping PATH_MAPPINGS
                        A mapping of the form `NAME|PATH|DESCRIPTION` of a
                        logical name to a concrete local absolute path with an
                        optional description. Can be specified multiple times.
                        The mapping must include the pipe (`|`) separated name
                        and absolute path components, but the trailing pipe-
                        separated description is optional. The mapping is used
                        when creating, and later reading, lock files to ensure
                        the lock file created on one machine can be used on
                        another with a potentially different realization of
                        various paths used in the resolve. A typical example
                        is a find-links repo. This might be provided on the
                        file-system via a network mount instead of via an
                        HTTP(S) server and that network mount may be at
                        different absolute paths on different machines.
                        Classically, it may be in a user's home directory;
                        whose path will vary from user to user.

An example can be found here in an integration test: https://github.com/pex-tool/pex/blob/0f4dc43f2ebcefbed052ce31c5f0628e8c721ed6/tests/integration/test_lock_resolver.py#L316-L378

I have no clue if Pants allows you to leverage this.

huonw commented 2 months ago

I think https://www.pantsbuild.org/2.21/reference/subsystems/python-repos#path_mappings may be how Pants exposes this.

@jasondamour Is the docs there enough to get this working for you?

jasondamour commented 2 months ago

It's still not entirely clear to me... Here's what I tried

[python-repos]
find_links = [
    'file://$BUILD_ROOT/backend/python/ml/ext/dist/'
]
path_mappings = [
    'BUILD_ROOT|%(buildroot)s',
]

And the output:

➜  pants generate-lockfiles --resolve=ml
⠋ 7.72s Generate lockfile for ml
⠓ 
13:43:25.39 [INFO] Completed: Generate lockfile for ml
13:43:25.40 [ERROR] 1 Exception encountered:

Engine traceback:
  in `generate-lockfiles` goal

ProcessExecutionFailure: Process 'Generate lockfile for ml' failed with exit code 1.
stdout:

stderr:
pid 51424 -> /Users/jasondamour/.cache/pants/named_caches/pex_root/venvs/0deb89b23adbbad7a5b9aac06d2362acf7572cfa/4c982503433997e579e4274163218f2997dc553e/bin/python -sE /Users/jasondamour/.cache/pants/named_caches/pex_root/venvs/0deb89b23adbbad7a5b9aac06d2362acf7572cfa/4c982503433997e579e4274163218f2997dc553e/pex --disable-pip-version-check --no-python-version-warning --exists-action a --no-input --isolated -q --cache-dir /Users/jasondamour/.cache/pants/named_caches/pex_root/pip/24.0/pip_cache --log /private/var/folders/_9/qvrc0kmj4ssfvbybnz4_wb7c0000gn/T/pants-sandbox-Dkbc5y/.tmp/pex-pip-log.e9pz6bhu/pip.log download --dest /private/var/folders/_9/qvrc0kmj4ssfvbybnz4_wb7c0000gn/T/pants-sandbox-Dkbc5y/.tmp/tmpdkm0pmaq/Users.jasondamour..pyenv.versions.3.10.6.bin.python3.10 snorkel@ file:///Users/jasondamour/Workspace/x/x/backend/python/ml/ext/dist/snorkel-0.9.9-py3-none-any.whl --index-url https://pypi.org/simple/ --find-links file://$BUILD_ROOT/backend/python/ml/ext/dist/ --retries 5 --timeout 15 exited with 1 and STDERR:
  error: subprocess-exited-with-error

  × pip subprocess to install build dependencies did not run successfully.
  │ exit code: 2
  ╰─> See above for output.

This is really really frustrating boilerplate... I know it's more on pex/pip than pants, but this is another straw on the camels back :/

jasondamour commented 2 months ago

Ok using something like this actually worked:

[python-repos]
find_links = [
    'file://%(buildroot)s/backend/python/ml/ext/dist/'
]
path_mappings = [
    'BUILD_ROOT|%(buildroot)s',
]

However I didn't realize at first, because the summary at the top of the lockfile still uses an invalid absolute path:

// --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
// {
//   "version": 3,
//   "valid_for_interpreter_constraints": [
//     "CPython==3.10.6"
//   ],
//   "generated_with_requirements": [
//     "snorkel@ file:///Users/jasondamour/Workspace/x/x/backend/python/ml/ext/dist/snorkel-0.9.9-py3-none-any.whl",
.
.
.
              "url": "file://${BUILD_ROOT}/backend/python/ml/ext/dist/snorkel-0.9.9-py3-none-any.whl"
jsirois commented 2 months ago

Great, I'm glad Pants plumbed that option.

Yeah - that whole header thing is invalid in the 1st place since json doesn't take comments. That's a Pants added thing I can't help you with.

jasondamour commented 2 months ago

It works with just the following actually:

[python-repos]
path_mappings = [
    'BUILD_ROOT|%(buildroot)s',
]

Couldn't this be a default?

jsirois commented 2 months ago

This is really really frustrating boilerplate... I know it's more on pex/pip than pants

FWIW, this is fully on Pants. For Pex, there is no project configuration file. It doesn't use pyproject.toml nor does it have its own config. Pex is a global tool that knows nothing of a "project"; so you must tell it everything via the CLI and this is very deliberate. Pants, though, does have the notion of a project with pants.toml very specifically needing to be identical to the build root and so it is in a position to reduce boilerplate here as you suggest.

I no longer use or contribute to Pants, but since you do, I would suggest digging in and making things better. Pants is not a product you pay for and can demand things of.

jasondamour commented 2 months ago

Pants is not a product you pay for and can demand things of.

Of course, sorry if this came off wrong. I was asking if it's a reasonable logical assumption. I can try and open up a PR for the change itself.