code review round 0 for Full Debug Symbols on MacOS X hotspot (7165611)

[Staffan Larsen]

On 9 okt 2013, at 18:06, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:

> 
> On 10/9/13 9:03 AM, Daniel D. Daugherty wrote:
>> Replies also inline...
>> 
>> 
>> On 10/9/13 6:02 AM, David Holmes wrote:
>>> inline ...
>>> 
>>> On 9/10/2013 8:59 AM, Daniel D. Daugherty wrote:
>>>> On 10/1/13 8:52 PM, David Holmes wrote:
>>> 
>>>>> - make/bsd/makefiles/universal.gmk
>>>>> 
>>>>> I did not understand the additional logic here:
>>>>> 
>>>>>  63 # Copy built non-universal binaries in place
>>>>>  64 $(UNIVERSAL_COPY_LIST):
>>>>>  65         BUILT_COPY_FILES="`find
>>>>> $(EXPORT_JRE_LIB_DIR)/{i386,amd64}/$(subst $(EXPORT_JRE_LIB_DIR)/,,$@)
>>>>> 2>/dev/null`"; \
>>>>>  66         if [ -n "$${BUILT_COPY_FILES}" ]; then \
>>>>>  67           for i in $${BUILT_COPY_FILES}; do \
>>>>>  68             if [ -f $${i} ]; then \
>>>>>  69               $(MKDIR) -p $(shell dirname $@); \
>>>>>  70               $(CP) -R $${i} $@; \
>>>>>  71             fi; \
>>>>>  72             if [ -d $${i} ]; then \
>>>>>  73               $(MKDIR) -p $@; \
>>>>>  74             fi; \
>>>>>  75           done; \
>>>>>  76         fi
>>>>> 
>>>>> until I realized that foo.dSYM is a directory not a file! Even so
>>>>> don't you want to copy the contents of the directory across ?
>>>> 
>>>> BUILT_COPY_FILES includes both files and directories so everything
>>>> should get copied across. I've added some commments to the rule
>>>> header and reorganized the logic a bit to hopefully be more clear.
>>> 
>>> Doesn't this assume that the directory will appear before the files within it? Is that guaranteed?
>> 
>> The way find works is that it lists the directory prior to listing
>> the files within the directory. However, even if find didn't, the
>> containing directory would be created via line 69 above. The one
>> non-obvious feature of lines 72-74 is that an empty directory named
>> in the BUILD_COPY_FILES list would get 'copied' to the destination.
>> 
>> Please check out the latest version when I get it out for review.
> 
> It turns out that the above block was the reason for the flat hierarchy
> reported by Staffan when:
> 
>    ZIP_DEBUGINFO_FILES=0 a.k.a. '--disable-zip-debug-info'
> 
> is used. I've redone this block again in a much simpler fashion
> and that seems to be working.

Very good! I look forward to testing it.

/Staffan