Add build_workspace_root to repository_ctx #13417

illicitonion · 2021-04-30T18:05:15Z

There are a number of use-cases for this, and I've seen it come up in
bazel-discuss and similar. It is possible to abuse this information,
but this information is currently available by looking up something like:

repository_ctx.path(Label("@//:WORKSPACE")).dirname

But this is unreliable as WORKSPACE.bazel was added as an alternative
to WORKSPACE, and trying to look up a label which doesn't exist is a
fatal error.

We're currently using this to work around the fact that labels can't
exist if the file they point at doesn't already exist.
In repository rules we want users to be able to set an attribute to
point at a lockfile, which may not yet exist, and which our rule may
create as a side-effect of running. The nicest way we've worked out how
to do this is to use a attr.string, and use repository_ctx.execute
to test for the existence of a file, but that relies on being able to
find the path to the root repository.

It may be interesting to try to introduce some kind of fallible
label/path lookup, but that feels like a much bigger change for the
future.

illicitonion · 2022-01-04T17:44:18Z

@philwo I've just pushed a rebase, if it's possible to get a review that's be really handy, it's a small but very handy PR!

UebelAndre · 2022-04-16T14:31:16Z

@illicitonion would you be able to resolve the conflicts here?

@philwo @Wyverald @meteorcloudy could someone take a look at the change? It'd be nice to get this in.

illicitonion · 2022-04-19T10:59:55Z

Rebased :)

meteorcloudy · 2022-04-19T14:00:46Z

@Wyverald WDYT? I guess this should also work with Bzlmod?

Wyverald · 2022-04-19T14:49:35Z

.../java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java

  }

+  @StarlarkMethod(
+      name = "build_workspace_directory",


this concept is usually referred to as the "workspace root". Could you update the name throughout this PR?

For what it's worth, build_workspace_directory makes sense to me and matches the related environment variable that's set on executable targets:
https://docs.bazel.build/versions/main/user-manual.html#run

The following extra environment variables are also available to the binary:

BUILD_WORKSPACE_DIRECTORY: the root of the workspace where the build was run.
BUILD_WORKING_DIRECTORY: the current working directory where Bazel was run from.

Done - thanks!

https://docs.bazel.build/versions/main/skylark/lib/Label.html#workspace_root

Obviously we have a disagreement in Bazel itself, but I think it's more important to keep it consistent among Bazel's Starlark APIs. Can you rename it to just workspace_root?

sorry to be nitpicky -- is there a reason why "build" is in the name? I kind of thought ctx.workspace_root would work well.

Also thanks @UebelAndre for the context -- I wasn't aware of the env variables. It makes more sense that those names include "BUILD", since env variables aren't Bazel-specific; but with the ctx object, we're obviously in Bazel.

@meteorcloudy Oh boy, Label.workspace_root has a completely different meaning, though... I think we should deprecate the Label.workspace_* fields and replace them with Label.repo_* or something. The terms "repo" and "workspace" used to be used interchangeably but now we mostly think of the "workspace" as the overall space we work in, housing the main repo and the various external repos.

Removed the build :)

Label.workspace_root has a completely different meaning, though...

Indeed, it's actually the workspace root (which contains the WORKSPACE file) of the repo of the label, repo_root is much better.

There are a number of use-cases for this, and I've seen it come up in `bazel-discuss` and similar. It is possible to abuse this information, but this information is currently available by looking up something like: ```python repository_ctx.path(Label("@//:WORKSPACE")).dirname ``` But this is unreliable as `WORKSPACE.bazel` was added as an alternative to `WORKSPACE`, and trying to look up a label which doesn't exist is a fatal error. We're currently using this to work around the fact that labels can't exist if the file they point at doesn't already exist. In repository rules we want users to be able to set an attribute to point at a lockfile, which may not yet exist, and which our rule may create as a side-effect of running. The nicest way we've worked out how to do this is to use a `attr.string`, and use `repository_ctx.execute` to test for the existence of a file, but that relies on being able to find the path to the root repository. It may be interesting to try to introduce some kind of fallible label/path lookup, but that feels like a much bigger change for the future.

brentleyjones · 2022-04-19T17:07:36Z

@bazel-io flag

ckolli5 · 2022-04-21T15:37:57Z

@bazel-io fork 5.2.0

There are a number of use-cases for this, and I've seen it come up in `bazel-discuss` and similar. It is possible to abuse this information, but this information is currently available by looking up something like: ```python repository_ctx.path(Label("@//:WORKSPACE")).dirname ``` But this is unreliable as `WORKSPACE.bazel` was added as an alternative to `WORKSPACE`, and trying to look up a label which doesn't exist is a fatal error. We're currently using this to work around the fact that labels can't exist if the file they point at doesn't already exist. In repository rules we want users to be able to set an attribute to point at a lockfile, which may not yet exist, and which our rule may create as a side-effect of running. The nicest way we've worked out how to do this is to use a `attr.string`, and use `repository_ctx.execute` to test for the existence of a file, but that relies on being able to find the path to the root repository. It may be interesting to try to introduce some kind of fallible label/path lookup, but that feels like a much bigger change for the future. Closes #13417. PiperOrigin-RevId: 442839850 Co-authored-by: Daniel Wagner-Hall <[email protected]>

google-cla bot added the cla: yes label Apr 30, 2021

oquenchil assigned philwo May 3, 2021

oquenchil added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label May 3, 2021

philwo force-pushed the master branch from 26cb401 to 168b89b Compare December 2, 2021 18:07

abrisco mentioned this pull request Jan 3, 2022

Improve initial-setup/bootstrapping abrisco/cargo-bazel#112

Open

illicitonion force-pushed the build-workspace-directory branch from 2b36629 to 4f21494 Compare January 4, 2022 17:10

philwo requested a review from Wyverald February 15, 2022 05:05

philwo assigned meteorcloudy and unassigned philwo Mar 22, 2022

illicitonion force-pushed the build-workspace-directory branch from 4f21494 to c9a6435 Compare April 19, 2022 10:59

Wyverald reviewed Apr 19, 2022

View reviewed changes

illicitonion force-pushed the build-workspace-directory branch from c9a6435 to dc03879 Compare April 19, 2022 15:31

illicitonion changed the title ~~Add build_workspace_directory to repository_ctx~~ Add build_workspace_root to repository_ctx Apr 19, 2022

illicitonion force-pushed the build-workspace-directory branch from dc03879 to 7ac28af Compare April 19, 2022 15:41

illicitonion force-pushed the build-workspace-directory branch from 7ac28af to 948a2f4 Compare April 19, 2022 15:45

bazel-io closed this in 8edf6ab Apr 19, 2022

bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Apr 19, 2022

bazel-io mentioned this pull request Apr 21, 2022

[5.2.0] Add build_workspace_root to repository_ctx #15312

Closed

bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Apr 21, 2022

illicitonion deleted the build-workspace-directory branch April 21, 2022 16:00

ckolli5 mentioned this pull request May 10, 2022

Add workspace_root to repository_ctx #15441

Merged

illicitonion mentioned this pull request Nov 29, 2022

crate_universe: Start auto-populating missing lockfiles and cargo_lockfiles in Bazel 6 bazelbuild/rules_rust#1676

Open

Add build_workspace_root to repository_ctx #13417

Add build_workspace_root to repository_ctx #13417

Uh oh!

Conversation

illicitonion commented Apr 30, 2021

Uh oh!

illicitonion commented Jan 4, 2022

Uh oh!

UebelAndre commented Apr 16, 2022

Uh oh!

illicitonion commented Apr 19, 2022

Uh oh!

meteorcloudy commented Apr 19, 2022

Uh oh!

Wyverald Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

UebelAndre Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

illicitonion Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

meteorcloudy Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

Wyverald Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

Wyverald Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

illicitonion Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

meteorcloudy Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brentleyjones commented Apr 19, 2022

Uh oh!

ckolli5 commented Apr 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

meteorcloudy Apr 19, 2022 •

edited

Loading