RFR(S): 8056246 : Fix AIX build after the Modular Source Code change 8054834

[Volker Simonis]

Hi Erik,

thanks for reviewing my change!
Please find my comments inline:

On Thu, Aug 28, 2014 at 5:56 PM, Erik Joelsson <erik.joelsson at oracle.com> wrote:
> Hello Volker,
>
> Thanks for cleaning up!
>
>
> On 2014-08-28 17:22, Volker Simonis wrote:
>>
>> Hi,
>>
>> could somebody please review the following small changes to fix the
>> AIX build after the Modular Source Code change 8054834. I've also done
>> a little cleanup which is detailed below:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8056246/
>> https://bugs.openjdk.java.net/browse/JDK-8056246
>>
>> make/lib/Awt2dLibraries.gmk
>>
>> LIBAWT_CFLAGS contains quite some duplicate include parameters which
>> can be removed.
>>
>>
>> -I$(JDK_TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS_API_DIR)/native/common/sun/awt
>> isn't required because the path
>>
>> $(JDK_TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS_API_DIR)/native/common/sun/awt
>> is already part of LIBAWT_DIRS and the corresponding include parameter
>> is already generated by $(addprefix -I, $(shell find $(LIBAWT_DIRS)
>> -type d))
>> $(foreach dir, $(LIBAWT_DIRS), -I$(dir)) isn't required either,
>> because $(addprefix -I, $(shell find $(LIBAWT_DIRS) -type d)) not only
>> creates include parameters for every sub-directory contained in
>> LIBAWT_DIRS, but also for the directories themselves.
>
> This all looks good.
>
>> make/rmic/Rmic-java.management.gmk
>>
>> This change is a little bit hacky, but it's the best I could find for
>> the moment. The problem is that find on AIX (but also GNU find before
>> version 4.2) are buggy in the sense that they will report an error if
>> "-exec \+" is used in cases where the find command won't find any
>> files.
>>
>> Therefore, the following make rule will fail for incremental builds if
>> the $(RMIC_GENSRC_DIR) directory won't contain any *.class files any
>> more. These class files are deleted during the first invocation of the
>> rule. However, the rule will be re-executed for every new build
>> because its target depends on the deleted files.
>>
>> $(RMIC_GENSRC_DIR)/_the.classes.removed: $(RMI_IIOP) $(RMI_SRC)
>>         $(FIND) $(RMIC_GENSRC_DIR) -name "*.class" $(FIND_DELETE)
>>         $(TOUCH) $@
>>
>> I've therefore added the file _the.classes.removed to the find
>> pattern. That way, the find command will always match at least one
>> file and the buggy "-exec \+" won't complain.
>>
>> $(RMIC_GENSRC_DIR)/_the.classes.removed: $(RMI_IIOP) $(RMI_SRC)
>>         $(FIND) $(RMIC_GENSRC_DIR) \( -name "*.class" -o -name $(@F) \)
>> $(FIND_DELETE)
>>         $(TOUCH) $@
>>
>> Notice, that deleting _the.classes.removed is no problem here, because
>> the the file will be recreated by the following touch command.
>
> This looks safe, but it's not clear why it needs to be done so it certainly
> warrants a comment. Another approach on this would be to find a different
> way of expressing FIND_DELETE on AIX that works better. Perhaps an xargs
> construct? FIND_DELETE is defined in basics.m4 if you would want to give it
> a shot.

I already thought about this possibility, but then again, AIX
find/xargs don't support "-print0"/"-O" which is considered a little
unsafe (see for example
http://stackoverflow.com/questions/896808/find-exec-cmd-vs-xargs).
What do you think, change it to xargs on AIX nevertheless or just add
a comment to the current version?

>> I've build and smoke tested these changes on Linux/x86_64,
>> Linux/ppc64, MacOS X, Solaris10/SPARC and AIX.
>>
>> Thank you and best regards,
>> Volker
>>
>> PS: I would also like to do some further cleanup in
>> NetworkingLibraries.gmk in a follow-up change. I think now that we
>> have all the corresponding directories in place we could rename
>> {solaris,linux,bsd}_close.c to just close.c and move them to
>> $(JDK_TOPDIR)/src/java.base/$(OPENJDK_TARGET_OS)/native/libnet. We
>> could than get rid of the platform dependent  LIBNET_EXCLUDE_FILES in
>> NetworkingLibraries.gmk. What's your opinion?
>
> As Alan said, we intend to follow up on these kinds of changes. I would
> welcome any help in getting them started.
>

Great, thanks. I'll start right after this one is pushed:)

Regards,
Volker

> /Erik