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

Prevent failures creating output directories #17968

Conversation

EdSchouten
Copy link
Contributor

@EdSchouten EdSchouten commented Apr 4, 2023

I would receive reports of build failures along these lines:

ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo'

In order to root cause this, I made a minor change to Bazel to extend its logging. See #17951. With that change applied, the error became:

ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo': .../some/path/foo (Not a directory)

I was eventually able to reproduce this by manually creating an output file at bazel-bin/some/path, followed by attempting to build the target. It looks like Bazel simply attempts to create output directories without taking into consideration whether a file already exists at the target location. This is problematic when someone alters an existing build target to emit a directory instead of a regular file.

Note that this only an issue when the remote action file system is used. Plain builds (ones that don't use Builds without the Bytes or bb_clientd) are unaffected. This change addresses this issue by reusing the same fallback path creation strategy that plain builds already use.

@EdSchouten EdSchouten requested a review from coeuvre April 4, 2023 07:31
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 4, 2023
@sgowroji sgowroji added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Apr 4, 2023
@EdSchouten EdSchouten marked this pull request as draft April 4, 2023 09:33
I would receive reports of build failures along these lines:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo'

In order to root cause this, I made a minor change to Bazel to extend
its logging. See bazelbuild#17951. With
that change applied, the error became:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo': .../some/path/foo (Not a directory)

I was eventually able to reproduce this by manually creating an output
file at bazel-bin/some/path, followed by attempting to build the target.
It looks like Bazel simply attempts to create output directories without
taking into consideration whether a file already exists at the target
location. This is problematic when someone alters an existing build
target to emit a directory instead of a regular file.

Note that this only an issue when the remote action file system is used.
Plain builds (ones that don't use Builds without the Bytes or
bb_clientd) are unaffected. This change addresses this issue by reusing
the same fallback path creation strategy that plain builds already use.
@EdSchouten EdSchouten force-pushed the eschouten/20230404-create-output-directories branch from 1d5c4c4 to 1a96230 Compare April 4, 2023 11:43
@EdSchouten EdSchouten changed the title Prevent failures creating output directories when using bb_clientd Prevent failures creating output directories Apr 4, 2023
@EdSchouten EdSchouten marked this pull request as ready for review April 4, 2023 11:44
@EdSchouten EdSchouten requested a review from a team as a code owner April 4, 2023 11:44
Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

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

Thanks!

@coeuvre coeuvre added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Apr 6, 2023
@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Apr 11, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Apr 12, 2023
@keertk
Copy link
Member

keertk commented Apr 12, 2023

@bazel-io fork 6.2.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Apr 12, 2023
ShreeM01 pushed a commit to ShreeM01/bazel that referenced this pull request Apr 12, 2023
I would receive reports of build failures along these lines:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo'

In order to root cause this, I made a minor change to Bazel to extend its logging. See bazelbuild#17951. With that change applied, the error became:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo': .../some/path/foo (Not a directory)

I was eventually able to reproduce this by manually creating an output file at bazel-bin/some/path, followed by attempting to build the target. It looks like Bazel simply attempts to create output directories without taking into consideration whether a file already exists at the target location. This is problematic when someone alters an existing build target to emit a directory instead of a regular file.

Note that this only an issue when the remote action file system is used.    Plain builds (ones that don't use Builds without the Bytes or    bb_clientd) are unaffected. This change addresses this issue by reusing    the same fallback path creation strategy that plain builds already use.

Closes bazelbuild#17968.

PiperOrigin-RevId: 523408378
Change-Id: I0d7559352687c317de72e0bf5bc6b5ba7452f97e
@EdSchouten EdSchouten deleted the eschouten/20230404-create-output-directories branch April 13, 2023 09:16
ShreeM01 added a commit that referenced this pull request Apr 13, 2023
I would receive reports of build failures along these lines:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo'

In order to root cause this, I made a minor change to Bazel to extend its logging. See #17951. With that change applied, the error became:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo': .../some/path/foo (Not a directory)

I was eventually able to reproduce this by manually creating an output file at bazel-bin/some/path, followed by attempting to build the target. It looks like Bazel simply attempts to create output directories without taking into consideration whether a file already exists at the target location. This is problematic when someone alters an existing build target to emit a directory instead of a regular file.

Note that this only an issue when the remote action file system is used.    Plain builds (ones that don't use Builds without the Bytes or    bb_clientd) are unaffected. This change addresses this issue by reusing    the same fallback path creation strategy that plain builds already use.

Closes #17968.

PiperOrigin-RevId: 523408378
Change-Id: I0d7559352687c317de72e0bf5bc6b5ba7452f97e

Co-authored-by: Ed Schouten <[email protected]>
fweikert pushed a commit to fweikert/bazel that referenced this pull request May 25, 2023
I would receive reports of build failures along these lines:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo'

In order to root cause this, I made a minor change to Bazel to extend its logging. See bazelbuild#17951. With that change applied, the error became:

> ERROR: some/path/BUILD:156:32: Executing genrule //some/path:some_target failed: failed to create output directory '.../some/path/foo': .../some/path/foo (Not a directory)

I was eventually able to reproduce this by manually creating an output file at bazel-bin/some/path, followed by attempting to build the target. It looks like Bazel simply attempts to create output directories without taking into consideration whether a file already exists at the target location. This is problematic when someone alters an existing build target to emit a directory instead of a regular file.

Note that this only an issue when the remote action file system is used.    Plain builds (ones that don't use Builds without the Bytes or    bb_clientd) are unaffected. This change addresses this issue by reusing    the same fallback path creation strategy that plain builds already use.

Closes bazelbuild#17968.

PiperOrigin-RevId: 523408378
Change-Id: I0d7559352687c317de72e0bf5bc6b5ba7452f97e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants