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

Use prebuilt protoc toolchain #139

Merged

Conversation

stagnation
Copy link
Collaborator

@stagnation stagnation commented Jul 2, 2024

This downloads a prebuilt protoc binary and uses that through regular toolchain resolution (with the incompatible flag). This increases build speed and removes the many compiler warnings that pop-up during compilation of protoc sources. More importantly, works better on Windows where, for some reason, the protoc compilation fails from an external module:

gits/bb-deployments $ bazel build \
    --override_module protobuf=../protobuf \
    @protobuf//src/google/protobuf:timestamp_proto
ERROR: C:/tmp/eprbnvuo/external/protobuf~/src/google/protobuf/compiler/java/BUILD.bazel:99:11: Compiling src/google/protobuf/compiler/java/context.cc [for tool] failed: undeclared inclusion(s) in rule '@@protobuf~//src/google/protobuf/compiler/java:java':
this rule is missing dependency declarations for the following files included by 'src/google/protobuf/compiler/java/context.cc':
  'bazel-out/x64_windows-opt-exec-ST-d57f47055a04/bin/external/protobuf~/src/google/protobuf/compiler/java/_virtual_includes/java/google/protobuf/compiler/java/context.h'

Note that building protoc from its own checkout works:

~/gits/protobuf $ bazel build \
    //src/google/protobuf:timestamp_proto
Target //src/google/protobuf:timestamp_proto up-to-date:
  bazel-bin/src/google/protobuf/timestamp_proto-descriptor-set.proto.bin
INFO: Build completed successfully, 1 total action

@stagnation stagnation changed the title Use prebuilt protoc as toolchains Use prebuilt protoc toolchain Jul 2, 2024
@stagnation stagnation force-pushed the feature/use-prebuilt-protoc-toolchain branch 2 times, most recently from f7a4229 to 5db9741 Compare July 2, 2024 08:35
Copy link
Collaborator Author

@stagnation stagnation left a comment

Choose a reason for hiding this comment

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

typo in commit message "downloades"

@stagnation stagnation marked this pull request as ready for review August 20, 2024 08:28
Copy link
Collaborator

@moroten moroten left a comment

Choose a reason for hiding this comment

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

16 minutes instead of 24 minutes for the CI checks is a good improvement.

@stagnation stagnation force-pushed the feature/use-prebuilt-protoc-toolchain branch from 5db9741 to a2c16d0 Compare August 20, 2024 12:56
@stagnation
Copy link
Collaborator Author

typo in commit message "downloades"

Done

This downloads a prebuilt protoc binary and uses that through regular
toolchain resolution (with the incompatible flag). This increases build
speed and removes the many compiler warnings that pop-up during
compilation of protoc sources. And more importantly works better on
Windows. Where for some reason the protoc compilation fails from an
external module:

    gits/bb-deployments $ bazel build \
        --override_module protobuf=../protobuf \
        @protobuf//src/google/protobuf:timestamp_proto
    ERROR: C:/tmp/eprbnvuo/external/protobuf~/src/google/protobuf/compiler/java/BUILD.bazel:99:11: Compiling src/google/protobuf/compiler/java/context.cc [for tool] failed: undeclared inclusion(s) in rule '@@protobuf~//src/google/protobuf/compiler/java:java':
    this rule is missing dependency declarations for the following files included by 'src/google/protobuf/compiler/java/context.cc':
      'bazel-out/x64_windows-opt-exec-ST-d57f47055a04/bin/external/protobuf~/src/google/protobuf/compiler/java/_virtual_includes/java/google/protobuf/compiler/java/context.h'

Whereas it does work when building from its own checkout:

    ~/gits/protobuf $ bazel build \
        //src/google/protobuf:timestamp_proto
    Target //src/google/protobuf:timestamp_proto up-to-date:
      bazel-bin/src/google/protobuf/timestamp_proto-descriptor-set.proto.bin
    INFO: Build completed successfully, 1 total action
@stagnation stagnation force-pushed the feature/use-prebuilt-protoc-toolchain branch from a2c16d0 to e94dda4 Compare August 21, 2024 12:36
@stagnation stagnation merged commit 5039977 into buildbarn:master Aug 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants