On Tue, 22 Oct 2024 21:30:40 GMT, Erik Joelsson <er...@openjdk.org> wrote:
> The target mac-jdk-bundle can fail randomly. MacBundles.gmk defines a large
> number of individual copy rules, which can execute in any order. The
> "install-file" (our copy) macro on macos includes a check for weird
> attributes using `xattr` so that we can remove them. We use the switch `-s`
> to make sure that xattr operates on symlinks instead of their targets.
> However, if we find something, we also run `chmod` to make sure we have the
> permissions to remove attributes in the first place, but the chmod command
> does not have the `-h` switch to operate on the symlink instead of the
> target. So if the symlink gets copied before its target, there is a chance
> that we try to run chmod before the target exists, which will result in a
> failure. My proposed fix is to add `-h` to the chmod command so that
> everything operates on the symlink instead of the target.
Changes requested by lucy (Reviewer).
make/common/FileUtils.gmk line 141:
> 139: $(CHMOD) -h u+w '$(call DecodeSpace, $@)'; \
> 140: $(XATTR) -cs '$(call DecodeSpace, $@)'; \
> 141: fi
What about running chmod only against real files?
else \
$(CP) -fRP '$(call DecodeSpace, $<)' '$(call DecodeSpace, $@)'; \
if [ -n "`$(XATTR) -ls '$(call DecodeSpace, $@)'`" ]; then \
$(CHMOD) -h u+w '$(call DecodeSpace, $@)'; \
$(XATTR) -cs '$(call DecodeSpace, $@)'; \
fi \
fi
And maybe add an explaining comment after line 122:
# The above fixup actions are only necessary if the file is actually copied.
# When creating just a softlink, we rely on the softlink target being fixed
when copied.
-------------
PR Review: https://git.openjdk.org/jdk/pull/21649#pullrequestreview-2387551268
PR Review Comment: https://git.openjdk.org/jdk/pull/21649#discussion_r1812046118
