OK, I only want to get this fixed now. Do you agree with the new fix which is AIX-only and prepends '/' to the filename as suggested by you:
http://cr.openjdk.java.net/~simonis/webrevs/2016/8146425.v1/ Regards, Volker On Tue, Jan 5, 2016 at 5:47 PM, Erik Joelsson <erik.joels...@oracle.com> wrote: > Hello Volker, > > On 2016-01-05 14:27, Volker Simonis wrote: >> >> I see, but as far as I understand all you are saying only applies to >> the new hotspot-build system in build-infra. So I really don't see why >> these changes had to be integrated into jdk9-dev. As far as I can see, >> EXCLUDE_PATTERNS isn't currently used anywhere in jdk9-dev. > > That is correct. We use the functionality in build-infra/jdk9 for the new > hotspot build. We integrated a bunch of general build changes early to > reduce the burden of maintaining those changes in the build-infra > repository. This was one of them. Some were relevant to jdk9, this one > wasn't yet. >> >> And I'm a little reluctant to use '/NativeThread.c' as you proposed >> because according to the link you posted this pattern will be >> interpreted as an absolute path with the upcoming changes you are >> doing. > > In my changes, I have to change and verify all the includes/excludes > everywhere since I'm changing the semantics again. I would promise to keep > an extra eye on NativeThread.c. >> >> What is the actual problem with my fix? I could build without >> problems. And I still think EXCLUDE_FILE should just be just a file >> name. If you need to exclude a file by name without specifying a >> sub-directory you can build such a functionality into the new >> EXCLUDE_PATTERNS. > > The problem is that we now have closed code where we already prepended a > slash as part of the change. If you change this behavior back, I will need > to quickly fix those closed places too. That is of course Oracle's problem > and not OpenJDK's. > > While I agree that a parameter named "EXCLUDE_FILES" should probably just > refer to exact filenames, I find the new semantics more useful, so the error > is rather in the parameter name. Also note that SetupJavaCompilation has > been working this way all along so this change was also intended to align > the semantics between the different macros. >> >> I really would appreciate a fast resolution of this problem because >> all our AIX builds are currently unusable. > > I certainly understand that. > > /Erik > >> Thank you and best regards, >> Volker >> >> >> On Tue, Jan 5, 2016 at 9:53 AM, Erik Joelsson <erik.joels...@oracle.com> >> wrote: >>> >>> >>> On 2016-01-05 09:51, Erik Joelsson wrote: >>>> >>>> Hello Volker, >>>> >>>> I believe this change in behavior was intended at the time. In an >>>> internal >>>> situation where the same thing hit us, we changed the exclude pattern in >>>> question by adding a slash at the front: '/NativeThread.c'. >>>> >>> And the reason we needed this change was that we wanted to exclude files >>> by >>> name without having to specify sub directory. Typically in Hotspot, there >>> are a lot of sub directories with native source. >>> >>> /Erik >>> >>>> I should also inform you that I'm currently working on a major overhaul >>>> of >>>> all the INCLUDE/EXCLUDE file finding mechanisms in the build so that >>>> SetupNativeCompilation, SetupJavaCompilation, SetupCopyFiles etc all use >>>> the >>>> same semantics (and same implementation) for finding source files. I'm >>>> doing >>>> the work in the sandbox forest in a branch named >>>> erikj-makefilesets-branch >>>> if you are interested in checking it out. So far I've converted >>>> SetupCopyFiles and SetupTextFileProcessing. The common source file >>>> definitions can be found in the new FileSet.gmk: >>>> >>>> >>>> >>>> http://hg.openjdk.java.net/jdk9/sandbox/file/7ad550ee1d67/make/common/FileSet.gmk >>>> >>>> /Erik >>>> >>>> On 2016-01-04 20:26, Volker Simonis wrote: >>>>> >>>>> Hi, >>>>> >>>>> could someone please review the following small fix: >>>>> >>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8146425/ >>>>> https://bugs.openjdk.java.net/browse/JDK-8146425 >>>>> >>>>> Change "8142907: Integration of minor fixes from the build-infra >>>>> project" has introduced a new parameter called EXCLUDE_PATTERN for >>>>> calls to SetupNativeCompilation. >>>>> >>>>> Unfortunately this change also altered the semantics of EXCLUDE_FILE >>>>> which is now interpreted as a pattern of the form "*EXCLUDE_FILE". >>>>> This is because of the following code: >>>>> >>>>> ifneq ($$($1_ALL_EXCLUDE_FILES),) >>>>> $1_EXCLUDE_FILES_PAT := $$($1_ALL_EXCLUDE_FILES) \ >>>>> $$(foreach i,$$($1_SRC),$$(addprefix >>>>> $$i/,$$($1_ALL_EXCLUDE_FILES))) >>>>> $1_EXCLUDE_FILES_PAT := $$(addprefix %,$$($1_EXCLUDE_FILES_PAT)) >>>>> $1_SRCS := $$(filter-out $$($1_EXCLUDE_FILES_PAT),$$($1_SRCS)) >>>>> >>>>> $1_EXCLUDE_FILES_PAT is initialized to contain all the simple file >>>>> names which are to be excluded plus all of these file names prefixed >>>>> with each src path. In the next step, all the entries in >>>>> $1_EXCLUDE_FILES_PAT are converted to patterns by prefixing them with >>>>> the wildcard character "%". Finally, the patterns are matched against >>>>> all the existing source files. This leads to the problem that every >>>>> file which was given as EXCLUDE_FILES will be converted into a >>>>> "%EXCLUDE_FILES" pattern thus effectively excluding not just files >>>>> with the name EXCLUDE_FILES but actually all files ending in >>>>> EXCLUDE_FILES. >>>>> >>>>> This hit us badly on AIX where we have the implementation file >>>>> AixNativeThread.c and the exclude NativeThread.c. After change 8142907 >>>>> AixNativeThread.c was silently excluded from the compilation leading >>>>> to errors during runtime because the file contained some native >>>>> methods from sun.nio.ch which are not always used. >>>>> >>>>> Thank you and best regards, >>>>> Volker >>>> >>>> >
