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

Create parent folder for build event protocol files #24822

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agustinmista
Copy link

@agustinmista agustinmista commented Jan 3, 2025

Hi!

Some of our CI workflows rely on the --build_event_*_file flags to store the BEP events for further postprocessing. Concretely, our .bazelrc defines:

build --build_event_json_file=build/events.json

The problem is that we output these events files into a build folder that is .gitignored, and hence doesn't exist by default on a fresh clone of our repo. This forces any script that invokes bazel to make sure to mkdir -p build in advance.

We believe this is something that could be elegantly solved on the bazel side by creating the parent build folder if it doesn't exist. And fortunately, there is support for this in the underlying call to InstrumentationOutput.createInstrumentationOutput (added a couple of days ago in c13c669).

This PR sets createParent=true in the underlying call to InstrumentationOutput.createInstrumentationOutput made by BuildEventServiceModule.create.

We'll be happy to discuss about whether this should be the default behavior, or if it's worth it adding a new flag to enable it :)

Copy link

google-cla bot commented Jan 3, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Jan 3, 2025
@agustinmista agustinmista force-pushed the master branch 2 times, most recently from 1406708 to 110f864 Compare January 3, 2025 11:57
@agustinmista
Copy link
Author

Not sure why the PR fails under darwin, the errors look unrelated, e.g.:

Server terminated abruptly (error code: 14, error message: 'Socket closed', log file: '/private/var/tmp/_bazel_buildkite/692ea664b0b8c296611e62f95b1b49ee/sandbox/darwin-sandbox/8313/execroot/_main/_tmp/4b1938da7d4f3c38b4ba75bf7f352f7f/root/6db04d6ef462efd508165bfd20c8851a/server/jvm.out')

Is this a known flakiness or is there something else I'm missing?

@sgowroji sgowroji added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 6, 2025
@sgowroji
Copy link
Member

sgowroji commented Jan 6, 2025

Hi @agustinmista, Below testcase is failing can you please take a look.
//src/test/shell/bazel:command_profiler_test FAILED

@sgowroji sgowroji added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Jan 6, 2025
@agustinmista
Copy link
Author

Hi @sgowroji!

Please check my previous comment. If I understand correctly, the //src/test/shell/bazel:command_profiler_test target fails under darwin with a "socket closed" error that seems (at least in principle) unrelated to my PR (log here). I'll be happy to dive deeper if the issue is on my side, but I'm not sure how can I help addressing that particular error.

Is there any way to re-trigger those checks to discard a possible intermittency?

@sgowroji sgowroji added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants