Migrate rules_closure to Bzlmod #635

mollyibot · 2024-12-25T19:50:22Z

No description provided.

…stem

…e of assertHTMLEquals

gkdn · 2025-01-07T01:16:56Z

Is this ready for review?

mollyibot · 2025-01-07T02:52:46Z

Is this ready for review?

yes, let us start the review.

gkdn · 2025-01-07T04:09:29Z

Why did you change the name of all BULD files?

.gitignore

README.md

.bazelversion

MODULE.bazel

closure/extension.bzl

gkdn · 2025-01-07T04:22:39Z

MODULE.bazel

+bazel_dep(
+    name = "platforms",
+    version = "0.0.5",
+    dev_dependency = True,


Is this only dev_dependency?

we can add more dev_dependency to more deps, but I only do not want platform propagated to rules_closure's dependents

let's go through each to decide which ones are for dev so we can help with the build performance (let's leave this to the end)

MODULE.bazel

README.md

closure/testing/closure_js_test.bzl

mollyibot · 2025-01-07T18:56:06Z

Is this ready for review?

Why did you change the name of all BULD files?
lt looks like that BUILD.bazel is preferred over BUILD for cross reference , there are some discussions on
bazelbuild/bazel#4517. also I saw now rules_webtesting begins to use to BUILD.bazel to replace BUILD in https://github.com/bazelbuild/rules_webtesting/pull/478/commits, so I followed that. feel free to reopen the comment if you feel it is not neccessary.

mollyibot · 2025-01-15T21:56:14Z

PTAL.

MODULE.bazel

gkdn · 2025-01-15T23:59:16Z

MODULE.bazel

+maven.override(
+    coordinates = "javacc:javacc",
+    target = "@maven//:net_java_dev_javacc_javacc",
+)


I don't follow this one; what is the purpose?

javacc is no longer updated and relocated → net.java.dev.javacc, so when javacc is needed, it should point to net_java_dev_javacc_javacc

Let's add a clatification comment and also double check jscompiler pom to see how it is working for them.

README.md

gkdn · 2025-01-16T00:04:36Z

README.md

-    ],
-)
+```bzl
+bazel_dep(name = "io_bazel_rules_closure", version = "0.15.0")


Let's have this instruction reflect current state that require manual fetching.

I think you missed this.

I removed the version. do you mean removing the version?

No, I think we need to have extra bazel_dep for things that are not released.

closure/stylesheets/test/BUILD

mollyibot · 2025-01-16T05:28:47Z

PTAL

gkdn

Let's use https://github.com/google/bazel-common and remove manual auto-value / dagger stuff from there like @google_bazel_common//third_party/java/auto:value

gkdn · 2025-01-16T19:53:48Z

README.md

-    ],
-)
+```bzl
+bazel_dep(name = "io_bazel_rules_closure", version = "0.15.0")


I think you missed this.

mollyibot · 2025-01-16T20:30:28Z

I do not see dagger in https://github.com/google/bazel-common ?

gkdn · 2025-01-16T21:22:15Z

I do not see dagger in https://github.com/google/bazel-common ?

I see. I will take a look for an alternative.

mollyibot · 2025-01-17T05:30:42Z

PTAL (apart from dagger related)

gkdn · 2025-01-17T08:40:14Z

MODULE.bazel

        "com.google.jimfs:jimfs:1.1",
-        "com.google.truth:truth:1.4.0",
        "javax.inject:javax.inject:1",


google3/third_party/bazel_common/third_party/java/jsr330_inject ?

gkdn · 2025-01-17T08:44:01Z

MODULE.bazel

        "org.jsoup:jsoup:1.16.1",
+        "org.mockito:mockito-core:5.4.0",


google3/third_party/bazel_common/third_party/java/mockito

Though we might need to update bazel_common version.

yes, I've tried that in my previous version and it broke a test, that is why I revert back using this.

closure/repositories.bzl

gkdn · 2025-01-17T08:50:19Z

README.md

-    ],
-)
+```bzl
+bazel_dep(name = "io_bazel_rules_closure", version = "0.15.0")


No, I think we need to have extra bazel_dep for things that are not released.

mollyibot · 2025-01-18T00:07:14Z

PTAL

mollyibot · 2025-01-18T00:06:38Z

javatests/io/bazel/rules/closure/webfiles/BUILD


 java_test(
    name = "WebfilesTestSuite",
    size = "small",
    srcs = glob(["*.java"]),
    deps = [
+        ":mockito",


I tried to port @google_bazel_common//third_party/java/mockito directly as deps for WebfilesTestSuite, but it gives Unable to make private java.io.PrintStream(boolean,java.io.OutputStream) accessible: module java.base does not "opens java.io" to unnamed module @63b0c6dc

mollyibot · 2025-01-23T04:43:53Z

Add some comments, PTAL.

mollyibot · 2025-01-23T23:16:41Z

I tried bumping rules_scala using the head from this branch https://github.com/mbland/rules_scala/commits/bzlmod-bazel-8/?before=031fb0d3a63467d87f25397eb8e23faa57d1d3a6+35 (his is the branch the current rules_webtesting is using)and got some errors related to scala deps https://paste.googleplex.com/6526197736144896.

gkdn · 2025-01-23T23:27:32Z

MODULE.bazel

+maven.override(
+    coordinates = "javacc:javacc",
+    target = "@maven//:net_java_dev_javacc_javacc",
+)


Let's add a clatification comment and also double check jscompiler pom to see how it is working for them.

MODULE.bazel

README.md

gkdn · 2025-01-23T23:31:17Z

java/io/bazel/rules/closure/BUILD

-        "@javax_inject",
+        "@google_bazel_common//third_party/java/dagger",
+        "@google_bazel_common//third_party/java/guava",
+        "@google_bazel_common//third_party/java/jsr330_inject",


do we still need explicit, maybe exported now?

gkdn · 2025-01-23T23:31:31Z

java/io/bazel/rules/closure/BUILD

-        "@javax_inject",
+        "@google_bazel_common//third_party/java/guava",
+        "@google_bazel_common//third_party/java/jsr305_annotations",
+        "@google_bazel_common//third_party/java/jsr330_inject",


double check if all needed

gkdn · 2025-01-23T23:31:41Z

java/io/bazel/rules/closure/http/BUILD

+        "@google_bazel_common//third_party/java/dagger",
+        "@google_bazel_common//third_party/java/guava",
+        "@google_bazel_common//third_party/java/jsr305_annotations",
+        "@google_bazel_common//third_party/java/jsr330_inject",


mollyibot added 4 commits December 25, 2024 19:48

Migrate rules_closure to Bzlmod

08078cf

Migrate rules_closure to Bzlmod

0ec6168

Migrate rules_closure to Bzlmod

26efca6

import the change of abseil cpp's fix of Compilation error for filesy…

bcf8d47

…stem

mollyibot closed this Dec 28, 2024

mollyibot reopened this Dec 28, 2024

mollyibot marked this pull request as ready for review December 28, 2024 02:29

mollyibot requested review from soldair, gregmagolan, gkdn and jDramaix as code owners December 28, 2024 02:29

mollyibot added 7 commits December 28, 2024 02:39

More polish

d0f0f87

Update protobuf test to use jssuite from forked files

83b25d4

Update bazel version

db621a3

Migrate to bzlmod

5839801

Update protobuf test to use jssuite from forked files

de3b0d5

Update protobuf test to use jssuite from forked files and remove usag…

bf0b21c

…e of assertHTMLEquals

remove repositories.bzl

7b5bc80

Remove legacy protobuf test

9a8548e

gkdn reviewed Jan 7, 2025

View reviewed changes

mollyibot added 2 commits January 7, 2025 18:34

Add back legacy test

a15a84a

Add a blank line at the end of legacy example test

a448728

mollyibot closed this Jan 7, 2025

mollyibot reopened this Jan 7, 2025

mollyibot added 2 commits January 7, 2025 19:10

Update legacy test to use forked jsunit and asserts

23cf737

Update legacy test to use forked jsunit and asserts

6e8ca83

Remove dev_dependency in platform dep as it is needed in j2cl

7986432

gkdn reviewed Jan 16, 2025

View reviewed changes

mollyibot added 2 commits January 16, 2025 05:23

Resolve some comments

3c1d4ab

Revert Order

d11f6fa

gkdn reviewed Jan 16, 2025

View reviewed changes

README.md Outdated

],

)

```bzl

bazel_dep(name = "io_bazel_rules_closure", version = "0.15.0")

Copy link

Collaborator

gkdn Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you missed this.

mollyibot added 4 commits January 17, 2025 01:44

Remove guava dagger and autovalue deps

bbd1545

More cleanup

19e404c

Remove more deps with bazel_common deps

fd58b31

Clean up some deps

646f734

revert mockito deps as it breaks WorkerTestSuite

740a644

gkdn reviewed Jan 17, 2025

View reviewed changes

mollyibot added 2 commits January 17, 2025 21:00

Replace more deps to use indirect deps from bazel_common

40bc8c1

Modify readme and resolve mockito

c2cc305

mollyibot commented Jan 18, 2025

View reviewed changes

mollyibot added 3 commits January 18, 2025 00:37

Some cleanup

d9d8e18

Revert

4d5d098

Add a commit for the work around

564bf95

Merge branch 'bazelbuild:master' into master

9a3c8cd

gkdn reviewed Jan 23, 2025

View reviewed changes

mollyibot and others added 3 commits January 23, 2025 20:33

Merge branch 'bazelbuild:master' into master

52f7e02

Merge from main branch

36c0ea5

Update MODULE.bazel README.md and delete bazelversion

95675f8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate rules_closure to Bzlmod #635

Migrate rules_closure to Bzlmod #635

mollyibot commented Dec 25, 2024

gkdn commented Jan 7, 2025

mollyibot commented Jan 7, 2025

gkdn commented Jan 7, 2025

gkdn Jan 7, 2025

mollyibot Jan 8, 2025

gkdn Jan 9, 2025

mollyibot commented Jan 7, 2025

mollyibot commented Jan 15, 2025

gkdn Jan 15, 2025

mollyibot Jan 16, 2025

gkdn Jan 23, 2025

gkdn Jan 16, 2025

gkdn Jan 16, 2025

mollyibot Jan 17, 2025

gkdn Jan 17, 2025

mollyibot commented Jan 16, 2025

gkdn left a comment

gkdn Jan 16, 2025

mollyibot commented Jan 16, 2025

gkdn commented Jan 16, 2025

mollyibot commented Jan 17, 2025

gkdn Jan 17, 2025

gkdn Jan 17, 2025

mollyibot Jan 17, 2025

gkdn Jan 17, 2025

mollyibot commented Jan 18, 2025

mollyibot Jan 18, 2025

mollyibot commented Jan 23, 2025

mollyibot commented Jan 23, 2025 •

edited

Loading

gkdn Jan 23, 2025

gkdn Jan 23, 2025

gkdn Jan 23, 2025

gkdn Jan 23, 2025

Migrate rules_closure to Bzlmod #635

Are you sure you want to change the base?

Migrate rules_closure to Bzlmod #635

Conversation

mollyibot commented Dec 25, 2024

gkdn commented Jan 7, 2025

mollyibot commented Jan 7, 2025

gkdn commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollyibot commented Jan 7, 2025

mollyibot commented Jan 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollyibot commented Jan 16, 2025

gkdn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollyibot commented Jan 16, 2025

gkdn commented Jan 16, 2025

mollyibot commented Jan 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollyibot commented Jan 18, 2025

Choose a reason for hiding this comment

mollyibot commented Jan 23, 2025

mollyibot commented Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollyibot commented Jan 23, 2025 •

edited

Loading