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.

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

No lock files for bzlmod modules #14554

Closed
shs96c opened this issue Jan 11, 2022 · 24 comments
Closed

No lock files for bzlmod modules #14554

shs96c opened this issue Jan 11, 2022 · 24 comments
Assignees
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@shs96c
Copy link
Contributor

shs96c commented Jan 11, 2022

Description of the problem / feature request:

Although the algorithm used by bzlmod should lead to stable results, having to recalculate the dependency tree with each build is wasteful and may prevent reproducible builds, particularly of historic commits. The WORKSPACE file locked everything down, so there was no need to worry about this, but it would be useful if bzlmod had a lock file it could (optionally) use.

Feature requests: what underlying problem are you trying to solve with this feature?

I want reproducible builds, even if someone has yanked a module since the last build.

What's the output of bazel info release?

release 5.0.0rc3

@Wyverald
Copy link
Member

This is already on the roadmap. Bzlmod will support a lockfile prior to official launch. (Unclear whether we should leave this issue open -- @meteorcloudy wdyt?)

@meteorcloudy
Copy link
Member

I think we can leave this issue open to track the progress of lock file implementation.

@meteorcloudy meteorcloudy added area-Bzlmod Bzlmod-specific PRs, issues, and feature requests team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request P2 We'll consider working on this in future. (Assignee optional) labels Jan 11, 2022
@Wyverald Wyverald assigned SalmaSamy and unassigned Wyverald and meteorcloudy Jun 29, 2022
@brentleyjones
Copy link
Contributor

Bzlmod will support a lockfile prior to official launch.

Is this still on the roadmap?

@meteorcloudy
Copy link
Member

Definitely, @SalmaSamy is working on this

@fmeum
Copy link
Collaborator

fmeum commented Jan 4, 2023

@SalmaSamy Are you planning to have this go through the regular Bazel proposal process?

@SalmaSamy
Copy link
Contributor

@fmeum Yes, we will share the proposal draft soon.

@meteorcloudy
Copy link
Member

@keith
Copy link
Member

keith commented Mar 31, 2023

Looks like this just landed 3b9ec35

Minor nit, is there a way the contents could be formatted? Right now it's hard to diff since it's 1 giant line

@keith
Copy link
Member

keith commented Mar 31, 2023

Also when building the second time after having created the file I get this exception:

FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileValue$$Lambda$239/0x000000080028b578@51b4971e' (requested by nodes 'com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue$$Lambda$425/0x00000008005ce2a8@7763b24e')
        at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:592)
        at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:375)
        at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: net.starlark.java.eval.Starlark$InvalidStarlarkValueException: invalid Starlark value: class java.util.ArrayList

@SalmaSamy
Copy link
Contributor

Hi @keith, A change is made to format the file.
Also, Can you provide a reproducible scenario for this error please?

@fmeum
Copy link
Collaborator

fmeum commented Apr 3, 2023

I tried this out and while I didn't run into the exception reported by @keith, I noticed a few issues. Let me know if you would like me to move them to separate issues. Readability improved a lot with the last commit, thanks for that!

  1. When I do a bazel build with --experimental_enable_bzlmod_lockfile and then touch the lock file (no changes made) or do bazel shutdown, resolution unexpectedly reruns for the next build.
  2. Since the lockfile flag is part of Starlark semantics, flipping it causes all repository rules to be rerun. Based on a quick glance at the PR it looks like the value of this flag isn't ever accessed from Starlark and doesn't seem to influence the way a MODULE file would be evaluated (it just skips evaluation). Maybe this invalidation isn't necessary?
  3. *.bazel files are usually written in some dialect of Starlark, but MODULE.lock.bazel is JSON. The proposal included suggestions such as bazel-lock.json, which I think would be more descriptive.
  4. Gson escapes the angle brackets in <root> by default, which can be confusing. This can easily be disabled.

@meteorcloudy
Copy link
Member

*.bazel files are usually written in some dialect of Starlark, but MODULE.lock.bazel is JSON. The proposal included suggestions such as bazel-lock.json, which I think would be more descriptive.

@fmeum @Wyverald @SalmaSamy
Do you think MODULE.bazel.lockwould be better? It'll make sure the lock file appears next to the MODULE.bazel, but doesn't make strong implication on its format.

@Wyverald
Copy link
Member

Wyverald commented Apr 3, 2023

I'm ambivalent between MODULE.bazel.lock and MODULE.lock.bazel. Either is okay IMO :)

@meteorcloudy
Copy link
Member

I'm slightly in favor of MODULE.bazel.lock, because in reality people easily associate file extension with file format, although technically in our case it's the full file names associated with the format (WORKSPACE.bazel, BUILD.bazel, MODULE.bazel => Starlark).

@fmeum
Copy link
Collaborator

fmeum commented Apr 3, 2023

I do like MODULE.bazel.lock more than MODULE.lock.bazel for the reasons @meteorcloudy gave.

I'm not completely sure whether I like it more than a name that doesn't involve MODULE: MODULE.bazel.lock may give off the impression that it "locks" the current module rather than all other modules (and, in the future, possibly also all external repositories?). For go, go.sum probably shouldn't be called go.mod.lock for similar reasons.

@SalmaSamy
Copy link
Contributor

I am also okay with MODULE.bazel.lock.

@fmeum Can you explain more what you mean by "it 'locks' the current module rather than all other modules"?
We are suppose to eventually lock the current module with its dependencies, overrides, extensions and so on.

@SalmaSamy
Copy link
Contributor

SalmaSamy commented Apr 3, 2023

@fmeum Regarding the other issues:

  1. I will look into that.
  2. This is very temporary, I am working on the flag mentioned int the document which won't be part of Starlark semantics.
  3. Will update soon!
  4. An update is pushed for that.

Thanks!

@fmeum
Copy link
Collaborator

fmeum commented Apr 3, 2023

@fmeum Can you explain more what you mean by "it 'locks' the current module rather than all other modules"?
We are suppose to eventually lock the current module with its dependencies, overrides, extensions and so on.

The MODULE.bazel file, assuming a read-only registry, fully determines all the module dependencies and module extension tags of all transitive Bzlmod dependencies. However, it does not necessarily fully determine the attributes of the repository rules instantiated by module extension. For example, the repository rules instantiated by a go_sdk extension could depend on the result of an API call that determines the latest Go SDK version.

In this sense the Bzlmod lock file will in the future pin more than just the resolved contents of all MODULE.bazel files, it will also pin the results of arbitrary non-hermetic operations performed by module extensions. I am just wondering whether MODULE.bazel.lock could give of the impression that it would only do the former. But I still like the name and this may just be a theoretical concern, so feel free to go ahead with it.

@meteorcloudy
Copy link
Member

Yeah, I guess no matter what name we choose, eventually experienced users will understand what it is for. The difference is basically which one is more friendly to beginner users, and for that I still prefer MODULE.bazel.lock a bit more that bazel-lock.json.

@SalmaSamy
Copy link
Contributor

The name is updated to MODULE.bazel.lock.

@SalmaSamy
Copy link
Contributor

SalmaSamy commented Apr 18, 2023

The flag has been updated to match the document.
Now it is "--lockfile_mode={mode}" where mode can be off, update and error.

@fmeum Can you please provide a reproducible scenario for point number 1, and how do you know that resolution was rerun?

@fmeum
Copy link
Collaborator

fmeum commented Apr 18, 2023

@SalmaSamy I cannot reproduce anymore (I just checked for internet access to the registry before). Looks pretty good now!

@SalmaSamy
Copy link
Contributor

SalmaSamy commented Apr 18, 2023

@fmeum I see.
That is because we were storing the module registry in the lockfile, so it was working (using the lockfile instead of running resolution) but still reaching to registry to get the repospecs. Now after adding repospec to the module, we don't store/use the registry anymore.

@Wyverald Wyverald added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Apr 25, 2023
@Wyverald
Copy link
Member

This has mostly landed on master and will be in 6.3.0. Closing.

@Wyverald Wyverald added this to the 7.0.0 branch cut milestone Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
Archived in project
Development

No branches or pull requests

7 participants