build: ensure no permission errors thrown in packages-dist script #17566
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devversion
added
pr: merge safe
target: patch
googlebot
added
the
cla: yes
devversion
commented
Nov 1, 2019
scripts/build-packages-dist.sh
Outdated
| # Make all directories in the previous package output writable. Bazel by default | ||
| # makes tree artifacts and file outputs readonly. This causes permission errors | ||
| # when deleting the folder. To avoid these errors, we make all files writable. | ||
| chmod -R u+w ${pkg_dir} |
There was a problem hiding this comment.
I know that this also makes files writable, but it would make things unnecessarily more platform dependent if we rely on find to filter out directories. Nothing wrong with also making the files writable before deletion.
Technically files don't need to be writable for deletion since we have the x bit set in the folder permissions.
josephperrott
requested changes
Nov 1, 2019
devversion
force-pushed
the
build/avoid-permission-error-build-packages-dist
branch
from
db947b3 to
462f3ae
Compare
josephperrott
requested changes
Nov 1, 2019
Bazel by default makes output files or tree artifacts readonly. This means that the packages-dist script fails in case release output has been built previously. This has only an effect in non-windows environments. We can reasonably fix this by making the folder/files writable before trying to delete them. This is similar to what Bazel internally does when deleting outputs of previous builds. see: https://github.com/bazelbuild/bazel/blob/07924e8260a447b69b18e8248203e1089b46962a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java#L1251
devversion
force-pushed
the
build/avoid-permission-error-build-packages-dist
branch
from
462f3ae to
2e6eb22
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bazel by default makes output files or tree artifacts readonly. This
means that the packages-dist script fails in case release output
has been built previously. This has only an effect in non-windows
environments.
We can reasonably fix this by making the folder/files writable
before trying to delete them. This is similar to what Bazel
internally does when deleting outputs of previous builds.
see:
https://github.com/bazelbuild/bazel/blob/07924e8260a447b69b18e8248203e1089b46962a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java#L1251