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

bzlmod: “bazel mod” should fail if bzlmod is disabled #21303

Closed
sluongng opened this issue Feb 12, 2024 · 8 comments
Closed

bzlmod: “bazel mod” should fail if bzlmod is disabled #21303

sluongng opened this issue Feb 12, 2024 · 8 comments
Assignees
Labels
help wanted Someone outside the Bazel team could own this P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@sluongng
Copy link
Contributor

Description of the feature request:

It would be nice to have bazel mod failing with a fixed exit code when common —-noenable_bzlmod is set in .bazelrc. Right now, bazel mod always succeed.

Which category does this issue belong to?

External Dependency

What underlying problem are you trying to solve with this feature?

Standalone tools (tools that are not managed or distributed in-repo by Bazel) have a need to detect whether a repository is using bzlmod or not.

Let’s say we have an internal tool that’s get distributed with go install .../abc-cli@latest. The abc cli needs to know whether the current repo is using bzlmod programmatically to change it’s behavior accordingly.

Which operating system are you running Bazel on?

MacOS

What is the output of bazel info release?

7.0.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

Discussed in https://bazelbuild.slack.com/archives/C014RARENH0/p1707680524793959

@github-actions github-actions bot added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Feb 12, 2024
@meteorcloudy meteorcloudy added P2 We'll consider working on this in future. (Assignee optional) help wanted Someone outside the Bazel team could own this and removed untriaged labels Feb 13, 2024
@fmeum
Copy link
Collaborator

fmeum commented Feb 16, 2024

aa60e35 may not provide a full solution to what @sluongng described:

  1. The exit code is not specific to Bzlmod being disabled, so a tool using this can't cleanly distinguish between e.g. syntax for a bazel mod command having changed and Bzlmod being disabled.
  2. Most of the available bazel mod commands, e.g. graph and deps, require information (the unpruned dep graph) that isn't stored in the lockfile and thus take at least a few seconds to run if they haven't before. This makes them pretty costly to run just to learn whether Bzlmod is enabled.

I just learned that bazel info starlark-semantics exists, which will contain enable_bzlmod=true or enable_bzlmod=false if --enable_bzlmod is set to a state that doesn't correspond to its default value (which is true exactly in Bazel 7 and higher). I could see that being a better fit for tools.

SalmaSamy added a commit that referenced this issue Feb 16, 2024
PiperOrigin-RevId: 607621058
Change-Id: Id58e92d4f80e408e5bf4deabe08e8a864940de63
@sluongng
Copy link
Contributor Author

@fmeum bazel info starlark-semantics is a nice trick

However, it does not show the actual value when it's the default value.

And between Bazel 6 and Bazel 7, the default value is flipped. So a more reliable way is something like this

# With bzlmod
> bazel info release starlark-semantics
release: release 6.4.0
starlark-semantics: StarlarkSemantics{enable_bzlmod=true}

# Without bzlmod
> bazel info release starlark-semantics
release: release 7.0.2
starlark-semantics: StarlarkSemantics{enable_bzlmod=false}

# With bzlmod
> bazel info release starlark-semantics
release: release 7.0.2
starlark-semantics: StarlarkSemantics{}

and then switch the behavior depending on the parsed release version.

@fmeum
Copy link
Collaborator

fmeum commented Mar 19, 2024

@hvadehra I noticed 2148d9b, would you say it makes sense to also have this behavior for bazel info starlark-semantics?

@sluongng
Copy link
Contributor Author

Small note: I would appreciate it if the fix commit(s) were cherry-picked back to Bazel 7.

When I view it on Github, it seems to be a Bazel-8-only fix.

@fmeum
Copy link
Collaborator

fmeum commented Mar 19, 2024

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Mar 19, 2024
@iancha1992
Copy link
Member

@bazel-io fork 7.2.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Mar 19, 2024
@hvadehra
Copy link
Member

I noticed 2148d9b, would you say it makes sense to also have this behavior for bazel info starlark-semantics?

Maybe? Note that change only affects the canonicalize-flags command, regular bazel options parsing still excludes starlark flags with default values. Similarly, we'll probably want to keep excluding defaults in starlark semantics generally (to save memory) but adding a toggle to change this for bazel info sounds reasonable to me.

cc @brandjon

@iancha1992
Copy link
Member

iancha1992 commented Apr 9, 2024

Small note: I would appreciate it if the fix commit(s) were cherry-picked back to Bazel 7.

When I view it on Github, it seems to be a Bazel-8-only fix.

I tried to cherry-pick this to release-7.2.0, but looks like the changes are already included with 7.1.0. So we don't need to cherry-pick this to release-7.2.0. See 08cc45b

cc: @sluongng @fmeum

hb-man pushed a commit to JetBrains/hirschgarten that referenced this issue Sep 3, 2024
hb-man pushed a commit to JetBrains/hirschgarten that referenced this issue Sep 3, 2024
hb-man pushed a commit to JetBrains/hirschgarten that referenced this issue Sep 3, 2024
bazel mod show_repo returns a successful exit code even if you add build --enable_bzlmod=false to .bazelrc on older Bazel versions, see bazelbuild/bazel#21303
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Someone outside the Bazel team could own this P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

No branches or pull requests

9 participants