Don't save mac metadata/extended attributes for brew bottle
#15173
Conversation
This file contains 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
ywwry66
force-pushed
the
fix-brew-tests-tar
branch
from
2c0cfb0
to
f3bcca1
Compare
MikeMcQuaid
reviewed
Apr 10, 2023
ywwry66
force-pushed
the
fix-brew-tests-tar
branch
from
4828eda
to
04c73a0
Compare
MikeMcQuaid
reviewed
Apr 11, 2023
Comment on lines
+7
to
+8
| # Without --only-json-tab bottles are never reproducible | ||
| return ["tar", tar_args].freeze unless args.only_json_tab? |
There was a problem hiding this comment.
Would be nice to share this with generic if possible. Not sure if it is though 🤔. Feel free to leave it!
There was a problem hiding this comment.
It is possible, but I end up with more lines of code and more complicated logic so I guess it is better to leave as is.
This commit includes `--no-mac-metadata` `--no-acls` and `--no-xattrs` in `default_tar_args` for `brew bottle` command. Although `default_tar_args` is only active when `--only-json-tab` is not passed, in which case we don't require reproducible bottles, it is nonetheless beneficial to "regularize" tarball creation. In particular, this resolves a sporadic `brew tests --only=dev-cmd/bottle:20` failure (see https://github.com/orgs/Homebrew/discussions/4376 and Homebrew#14997). Furthermore, with `gnu tar`, `--no-acls` and `--no-xattrs` are default flags. As for "mac metadata", although I couldn't find official documentation, this post (https://superuser.com/a/61188) shares some info: - Resource forks (resource forks have been extended attributes since 10.4) - Custom icons set in Finder and the images of Icon\r files - Metadata in PSD files - Objects stored in scpt files, AppleScript Editor window state, descriptions of scripts - Information about aliases (aliases stop working if extended attributes are removed) - Quarantine status or source URLs of files downloaded from the internet - Spotlight comments - Encoding of files saved with TextEdit - Caret position of files opened with TextMate - Skim notes None of these is supposed to be in the bottle I believe.
ywwry66
force-pushed
the
fix-brew-tests-tar
branch
from
04c73a0
to
73a1380
Compare
MikeMcQuaid
approved these changes
Apr 11, 2023
|
Thanks again @ywwry66, great work! |
|
Test failure is irrelevant. |
apainintheneck
approved these changes
Apr 12, 2023
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.
This commit includes
--no-mac-metadata--no-aclsand--no-xattrsindefault_tar_argsforbrew bottlecommand.Although
default_tar_argsis only active when--only-json-tabis not passed, in which case we don't require reproducible bottles, it is nonetheless beneficial to "regularize" tarball creation. In particular, this resolves a sporadicbrew tests --only=dev-cmd/bottle:20failure (seehttps://github.com/orgs/Homebrew/discussions/4376 and #14997).
Furthermore, with
gnu tar,--no-aclsand--no-xattrsare default flags. As for "mac metadata", although I couldn't find official documentation, this post (https://superuser.com/a/61188) shares some info:None of these is supposed to be in the bottle I believe.
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?