On Wed, 23 Oct 2024 07:23:44 GMT, Lutz Schmidt <l...@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.
>
> 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.

The user reporting this issue actually had an attribute on the softlink itself, 
which I think we want to remove, so I would rather operate on the softlink. I 
haven't investigated how the attribute appeared on the softlink, but the 
purpose of this code is to clear away any unexpected attributes from files in 
the image.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21649#discussion_r1812654340

Reply via email to