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

Add bazel itself #1691

Closed
wants to merge 1 commit into from
Closed

Add bazel itself #1691

wants to merge 1 commit into from

Conversation

keith
Copy link
Member

@keith keith commented Mar 26, 2024

No description provided.

@keith keith added presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR labels Mar 26, 2024
@bazel-io
Copy link
Member

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (bazel) have been updated in this PR. Please review the changes.

@Wyverald
Copy link
Member

why do you need this? Bazel is not a library, and its codebase shouldn't be used as a dependency.

@keith
Copy link
Member Author

keith commented Mar 26, 2024

Copybara depends on it for a lot of build targets

@meteorcloudy
Copy link
Member

I think we should not encourage projects to depend on Bazel as a library. Maybe copybara can keep introduce Bazel as a http_archive via a module extension and bringing other transitive dependencies in MODULE.bazel?

@brentleyjones
Copy link
Contributor

brentleyjones commented Mar 27, 2024

Hmm. But Bazel does have libraries that projects can depend on. It would make https://github.com/mattrobmattrob/swift-bep-parser/blob/main/MODULE.bazel easier for example. (Though it would also need bazelbuild/bazel#16335 to be resolved for that particular use case.)

@Wyverald
Copy link
Member

Protos such as the BEP should live outside the Bazel repo (see bazelbuild/bazel#3684). For my information, is there anything other than proto definitions that other projects might need from the Bazel codebase? (except for Copybara, which is somewhat of a special case.)

@brentleyjones
Copy link
Contributor

That's my only use case. Maybe someone else will have another example.

@keith
Copy link
Member Author

keith commented Mar 27, 2024

I'm all for that in theory, but what about the copybara case? Reproducing bazel's MODULE.bazel entirely in copybara's isn't really feasible

@Wyverald
Copy link
Member

actually, which parts of Bazel does Copybara use? From a brief code search (https://github.com/search?q=repo%3Agoogle%2Fcopybara%20io_bazel&type=code), I couldn't find anything significant.

@keith
Copy link
Member Author

keith commented Mar 27, 2024

don't let github codesearch give you hope! https://github.com/google/copybara/blob/6485af6a5c662d5549d2f4d229c239d401132e58/third_party/BUILD#L166-L174

@Wyverald
Copy link
Member

OK, assuming that's the only substantial usage: the Starlark interpreter is a very self-contained part of the Bazel codebase. So we could actually create a module from that directory tree (using the strip_prefix trick), with the eventual goal of perhaps moving the interpreter out into its own repo. Alternatively, we could also just do the thing Yun said, and add very few new bazel_deps to Copybara (since the Starlark interpreter doesn't have many external deps).

cc @brandjon

@keith
Copy link
Member Author

keith commented Mar 27, 2024

is there an example of the trick you're talking about I can reference?

@keith
Copy link
Member Author

keith commented Mar 27, 2024

you're right about the starlark library not having many deps, besides the patch that bazel does to rules_jvm_external there was only rules_jvm_external copy pasting to get copybara building without this pr google/copybara#287

@Wyverald
Copy link
Member

is there an example of the trick you're talking about I can reference?

huh, I thought bazel_skylib_gazelle_plugin would use it, but no -- we actually build a release archive with just the subdirectory: https://github.com/bazelbuild/bazel-central-registry/blob/main/modules/bazel_skylib_gazelle_plugin/1.5.0/source.json

so I don't have an example handy, but it should be as easy as "strip_prefix": "src/main/java/net/starlark/java" or something to that effect.

@keith
Copy link
Member Author

keith commented Mar 27, 2024

Where would the separate MODULE.bazel for the deps that are needed live? in the BCR only duplicating what's needed from bazel itself?

@keith
Copy link
Member Author

keith commented Mar 27, 2024

a quick proto type shows we need to pick and choose some stuff from third_party/BUILD and end up with a MODULE.bazel like this:

bazel_dep(name = "rules_jvm_external", version = "6.0")

maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")
maven.install(
    name = "starlark_maven",
    artifacts = [
        "com.github.stephenc.jcip:jcip-annotations:1.0-1",
        "com.google.auto.service:auto-service-annotations:1.0.1",
        "com.google.auto.service:auto-service:1.0",
        "com.google.auto.value:auto-value-annotations:1.9",
        "com.google.auto.value:auto-value:1.8.2",
        "com.google.auto:auto-common:1.2.1",
        "com.google.code.findbugs:jsr305:3.0.2",
        "com.google.errorprone:error_prone_annotations:2.23.0",
        "com.google.errorprone:error_prone_type_annotations:2.23.0",
        "com.google.flogger:flogger-system-backend:0.5.1",
        "com.google.flogger:flogger:0.5.1",
        "com.google.flogger:google-extensions:0.5.1",
        "com.google.guava:failureaccess:1.0.1",
        "com.google.guava:guava:33.0.0-jre",
        "com.ryanharter.auto.value:auto-value-gson-extension:1.3.1",
        "com.ryanharter.auto.value:auto-value-gson-factory:1.3.1",
        "com.ryanharter.auto.value:auto-value-gson-runtime:1.3.1",
        "org.apache.tomcat:tomcat-annotations-api:8.0.5",
        "org.apache.velocity:velocity:1.7",
    ],
)
use_repo(maven, "starlark_maven")

I don't think the steps could be super automatic

@keith
Copy link
Member Author

keith commented Mar 27, 2024

bazel having a patch on rules_jvm_external makes this kinda difficult too

@Wyverald
Copy link
Member

Where would the separate MODULE.bazel for the deps that are needed live? in the BCR only duplicating what's needed from bazel itself?

For the short term, where we don't extract the Starlark interpreter into its own module, we can just put stuff into Copybara's MODULE.bazel file. In which case we don't even need the strip_prefix trick, actually -- they can just bring in the whole Bazel codebase and only use the Starlark interpreter part.

In the future, we should first create a MODULE.bazel in the Starlark tree inside the Bazel repo, publish it as a module, and then potentially split the source out into a separate repo.

@keith
Copy link
Member Author

keith commented Mar 28, 2024

Pulling it in like that works fine except for the rules_jvm_external patch, which I think means copybara has to patch that stuff out manually, which is ok but not ideal. I will try to push that since that's what's blocking #1703

@keith
Copy link
Member Author

keith commented Mar 28, 2024

#1703 is green, so lets just go with that for now!

@keith keith closed this Mar 28, 2024
@keith keith deleted the ks/add-bazel-itself branch March 28, 2024 00:41
@meteorcloudy
Copy link
Member

bazel having a patch on rules_jvm_external makes this kinda difficult too

The rules_jvm_external patch was just to make sure the Bazel bootstrap offline build could work, copybara can just ignore that patch file.

@keith
Copy link
Member Author

keith commented Mar 28, 2024

i'll comment on the other pr on that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants