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


Reply via email to