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

[12610] add versions to lockfiles #12788

Merged
merged 8 commits into from
Sep 9, 2021
Merged

[12610] add versions to lockfiles #12788

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a1acfcf
Adds "version" field to `LockfileMetadata`
Sep 8, 2021
91a756f
Updates lockfiles using `generate_all_lockfiles.sh`
Sep 8, 2021
fce4cb6
Update error messages in `from_lockfile`
Sep 8, 2021
2c79762
Removes `new`, adds lockfile scope name to `from_lockfile`
Sep 8, 2021
29a6825
Updates error message handling
Sep 8, 2021
0a08ec7
Address further code review concerns
Sep 8, 2021
32d3bf9
Fix lint complaint
Sep 8, 2021
711b9d4
fix pex_test
Sep 8, 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
1 change: 1 addition & 0 deletions 3rdparty/python/lockfiles/flake8.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "815457a1baf6226c993e5468ccdf64c69fe7214d3d9237911c762733e0130526",
# "valid_for_interpreter_constraints": [
# "CPython<3.10,>=3.7"
Expand Down
1 change: 1 addition & 0 deletions 3rdparty/python/lockfiles/pytest.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "a2057d396be0480d2586be2da25a021149a4f96276b853dd92cc63cfc3ae8503",
# "valid_for_interpreter_constraints": [
# "CPython<3.10,>=3.7"
Expand Down
1 change: 1 addition & 0 deletions 3rdparty/python/lockfiles/user_reqs.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "19770133e0608f747845bc429b61942d2f79b3cad46790962ade28db07e2b4fd",
# "valid_for_interpreter_constraints": [
# "CPython<3.10,>=3.7"
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/codegen/protobuf/python/mypy_protobuf_lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "babed61947e74aedbe0cdbaefdaec172db6d4a9d27e12acc80be5ab623e3acdf",
# "valid_for_interpreter_constraints": [
# "CPython>=3.6"
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/backend/python/lint/bandit/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "b18a9f4d6a8cb4aafb414e9c8108d39dca053f8910ac801c147a932cd37e0040",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6"
Expand Down Expand Up @@ -59,9 +60,9 @@ pyyaml==5.4.1; python_version >= "3.5" and python_full_version < "3.0.0" or pyth
--hash=sha256:49d4cdd9065b9b6e206d0595fee27a96b5dd22618e7520c33204a4a3239d5b10 \
--hash=sha256:c20cfa2d49991c8b4147af39859b167664f2ad4561704ee74c1de03318e898db \
--hash=sha256:607774cbba28732bfa802b54baa7484215f530991055bb562efbed5b2f20a45e
setuptools==57.4.0; python_version >= "3.6" \
--hash=sha256:a49230977aa6cfb9d933614d2f7b79036e9945c4cdd7583163f4e920b83418d6 \
--hash=sha256:6bac238ffdf24e8806c61440e755192470352850f3419a52f26ffe0a1a64f465
setuptools==58.0.3; python_version >= "3.6" \
--hash=sha256:1ceadf3ea9a821ef305505db995f2e21550ea62500900164278c4b23109204f3 \
--hash=sha256:5e4c36f55012a46c1b3e4b67a8236d1d73856a90fc7b3207d29bedb7d2bac417
six==1.16.0; python_version >= "3.5" and python_full_version < "3.0.0" or python_full_version >= "3.3.0" and python_version >= "3.5" \
--hash=sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254 \
--hash=sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/lint/black/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "1ba5f97d92f33b13e0cd56a960380fa6f616fae215d715ddd7a7ebf99795c890",
# "valid_for_interpreter_constraints": [
# "CPython>=3.6.2"
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/lint/docformatter/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "4ee50d37e5a334d7d496e9f7147dbbf19ee2eb0faaea33e46a553b6c692c7672",
# "valid_for_interpreter_constraints": [
# "CPython>=3.6"
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/lint/flake8/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "ebd4b2326fca44e0d0b362def86bddc1b57e6f6d6c4f077fef5e1218286d3883",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6"
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/backend/python/lint/isort/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "56fe21a2ac24d6f257d95eb97829e8c3cb65e2156cd987348ce837c055763302",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6.1"
# "CPython<4,>=3.7"
# ]
# }
# --- END PANTS LOCKFILE METADATA ---
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/lint/pylint/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "b098df4cb8b4729e91282a0894f8804314269decfafe1a00d65b254074c63b8b",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6"
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/lint/yapf/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "7b946efbcd8abc2a50c548b71c84b34e44eabf0430246b309d63d6200dc5715b",
# "valid_for_interpreter_constraints": [
# "CPython>=3.6"
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/subsystems/coverage_py_lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "ed5d33bbada4d1d96c7d57e100c72bdb5f26d6fed4f1e77b0e74f2ea5e43e642",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6"
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/backend/python/subsystems/ipython_lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "fe82372002915f2550a0b25fea5b6f360c639252ad607a978e7e2f6cbd94c99a",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6"
Expand Down Expand Up @@ -50,9 +51,9 @@ ptyprocess==0.7.0; sys_platform != "win32" and python_version >= "3.6" \
pygments==2.10.0; python_version >= "3.6" \
--hash=sha256:b8e67fe6af78f492b3c4b3e2970c0624cbf08beb1e493b2c99b9fa1b67a20380 \
--hash=sha256:f398865f7eb6874156579fdf36bc840a03cab64d1cde9e93d68f46a425ec52c6
setuptools==57.4.0; python_version >= "3.6" \
--hash=sha256:a49230977aa6cfb9d933614d2f7b79036e9945c4cdd7583163f4e920b83418d6 \
--hash=sha256:6bac238ffdf24e8806c61440e755192470352850f3419a52f26ffe0a1a64f465
setuptools==58.0.3; python_version >= "3.6" \
--hash=sha256:1ceadf3ea9a821ef305505db995f2e21550ea62500900164278c4b23109204f3 \
--hash=sha256:5e4c36f55012a46c1b3e4b67a8236d1d73856a90fc7b3207d29bedb7d2bac417
six==1.16.0; python_version >= "3.6" and python_full_version < "3.0.0" or python_full_version >= "3.3.0" and python_version >= "3.6" \
--hash=sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254 \
--hash=sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/subsystems/lambdex_lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "b3766b556f0c70fe89e235378292cb1369830e316df7ab0d4861a7dd0755f856",
# "valid_for_interpreter_constraints": [
# "CPython<3.10,>=3.6"
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/subsystems/pytest_lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "ac24b96fe05b827037bcb568a7578ecf36ae84929bb1409bc9dd0a89d5a2ec70",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6"
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/backend/python/subsystems/setuptools_lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "a5ec2e69360b67f3262d8ecc191c0227f09216343e97e8c9e2a34ce567b07c29",
# "valid_for_interpreter_constraints": [
# "CPython<4,>=3.6"
# ]
# }
# --- END PANTS LOCKFILE METADATA ---

setuptools==57.4.0; python_version >= "3.6" \
--hash=sha256:a49230977aa6cfb9d933614d2f7b79036e9945c4cdd7583163f4e920b83418d6 \
--hash=sha256:6bac238ffdf24e8806c61440e755192470352850f3419a52f26ffe0a1a64f465
setuptools==57.5.0; python_version >= "3.6" \
--hash=sha256:60d78588f15b048f86e35cdab73003d8b21dd45108ee61a6693881a427f22073 \
--hash=sha256:d9d3266d50f59c6967b9312844470babbdb26304fe740833a5f8d89829ba3a24
wheel==0.37.0; (python_version >= "2.7" and python_full_version < "3.0.0") or (python_full_version >= "3.5.0") \
--hash=sha256:21014b2bd93c6d0034b6ba5d35e4eb284340e09d63c59aef6fc14b0f346146fd \
--hash=sha256:e2ef7239991699e3355d54f8e968a21bb940a1dbf34a4d226741e64462516fad
1 change: 1 addition & 0 deletions src/python/pants/backend/python/typecheck/mypy/lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "2bcd33a72af5d12ea4fcc7ada9dcaf7fa1017ba8286216e88c43ad9e1823ce75",
# "valid_for_interpreter_constraints": [
# "CPython>=3.6"
Expand Down
49 changes: 38 additions & 11 deletions src/python/pants/backend/python/util_rules/lockfile_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
BEGIN_LOCKFILE_HEADER = b"# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---"
END_LOCKFILE_HEADER = b"# --- END PANTS LOCKFILE METADATA ---"

LOCKFILE_VERSION = 1


class InvalidLockfileError(Exception):
pass
Expand All @@ -26,7 +28,9 @@ class LockfileMetadata:
valid_for_interpreter_constraints: InterpreterConstraints

@classmethod
def from_lockfile(cls, lockfile: bytes) -> LockfileMetadata:
def from_lockfile(
cls, lockfile: bytes, lockfile_scope_name: str | None = None
Copy link
Contributor

Choose a reason for hiding this comment

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

If my read is correct, you always have the lockfile_path available. Only for tool lockfiles do you have the resolve_name (what you call lockfile_scope_name). If you're up for it, the error message would be even better if you could say

Could not find a Pants metadata block in the lockfile {path} for the resolve {resolve_name}.

Or if resolve_name is None,

Could not find a Pants metadata block in the lockfile {path}.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please call it resolve_name. This error applies to multiple user lockfiles too, and lockfile_scope_name is a misnomer there. The way to refer to this is resolve_name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. Making that change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like lockfile_path is only available for non-default lockfiles, so I've added lockfile_path but made it optional.

) -> LockfileMetadata:
"""Parse all relevant metadata from the lockfile's header."""
in_metadata_block = False
metadata_lines = []
Expand All @@ -38,40 +42,63 @@ def from_lockfile(cls, lockfile: bytes) -> LockfileMetadata:
elif in_metadata_block:
metadata_lines.append(line[2:])

error_suffix = "To resolve this error, you will need to regenerate the lockfile by running `./pants generate-lockfiles"
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting nit, this is over 100 lines and Black won't attempt to fix. You'd need to use implicit string concatenation.


if lockfile_scope_name:
error_suffix += "--resolve={tool_name}"

error_suffix += "`."

lockfile_description: str = ""
if lockfile_scope_name:
lockfile_description = f"the lockfile for `{lockfile_scope_name}`"
else:
"""this lockfile."""
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't do anything. Also prefer ternary expressions: https://www.pantsbuild.org/docs/style-guide#prefer-conditional-expressions-ternary-expressions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching the typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now a 4-level conditional, and it reads pretty poorly as a ternary expression, so I've kept it as an if statement for now. Let me know if this is a problem.


if not metadata_lines:
# TODO(#12314): Add a good error.
raise InvalidLockfileError("")
raise InvalidLockfileError(
f"Could not find a pants metadata block in this {lockfile_scope_name}. {error_suffix}"
)

try:
metadata = json.loads(b"\n".join(metadata_lines))
except json.decoder.JSONDecodeError:
# TODO(#12314): Add a good error.
raise InvalidLockfileError("")
raise InvalidLockfileError(
f"Metadata header in {lockfile_description} is not a valid JSON string and can't "
"be decoded. " + error_suffix
)

def get_or_raise(key: str) -> Any:
try:
return metadata[key]
except KeyError:
# TODO(#12314): Add a good error about the key not being defined.
raise InvalidLockfileError("")
raise InvalidLockfileError(
f"Required key `{key}` is not present in metadata header for "
f"{lockfile_description}. {error_suffix}"
)

requirements_digest = get_or_raise("requirements_invalidation_digest")
if not isinstance(requirements_digest, str):
# TODO(#12314): Add a good error about invalid data type.
raise InvalidLockfileError("")
raise InvalidLockfileError(
f"Metadata value `requirements_invalidation_digest` in {lockfile_description} must "
"be a string. " + error_suffix
)

try:
interpreter_constraints = InterpreterConstraints(
get_or_raise("valid_for_interpreter_constraints")
)
except TypeError:
# TODO(#12314): Add a good error about invalid data type.
raise InvalidLockfileError("")
raise InvalidLockfileError(
f"Metadata value `valid_for_interpreter_constraints` in {lockfile_description} "
"must be a list of valid Python interpreter constraints strings. " + error_suffix
)

return LockfileMetadata(requirements_digest, interpreter_constraints)

def add_header_to_lockfile(self, lockfile: bytes, *, regenerate_command: str) -> bytes:
metadata_dict = {
"version": LOCKFILE_VERSION,
"requirements_invalidation_digest": self.requirements_invalidation_digest,
"valid_for_interpreter_constraints": [
str(ic) for ic in self.valid_for_interpreter_constraints
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/util_rules/lockfile_metadata_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def test_add_header_to_lockfile() -> None:
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 1,
# "requirements_invalidation_digest": "000faaafcacacaca",
# "valid_for_interpreter_constraints": [
# "CPython>=3.7"
Expand Down
12 changes: 10 additions & 2 deletions src/python/pants/backend/python/util_rules/pex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,12 @@ async def build_pex_component(
constraint_file_digest = EMPTY_DIGEST
requirements_file_digest = EMPTY_DIGEST

lockfile_scope_name: str | None
if isinstance(request.requirements, (ToolDefaultLockfile, ToolCustomLockfile)):
lockfile_scope_name = request.requirements.options_scope_name
else:
lockfile_scope_name = None
Copy link
Contributor

Choose a reason for hiding this comment

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

As explained above, please call it resolve_name. Also generally prefer ternary expressions: https://www.pantsbuild.org/docs/style-guide#prefer-conditional-expressions-ternary-expressions

resolve_name = (
     request.requirements.options_scope_name
     if isinstance(request.requirements, (ToolDefaultLockfile, ToolCustomLockfile))
     else None
)

In the future, this will need to capture the resolve name for multiple user lockfiles. It's fine to punt on that, but can you please add something like:

# TODO(#12314): Capture the resolve name for multiple user lockfiles.


if isinstance(request.requirements, Lockfile):
argv.extend(["--requirement", request.requirements.file_path])
argv.append("--no-transitive")
Expand All @@ -494,7 +500,9 @@ async def build_pex_component(
)

requirements_file_digest_contents = await Get(DigestContents, PathGlobs, globs)
metadata = LockfileMetadata.from_lockfile(requirements_file_digest_contents[0].content)
metadata = LockfileMetadata.from_lockfile(
requirements_file_digest_contents[0].content, lockfile_scope_name
)
_validate_metadata(metadata, request, request.requirements, python_setup)

requirements_file_digest = await Get(Digest, PathGlobs, globs)
Expand All @@ -504,7 +512,7 @@ async def build_pex_component(
argv.extend(["--requirement", file_content.path])
argv.append("--no-transitive")

metadata = LockfileMetadata.from_lockfile(file_content.content)
metadata = LockfileMetadata.from_lockfile(file_content.content, lockfile_scope_name)
_validate_metadata(metadata, request, request.requirements, python_setup)

requirements_file_digest = await Get(Digest, CreateDigest([file_content]))
Expand Down