bazelbuild / rules_python

Bazel Python Rules
https://rules-python.readthedocs.io
Apache License 2.0
538 stars 542 forks source link

Avoid merge conflicts in MODULE.bazel.lock #2434

Open psalaberria002 opened 4 days ago

psalaberria002 commented 4 days ago

The digest of the requirements.txt file seems to be added to the lock file, which triggers merge conflicts.

    "@@rules_python+//python/extensions:pip.bzl%pip": {
      "general": {
        "bzlTransitiveDigest": "Iwo5aX1NCLf2xFr7Cq4NNxpYMRA7qIh3/uaa/Kk4myg=",
        "usagesDigest": "A5I0AW9kWKM1sMWDOt7FRtMoXFL0LONjpXqb43RHBrw=",
        "recordedFileInputs": {
          "@@grpc+//requirements.bazel.txt": "95a27c3f9a46b8114d464c70ba93cda18cfe8c02004db81028f9306b2691701e",
          "@@//requirements.txt": "29c6501451ce48549f1dac1b73e0cd42ee924a3b7649ed0080254de4bbcebb7f"
        },

Related thread on Slack https://bazelbuild.slack.com/archives/C014RARENH0/p1731817466761249

Is this necessary?

aignas commented 4 days ago

Previous analysis/comment on why we need to have the extension as non reproducible and include all of the contents in the main lock file. Please see the comment below for just looking at the requirements.txt hash in the lock file.

In terms of what rules_python can do, we have the following options:

In general I don't see good alternatives here except for changes in bazel or dropping support for requirements.txt as the lockfile format that we support cross platform builds for.

@fmeum are there any plans to make the bzlmod extensions being able to control what goes into the lock file?

EDIT: Looking that this proposal has been already implemented, it does not seem that there is any other option. ~than to split the extension into 2~

EDIT2: updated based on Richard's comment.

aignas commented 4 days ago

Just looking at the requirments.txt lock file hash in the MODULE.bazel.lock file - right now it is triggered by us reading the lock file with mctx.path/read. If the requirements.txt file changes, then bzlmod knows that it needs to re-evaluate the extension.

I am not sure if there is anything better we can do - @fmeum, what would be the proposed behaviour here? If we pass things as labels in the rule, are we OK to pass watch = False when reading the files?

rickeylev commented 4 days ago

EDIT: Looking that this proposal has been already implemented, it does not seem that there is any other option than to split the extension into 2.

but earlier in your post you say that having a second extension just reduces the problem, not avoids it?

aignas commented 4 days ago

FYI, just did a quick experiment to see if mctx.read(watch = 'no') would remove the requirements files hashes and it seems that no:

$ diff MODULE.bazel.lock{before,}
162c162
<         "bzlTransitiveDigest": "vHJJUty2FcJdIyZ/BT+BKmemuqJ4FOvi9k8DzEIbpQU=",
---
>         "bzlTransitiveDigest": "45+MBqepr0RThPGA9Ls9A3aC2xq8tQMQYvXNS4+yn7g=",
aignas@panda ~/src/github/aignas/rules_python exp/lock
$ gd
diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl
index 133ed18d..bb4fe659 100644
--- a/python/private/pypi/parse_requirements.bzl
+++ b/python/private/pypi/parse_requirements.bzl
@@ -93,7 +93,7 @@ def parse_requirements(
     for file, plats in requirements_by_platform.items():
         if logger:
             logger.debug(lambda: "Using {} for {}".format(file, plats))
-        contents = ctx.read(file)
+        contents = ctx.read(file, watch = 'no')

         # Parse the requirements file directly in starlark to get the information
         # needed for the whl_library declarations later.

So it suggests me that bazel itself is adding hashes based on the input to the tag classes.