cleanup: shorten Bazel project name#10207
Conversation
| # limitations under the License. | ||
|
|
||
| workspace(name = "com_github_googleapis_google_cloud_cpp") | ||
| workspace(name = "google_cloud_cpp") |
There was a problem hiding this comment.
I think this is a breaking change.
Customers that depend on @com_github_googleapis_google_cloud_cpp//... have to update their BUILD files.
is there some alias we can leave?
There was a problem hiding this comment.
I see that the name field in http_archive() is required. So maybe this is not a problem.
There was a problem hiding this comment.
I do not think this is breaking. Customers can continue to use something like this:
http_archive(
name = "com_github_googleapis_google_cloud_cpp",
sha256 = "245e198e29c4ec19734cc99ef631daaefbdb874307fc3743e22514ee8bcb36c4",
strip_prefix = "google-cloud-cpp-2.4.0",
url = "https://github.com/googleapis/google-cloud-cpp/archive/v2.4.0.tar.gz",
)
And use the @com_github_googleapis_google_cloud_cpp//... in their build scripts (or Bazel files). It used to be impossible to use any other name because we hard-coded @com_github_googleapis_google_cloud_cpp in our own BUILD files, but I have managed to remove that restriction (using Label() when possible, changing some stuff to be "patches" instead of references).
No, we cannot leave an alias. There is a long discussion in bazelbuild/bazel#3219 explaining what the Bazel team thinks is a solution, and everybody else telling them it is not.
There was a problem hiding this comment.
Please check my logic here. I could still be misunderstanding....
Considering this example:
google-cloud-cpp/bazel/google_cloud_cpp_deps.bzl
Lines 50 to 59 in 8a8ba10
Let's say abseil changes their project name to com_google_abseil_new. All of our code keeps working. No problem.
But if we change our load_deps() to check for "com_google_abseil_new", it would be a breaking change. (e.g. if a downstream app is pinning abseil to a specific version, we would ignore it in favor of our default version of abseil.).
So are we to blame for the break? or is it really abseil....
There was a problem hiding this comment.
But if we change our
load_deps()to check for"com_google_abseil_new"
Why would we do that? We are trying to find out if something else has loaded Abseil with the name we intend to use. The name that Abseil calls itself is not material here.
A different question would be:
- What if a downstream application starts loading Abseil with the
com_google_abseil_newname?
They could do that. And that may break things if they loaded a different version (ODR and diamond dependencies get messy quickly). In the Bazel world it is the job of the leaf WORKSPACE to satisfy all the dependencies consistently. That is cumbersome, and thus the invention of bzlmod.
For example, if gRPC starts using a different name for Abseil then we need to use the same name, or introduce some kind of repository mapping (and/or bind()) to make sure gRPC uses the same version we loaded.
There was a problem hiding this comment.
In the Bazel world it is the job of the leaf WORKSPACE to satisfy all the dependencies consistently.
Ah. I did not realize this was the paradigm. That clears things up. Thanks.
0db7fd9 to
79fca3d
Compare
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov ReportBase: 93.98% // Head: 93.98% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #10207 +/- ##
==========================================
- Coverage 93.98% 93.98% -0.01%
==========================================
Files 1531 1531
Lines 141082 141082
==========================================
- Hits 132597 132596 -1
- Misses 8485 8486 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Using a shorter project name will avoid problems on Windows, where Bazel can create paths exceeding the MSVC hardcoded limits (260 characters). Note that, AFAICT, this is a bug in the compiler. Windows supports longer paths, but the compiler cannot use them for some reason. Bazel does not help either, by creating deeply nested names for the object files.
79fca3d to
3388b81
Compare
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
dbolduc
left a comment
There was a problem hiding this comment.
FYI: I think this fixes the issue linked in the PR description
I was missing one change in CI, fixed, and will update the description. |
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Using a shorter project name will avoid problems on Windows, where Bazel
can create paths exceeding the MSVC hardcoded limits (260 characters).
Note that, AFAICT, this is a bug in the compiler. Windows supports
longer paths, but the compiler cannot use them for some reason. Bazel
does not help either, by creating deeply nested names for the object
files.
Fixes #9340
This change is