google / googletest

GoogleTest - Google Testing and Mocking Framework
https://google.github.io/googletest/
BSD 3-Clause "New" or "Revised" License
34.53k stars 10.09k forks source link

Add MODULE.bazel to support bzlmod #4368

Closed Vertexwahn closed 8 months ago

Vertexwahn commented 1 year ago

Life with Bzlmod is easier if every dependency provides a MODULE.bazel file. Therefore, the main purpose of this PR is to introduce a MODULE.bazel file in googletest. The MODULE.bazel can coexist with the WORKSPACE file. Users have to enable bzlmod via --enable_bzlmod flag (more details to the Bzlmod migration plan can be found here).

This PR is similar to https://github.com/google/googletest/pull/4118#pullrequestreview-1611267844

Differences:

Further remarks:

I recommend closing https://github.com/google/googletest/pull/4118#pullrequestreview-1611267844 and merging this PR

Vertexwahn commented 1 year ago

@derekmauro Please review!

meteorcloudy commented 1 year ago

Can we also upstream the MODULE.bazel to abseil-cpp? Then you won't need to add a patch for the archive_override.

As long as both googletest and abseil-cpp are published to the BCR, the end users can always specify the latest version for both of them, and Bzlmod will resolve the dependencies. I don't see any chicken-egg problem?

Vertexwahn commented 1 year ago

@derekmauro Should I open a PR on abseil-cpp?

derekmauro commented 1 year ago

As long as both googletest and abseil-cpp are published to the BCR, the end users can always specify the latest version for both of them, and Bzlmod will resolve the dependencies. I don't see any chicken-egg problem?

Sure, that works. But what if you are only using GoogleTest directly? You might not even know that you depend on Abseil.

derekmauro commented 1 year ago

@derekmauro Should I open a PR on abseil-cpp?

Let's figure out how to resolve this first.

meteorcloudy commented 1 year ago

Sure, that works. But what if you are only using GoogleTest directly? You might not even know that you depend on Abseil.

I assume googletest will introduce a suitable abseil version automatically? It might not be the latest one, but it should work with googletest?

derekmauro commented 1 year ago

I assume googletest will introduce a suitable abseil version automatically? It might not be the latest one, but it should work with googletest?

What does it mean to "introduce a suitable abseil version automatically"?

meteorcloudy commented 1 year ago

In googletest's MODULE.bazel file, depend on abseil-cpp via a bazel_dep statement.

derekmauro commented 1 year ago

I think this misses the point I made above. Let me try an explicit example.

Abseil and GoogleTest are both at version 1. Abseil introduces feature X in a dev commit. GoogleTest uses feature X in a dev commit. It is able to do this because it uses the archive_override feature to support live at head philosophy. GoogleTest introduces feature Y in a dev commit. Abseil uses feature Y in a dev commit. Again, it is able to this because it uses the archive_override feature to support live at head philosophy.

Now it comes time to release Abseil and GoogleTest version 2. If I release GoogleTest version 2 first, I can't refer to any released version of Abseil in MODULE.bazel, because none of them have feature X. If I release Abseil version 2 first, I can't refer to any released version of GoogleTest in MODULE.bazel, because none of them have feature Y.

meteorcloudy commented 1 year ago

Thanks for the example, I see the problem now.

I guess, we need to decouple the release processes and increase the frequency to address this issue.

Imagine if you release a new version every time you had a new commit to each of the libraries, we won't have this problem since this will fallback to the WORKSPACE use case (where you can point to any commit).

But that's the extreme case, we only need to have a new abseil version once feature X is introduced so that googletests can depend on a released version of abseil and vice versa.

Do you think this would be feasible? I'm not sure how expensive it is to make a new release for those two projects.

derekmauro commented 8 months ago

Support has been added in https://github.com/google/googletest/commit/6a5938233b6519ba99ddb7c7314d45d3fa877969. We will publish support to BCR after the next release.