Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Ioi Lam]

On 4/4/19 10:08 PM, David Holmes wrote:

Adding back build-dev

On 5/04/2019 2:41 pm, Ioi Lam wrote:

#define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)

This make me a little uneasy, as it could potential point past the 
end of the string.


The intent of course is that that is impossible:
- _FILE_ is an absolute file path within the repo: /something/repo/file
- FILE_MACRO_OFFSET gets you from / to the top-level repo directory 
leaving "file"


so it has to be in bounds.

For safety, Is it possible to get some sort of assert to make sure 
that __FILE__ always has more than FILE_MACRO_OFFSET characters?


Maybe we can add a static object in macros.hpp with a constructor 
that puts __FILE__ into a list, and then we can walk the list when 
the VM starts up? E.g.


 ...

 #ifdef ASSERT
 static ThisFileChecker __this_file_checker(__FILE__);
 #endif

 #endif // SHARE_UTILITIES_MACROS_HPP


So basically you're not trusting the compiler and build system to be 
correct here. :)


I am sure the build system is correct . but it's complicated.

BTW, we actually have generated sources that can live outside of the 
source repo. And with luck, their names can actually be shorter than 
FILE_MACRO_OFFSET.


$ cd /tmp/mybld
$ find . -name \*.cpp
./hotspot/variant-server/support/adlc/ad_x86_expand.cpp
./hotspot/variant-server/support/adlc/ad_x86_pipeline.cpp
./hotspot/variant-server/support/adlc/ad_x86_format.cpp
./hotspot/variant-server/support/adlc/dfa_x86.cpp
./hotspot/variant-server/support/adlc/ad_x86_misc.cpp
./hotspot/variant-server/support/adlc/ad_x86_gen.cpp
./hotspot/variant-server/support/adlc/ad_x86.cpp
./hotspot/variant-server/support/adlc/ad_x86_peephole.cpp
./hotspot/variant-server/support/adlc/ad_x86_clone.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86_expand.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86_pipeline.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86_format.cpp
./hotspot/variant-server/gensrc/adfiles/dfa_x86.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86_misc.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86_gen.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86_peephole.cpp
./hotspot/variant-server/gensrc/adfiles/ad_x86_clone.cpp
./hotspot/variant-server/gensrc/jvmtifiles/jvmtiEnter.cpp
./hotspot/variant-server/gensrc/jvmtifiles/bytecodeInterpreterWithChecks.cpp
./hotspot/variant-server/gensrc/jvmtifiles/jvmtiEnterTrace.cpp



Would it suffice to put a static assert in a common header, like 
macros.hpp, that verifies the length of _FILE_ or is that not 
available statically?


I don't know how a static assert would work. That's why I suggested the 
static object with a constructor.


Thanks
- Ioi


Cheers,
David



Thanks
- Ioi


On 4/4/19 9:04 PM, David Holmes wrote:

Hi Erik,

Build and hotspot changes seem okay.

Thanks,
David

On 5/04/2019 8:03 am, Erik Joelsson wrote:


On 2019-04-04 14:26, Kim Barrett wrote:


OK, I can do that.

-- 


src/hotspot/share/utilities/macros.hpp
  645 #if FILE_MACRO_OFFSET
  646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
  647 #else
  648 #define THIS_FILE __FILE__
  649 #endif

Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
an implicit test for "defined"?

If the former, e.g. we're assuming it will always be defined but 
might

have a 0 value, then I'd skip it and just unconditionally define
THIS_FILE as (__FILE__ + FILE_MACRO_OFFSET).


Right, that makes sense. I was sort of hedging for all 
possibilities here, but as the build logic is currently structured, 
it will always be defined, just sometimes 0.


New webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.02/

/Erik


If the latter, some compilers will (with some warning levels or
options, such as gcc -Wundef) complain about the (specified by the
standard) implicit conversion to 0 for an unrecognized identifier in
an #if expression, and an #ifdef should be used to protect against
that.

-- 

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[David Holmes]

Adding back build-dev

On 5/04/2019 2:41 pm, Ioi Lam wrote:

#define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)

This make me a little uneasy, as it could potential point past the end 
of the string.


The intent of course is that that is impossible:
- _FILE_ is an absolute file path within the repo: /something/repo/file
- FILE_MACRO_OFFSET gets you from / to the top-level repo directory 
leaving "file"


so it has to be in bounds.

For safety, Is it possible to get some sort of assert to make sure that 
__FILE__ always has more than FILE_MACRO_OFFSET characters?


Maybe we can add a static object in macros.hpp with a constructor that 
puts __FILE__ into a list, and then we can walk the list when the VM 
starts up? E.g.


     ...

     #ifdef ASSERT
     static ThisFileChecker __this_file_checker(__FILE__);
     #endif

     #endif // SHARE_UTILITIES_MACROS_HPP


So basically you're not trusting the compiler and build system to be 
correct here. :)


Would it suffice to put a static assert in a common header, like 
macros.hpp, that verifies the length of _FILE_ or is that not available 
statically?


Cheers,
David



Thanks
- Ioi


On 4/4/19 9:04 PM, David Holmes wrote:

Hi Erik,

Build and hotspot changes seem okay.

Thanks,
David

On 5/04/2019 8:03 am, Erik Joelsson wrote:


On 2019-04-04 14:26, Kim Barrett wrote:


OK, I can do that.

-- 


src/hotspot/share/utilities/macros.hpp
  645 #if FILE_MACRO_OFFSET
  646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
  647 #else
  648 #define THIS_FILE __FILE__
  649 #endif

Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
an implicit test for "defined"?

If the former, e.g. we're assuming it will always be defined but might
have a 0 value, then I'd skip it and just unconditionally define
THIS_FILE as (__FILE__ + FILE_MACRO_OFFSET).


Right, that makes sense. I was sort of hedging for all possibilities 
here, but as the build logic is currently structured, it will always 
be defined, just sometimes 0.


New webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.02/

/Erik


If the latter, some compilers will (with some warning levels or
options, such as gcc -Wundef) complain about the (specified by the
standard) implicit conversion to 0 for an unrecognized identifier in
an #if expression, and an #ifdef should be used to protect against
that.

-- 

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[David Holmes]

Hi Erik,

Build and hotspot changes seem okay.

Thanks,
David

On 5/04/2019 8:03 am, Erik Joelsson wrote:


On 2019-04-04 14:26, Kim Barrett wrote:


OK, I can do that.

-- 


src/hotspot/share/utilities/macros.hpp
  645 #if FILE_MACRO_OFFSET
  646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
  647 #else
  648 #define THIS_FILE __FILE__
  649 #endif

Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
an implicit test for "defined"?

If the former, e.g. we're assuming it will always be defined but might
have a 0 value, then I'd skip it and just unconditionally define
THIS_FILE as (__FILE__ + FILE_MACRO_OFFSET).


Right, that makes sense. I was sort of hedging for all possibilities 
here, but as the build logic is currently structured, it will always be 
defined, just sometimes 0.


New webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.02/

/Erik


If the latter, some compilers will (with some warning levels or
options, such as gcc -Wundef) complain about the (specified by the
standard) implicit conversion to 0 for an unrecognized identifier in
an #if expression, and an #ifdef should be used to protect against
that.

-- 

Re: RFR: 8221907: make reconfigure broken with "build/jmh/jars does not exist or is not a directory"

[David Holmes]

Thanks for clarifying Erik.

Looks good.

David

On 4/04/2019 11:48 pm, Erik Joelsson wrote:

Hello David,

On 2019-04-03 18:49, David Holmes wrote:

Hi Erik,

On 4/04/2019 1:33 am, Erik Joelsson wrote:

Hello Jie,

This issue applies not only to --with-jmh, but to any configure 
parameter given with a relative path. I think the proper fix would be 
to record the current working directory when configure is launched 
and cd to that directory when running reconfigure. Here is my 
suggested patch:


http://cr.openjdk.java.net/~erikj/8221907/webrev.01/index.html

The relevant parts are exporting the variable from configure and 
using it in Init.gmk. The rest is just renaming the variable since 
CURDIR would clash with the pre defined make variable CURDIR.


I see how the change fixes the issue with existing relative paths, but 
it indicates that OUTPUTDIR is different to CONFIGURE_START_DIR - so 
what happens with generated output now we have a different cwd? Is it 
all controlled by absolute paths and so will still go to the place(s) 
regardless?


All output is controlled with absolute paths yes. One of the first 
things that happens when configure is run (in basics.m4) is figuring out 
which dir is the workspace root (TOPDIR), which is the OUTPUTDIR etc. 
 From then on, everything is defined as absolute paths using those basic 
variables. Today you can run configure either from the root or from the 
output dir. With this change, any reconfigure will match the original 
cwd, so should get the same behavior as the original run of configure 
(which is the idea of reconfigure).


/Erik


Thanks,
David


/Erik

On 2019-04-03 05:28, Jie Fu wrote:

Hi all,

JBS: https://bugs.openjdk.java.net/browse/JDK-8221907

For more info (e.g. the symptom & how to reproduce), please see the 
JBS.


It can be fixed by
-
diff -r 3326be37cd9a make/autoconf/lib-tests.m4
--- a/make/autoconf/lib-tests.m4    Tue Apr 02 17:27:48 2019 -0700
+++ b/make/autoconf/lib-tests.m4    Wed Apr 03 19:56:24 2019 +0800
@@ -73,6 +73,10 @@
   else
 # Path specified
 JMH_HOME="$with_jmh"
+    if test "x${JMH_HOME:0:1}" != x/; then
+  JMH_HOME="$TOPDIR/$JMH_HOME"
+    fi
+
 if test ! -d [$JMH_HOME]; then
   AC_MSG_RESULT([no, error])
   AC_MSG_ERROR([$JMH_HOME does not exist or is not a directory])
-

Could you please review it?
Thanks a lot.

Best regards,
Jie

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Kim Barrett]

> On Apr 4, 2019, at 6:03 PM, Erik Joelsson  wrote:
> 
> 
> On 2019-04-04 14:26, Kim Barrett wrote:
>> 
>> OK, I can do that.
>> 
>> --
>> src/hotspot/share/utilities/macros.hpp
>>  645 #if FILE_MACRO_OFFSET
>>  646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
>>  647 #else
>>  648 #define THIS_FILE __FILE__
>>  649 #endif
>> 
>> Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
>> an implicit test for "defined"?
>> 
>> If the former, e.g. we're assuming it will always be defined but might
>> have a 0 value, then I'd skip it and just unconditionally define
>> THIS_FILE as (__FILE__ + FILE_MACRO_OFFSET).
> 
> Right, that makes sense. I was sort of hedging for all possibilities here, 
> but as the build logic is currently structured, it will always be defined, 
> just sometimes 0.
> 
> New webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.02/

src/hotspot changes look good.

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Erik Joelsson]

On 2019-04-04 14:26, Kim Barrett wrote:


OK, I can do that.

--
src/hotspot/share/utilities/macros.hpp
  645 #if FILE_MACRO_OFFSET
  646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
  647 #else
  648 #define THIS_FILE __FILE__
  649 #endif

Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
an implicit test for "defined"?

If the former, e.g. we're assuming it will always be defined but might
have a 0 value, then I'd skip it and just unconditionally define
THIS_FILE as (__FILE__ + FILE_MACRO_OFFSET).


Right, that makes sense. I was sort of hedging for all possibilities 
here, but as the build logic is currently structured, it will always be 
defined, just sometimes 0.


New webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.02/

/Erik


If the latter, some compilers will (with some warning levels or
options, such as gcc -Wundef) complain about the (specified by the
standard) implicit conversion to 0 for an unrecognized identifier in
an #if expression, and an #ifdef should be used to protect against
that.

--

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Kim Barrett]

> On Apr 4, 2019, at 9:36 AM, Erik Joelsson  wrote:
> 
> Hello Kim,
> 
> On 2019-04-03 16:34, Kim Barrett wrote:
>>> On Apr 3, 2019, at 9:51 AM, Erik Joelsson  
>>> wrote:Additionally, I would like to replace all (or at least most) 
>>> instances of __FILE__ with my new THIS_FILE, and I suspect some other 
>>> places would be more performance sensitive. Do you see any problem with 
>>> doing that substitution?
>> Given that our build system is mostly unhappy with basename collisions
>> (other than open/custom overrides), I don't think there's much of a
>> usage problem with using a basename-like approach everywhere either.
> 
> It's also allowed, though perhaps not used, to override less specific files 
> with more specific, as in OS specific file foo.cpp would override shared 
> foo.cpp.

I didn’t know that.  I don’t think that feature is being used, though could be 
mistaken.

> I won't update the other references in this patch. I will see if anyone else 
> thinks it's a good enough idea to give it a go. That kind of change would not 
> affect the build, nor the full paths in binaries, just (hopefully) improve 
> readability of error/log messages.

OK.

>> While researching what various platforms do in this area, I ran across
>> a suggestion that debug builds should maybe use full paths and release
>> builds use shortened paths.  That seems like an interesting idea, though
>> we still want short paths in the exceptions.hpp case.
> If you really think this is worth it, I could implement it, but I would 
> rather keep it the same if possible.

Not sure.  The point would be to provide full paths in builds that developers 
are
using, but not expose full paths in shipped binaries.  But I don’t have a 
strong opinion.

 This doesn't eliminate the full pathname string embedded in the
 executable, but I don't think the proposed FILE_MACRO_OFFSET will do
 that either.  Those get fixed by -fmacro-prefix-map=.
>>> Correct. While solving this universally would be nice, it wasn't the goal 
>>> of this bug. The main goal was to get precompiled headers to work with GCC 
>>> again.
>> I agree that either approach does that.  I looked at the webrev and
>> it's a bunch of build system changes that I don't feel qualified to
>> review, so I suggested an alternative that I understand.  We each have
>> our preferred tools.  I don't object to the build-system-based
>> approach, just can't give an actual thumbs-up.
> 
> Right, and I appreciate the input! But could you at least review the hotspot 
> part, so I can get someone else to review the build part?

OK, I can do that.

--
src/hotspot/share/utilities/macros.hpp
 645 #if FILE_MACRO_OFFSET
 646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
 647 #else
 648 #define THIS_FILE __FILE__
 649 #endif

Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
an implicit test for "defined"?

If the former, e.g. we're assuming it will always be defined but might
have a 0 value, then I'd skip it and just unconditionally define
THIS_FILE as (__FILE__ + FILE_MACRO_OFFSET).

If the latter, some compilers will (with some warning levels or
options, such as gcc -Wundef) complain about the (specified by the
standard) implicit conversion to 0 for an unrecognized identifier in
an #if expression, and an #ifdef should be used to protect against
that.

--

Re: RFR: JDK-8221996: Bootcycle build broken

[Tim Bell]

Erik:

Looks good.

Tim

On 04/04/19 12:56, Erik Joelsson wrote:

I found the issue. It was broken before too, but the old piping
construct just ignored the error (but it was still printed). Fixed by
making sure the bootcycle-build directory exists before calling the
bootcycle build.

Bug: https://bugs.openjdk.java.net/browse/JDK-8221996

Webrev: http://cr.openjdk.java.net/~erikj/8221996/webrev.01/

/Erik

On 2019-04-04 11:40, Erik Joelsson wrote:

It's not just you. I see it in our CI as well. It was caused by my
change https://bugs.openjdk.java.net/browse/JDK-8221764 Will look into
it.

/Erik

On 2019-04-04 11:35, Andrew Haley wrote:

This is jdk-jdk, 54423:6c0ab8bd8da5

Building JVM variant 'server' with features 'aot cds cmsgc compiler1
compiler2 dtrace epsilongc g1gc graal jfr jni-check jvmci jvmti
management nmt parallelgc serialgc services shenandoahgc vm-structs'
Boot cycle build step 2: Building a new JDK image using previously
built image
/usr/bin/tee:
/home/aph/jdk-jdk/build/linux-aarch64-server-slowdebug/bootcycle-build/build.log:
No such file or directory
Building target 'product-images' in configuration
'linux-aarch64-server-slowdebug'
gmake[3]: *** [main] Error 1
gmake[2]: *** [bootcycle-images] Error 2

Is this just me? Has anyone else seen it?

RFR: JDK-8221996: Bootcycle build broken

[Erik Joelsson]

I found the issue. It was broken before too, but the old piping 
construct just ignored the error (but it was still printed). Fixed by 
making sure the bootcycle-build directory exists before calling the 
bootcycle build.


Bug: https://bugs.openjdk.java.net/browse/JDK-8221996

Webrev: http://cr.openjdk.java.net/~erikj/8221996/webrev.01/

/Erik

On 2019-04-04 11:40, Erik Joelsson wrote:
It's not just you. I see it in our CI as well. It was caused by my 
change https://bugs.openjdk.java.net/browse/JDK-8221764 Will look into 
it.


/Erik

On 2019-04-04 11:35, Andrew Haley wrote:

This is jdk-jdk, 54423:6c0ab8bd8da5

Building JVM variant 'server' with features 'aot cds cmsgc compiler1 
compiler2 dtrace epsilongc g1gc graal jfr jni-check jvmci jvmti 
management nmt parallelgc serialgc services shenandoahgc vm-structs'
Boot cycle build step 2: Building a new JDK image using previously 
built image
/usr/bin/tee: 
/home/aph/jdk-jdk/build/linux-aarch64-server-slowdebug/bootcycle-build/build.log: 
No such file or directory
Building target 'product-images' in configuration 
'linux-aarch64-server-slowdebug'

gmake[3]: *** [main] Error 1
gmake[2]: *** [bootcycle-images] Error 2

Is this just me? Has anyone else seen it?

Re: RFR (S): 8221880: Better customization for Windows RC properties FileDescription and ProductName

[Erik Joelsson]

This looks good to me and should work continue to work for us.

/Erik

On 2019-04-04 08:36, Langer, Christoph wrote:

Hi Erik,


Good. Then we are back at my latest webrev:

http://cr.openjdk.java.net/~clanger/webrevs/8221880.1/

Ah, right, this change does not remove JDK_RC_PLATFORM_NAME from
version-numbers, but it does remove it from spec.gmk.in. Could you leave
it in spec.gmk.in? Then you can commit this at your own time.

Then we'd go like this: http://cr.openjdk.java.net/~clanger/webrevs/8221880.2/

Using "\$(JDK_RC_NAME)" instead of "\$(PRODUCT_NAME) \$(JDK_RC_PLATFORM_NAME)" 
in flags-other.m4 is alright then?

/Christoph

Re: bootcycle-images fail

[Erik Joelsson]

It's not just you. I see it in our CI as well. It was caused by my 
change https://bugs.openjdk.java.net/browse/JDK-8221764 Will look into it.


/Erik

On 2019-04-04 11:35, Andrew Haley wrote:

This is jdk-jdk, 54423:6c0ab8bd8da5

Building JVM variant 'server' with features 'aot cds cmsgc compiler1 compiler2 
dtrace epsilongc g1gc graal jfr jni-check jvmci jvmti management nmt parallelgc 
serialgc services shenandoahgc vm-structs'
Boot cycle build step 2: Building a new JDK image using previously built image
/usr/bin/tee: 
/home/aph/jdk-jdk/build/linux-aarch64-server-slowdebug/bootcycle-build/build.log:
 No such file or directory
Building target 'product-images' in configuration 
'linux-aarch64-server-slowdebug'
gmake[3]: *** [main] Error 1
gmake[2]: *** [bootcycle-images] Error 2

Is this just me? Has anyone else seen it?

bootcycle-images fail

[Andrew Haley]

This is jdk-jdk, 54423:6c0ab8bd8da5

Building JVM variant 'server' with features 'aot cds cmsgc compiler1 compiler2 
dtrace epsilongc g1gc graal jfr jni-check jvmci jvmti management nmt parallelgc 
serialgc services shenandoahgc vm-structs'
Boot cycle build step 2: Building a new JDK image using previously built image
/usr/bin/tee: 
/home/aph/jdk-jdk/build/linux-aarch64-server-slowdebug/bootcycle-build/build.log:
 No such file or directory
Building target 'product-images' in configuration 
'linux-aarch64-server-slowdebug'
gmake[3]: *** [main] Error 1
gmake[2]: *** [bootcycle-images] Error 2

Is this just me? Has anyone else seen it?

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

RE: RFR (S): 8221880: Better customization for Windows RC properties FileDescription and ProductName

[Langer, Christoph]

Hi Erik,

> > Good. Then we are back at my latest webrev:
> http://cr.openjdk.java.net/~clanger/webrevs/8221880.1/
> 
> Ah, right, this change does not remove JDK_RC_PLATFORM_NAME from
> version-numbers, but it does remove it from spec.gmk.in. Could you leave
> it in spec.gmk.in? Then you can commit this at your own time.

Then we'd go like this: http://cr.openjdk.java.net/~clanger/webrevs/8221880.2/ 

Using "\$(JDK_RC_NAME)" instead of "\$(PRODUCT_NAME) \$(JDK_RC_PLATFORM_NAME)" 
in flags-other.m4 is alright then?

/Christoph

Re: RFR: 8221894: Add comments for docker tests in the test doc

[Jie Fu]

Hi Erik,

Thank you so much for your review and suggestions.

Hello Jie,

Looks good, just some grammatical notes on the first paragraph. Here 
is my suggestion:


Docker tests with default parameters may fail on OS versions newer 
than oraclelinux 7.6.
For example, they pass on Ubuntu 16.04 but fail on Ubuntu 18.04 if run 
like this:


OK. I will update it next Monday since tomorrow is a public holiday in 
China.

Thanks.

Best regards,
Jie

Re: RFR: 8221907: make reconfigure broken with "build/jmh/jars does not exist or is not a directory"

[Jie Fu]

Thanks Erik for fixing this issue.


On 2019年04月04日 21:48, Erik Joelsson wrote:

Hello David,

On 2019-04-03 18:49, David Holmes wrote:

Hi Erik,

On 4/04/2019 1:33 am, Erik Joelsson wrote:

Hello Jie,

This issue applies not only to --with-jmh, but to any configure 
parameter given with a relative path. I think the proper fix would 
be to record the current working directory when configure is 
launched and cd to that directory when running reconfigure. Here is 
my suggested patch:


http://cr.openjdk.java.net/~erikj/8221907/webrev.01/index.html

The relevant parts are exporting the variable from configure and 
using it in Init.gmk. The rest is just renaming the variable since 
CURDIR would clash with the pre defined make variable CURDIR.


I see how the change fixes the issue with existing relative paths, 
but it indicates that OUTPUTDIR is different to CONFIGURE_START_DIR - 
so what happens with generated output now we have a different cwd? Is 
it all controlled by absolute paths and so will still go to the 
place(s) regardless?


All output is controlled with absolute paths yes. One of the first 
things that happens when configure is run (in basics.m4) is figuring 
out which dir is the workspace root (TOPDIR), which is the OUTPUTDIR 
etc. From then on, everything is defined as absolute paths using those 
basic variables. Today you can run configure either from the root or 
from the output dir. With this change, any reconfigure will match the 
original cwd, so should get the same behavior as the original run of 
configure (which is the idea of reconfigure).


/Erik


Thanks,
David


/Erik

On 2019-04-03 05:28, Jie Fu wrote:

Hi all,

JBS: https://bugs.openjdk.java.net/browse/JDK-8221907

For more info (e.g. the symptom & how to reproduce), please see the 
JBS.


It can be fixed by
-
diff -r 3326be37cd9a make/autoconf/lib-tests.m4
--- a/make/autoconf/lib-tests.m4Tue Apr 02 17:27:48 2019 -0700
+++ b/make/autoconf/lib-tests.m4Wed Apr 03 19:56:24 2019 +0800
@@ -73,6 +73,10 @@
   else
 # Path specified
 JMH_HOME="$with_jmh"
+if test "x${JMH_HOME:0:1}" != x/; then
+  JMH_HOME="$TOPDIR/$JMH_HOME"
+fi
+
 if test ! -d [$JMH_HOME]; then
   AC_MSG_RESULT([no, error])
   AC_MSG_ERROR([$JMH_HOME does not exist or is not a directory])
-

Could you please review it?
Thanks a lot.

Best regards,
Jie

Re: RFR (S): 8221880: Better customization for Windows RC properties FileDescription and ProductName

[Erik Joelsson]

On 2019-04-04 07:15, Langer, Christoph wrote:

Hello Erik,


In OpenJDK builds, the current strings evaluate to "OpenJDK Platform"
and for Oracle builds "Java(TM) Platform SE". It makes me curious as to
what you need to modify the string to?

We want to print "SapMachine" there, see this commit:


https://github.com/SAP/SapMachine/commit/e70a2bcb8a813bfca7adf82590
a4427114af88a6#diff-00a92a44400757c2a70383c1b51ab8fa

I would also be beneficial, if we could add a configure option for

MACOSX_BUNDLE_NAME_BASE and MACOSX_BUNDLE_ID_BASE. We have
a diff there as well:
https://github.com/SAP/SapMachine/commit/d5df3b2c65fd8b833989375886
640433ee9fa0f1#diff-ac0146eb7428c83f99766e2a13ff1b17
This makes me wonder why you don't also want to change the
PRODUCT_NAME
to SapMachine. Wouldn't it make more sense to have it all match? Or are
you worried about compatibility issues if users expect the value
OpenJDK? At least Oracle and OpenJDK builds have different values there
already.

Yes, we are worried about compatibility issues. It starts with (jtreg) tests 
that would fail. To that we could make adaptions, of course. But looking around 
all other downstream vendors stick with OpenJDK so far. So, no reason why we 
should go this route first 


Or are there maybe other hooks to customize these kinds of properties?

No other hooks that I know of. If you need customization, please add
configure parameters. We haven't added any only because nobody has
needed such customization yet.

Ok, I'll see if I come up with configuration parameters for the Mac properties. 
But I'll do it in a different change.


In any case, if it comes to a push of this change, I'd then let you do it to

coordinate with your internal infrastructure 

OK, I can do that if needed, but if we find a solution where it's not,
I'm happy to let you push yourself. :)

Good. Then we are back at my latest webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8221880.1/


Ah, right, this change does not remove JDK_RC_PLATFORM_NAME from 
version-numbers, but it does remove it from spec.gmk.in. Could you leave 
it in spec.gmk.in? Then you can commit this at your own time.


/Erik


Anything you want me to change? Otherwise, please do your preparations and let 
me know when and how this is going to be committed 

Thanks
Christoph

RE: RFR (S): 8221880: Better customization for Windows RC properties FileDescription and ProductName

[Langer, Christoph]

Hello Erik,

> >> In OpenJDK builds, the current strings evaluate to "OpenJDK Platform"
> >> and for Oracle builds "Java(TM) Platform SE". It makes me curious as to
> >> what you need to modify the string to?
> > We want to print "SapMachine" there, see this commit:
> >
> https://github.com/SAP/SapMachine/commit/e70a2bcb8a813bfca7adf82590
> a4427114af88a6#diff-00a92a44400757c2a70383c1b51ab8fa
> >
> > I would also be beneficial, if we could add a configure option for
> MACOSX_BUNDLE_NAME_BASE and MACOSX_BUNDLE_ID_BASE. We have
> a diff there as well:
> >
> https://github.com/SAP/SapMachine/commit/d5df3b2c65fd8b833989375886
> 640433ee9fa0f1#diff-ac0146eb7428c83f99766e2a13ff1b17
> This makes me wonder why you don't also want to change the
> PRODUCT_NAME
> to SapMachine. Wouldn't it make more sense to have it all match? Or are
> you worried about compatibility issues if users expect the value
> OpenJDK? At least Oracle and OpenJDK builds have different values there
> already.

Yes, we are worried about compatibility issues. It starts with (jtreg) tests 
that would fail. To that we could make adaptions, of course. But looking around 
all other downstream vendors stick with OpenJDK so far. So, no reason why we 
should go this route first 

> > Or are there maybe other hooks to customize these kinds of properties?
> No other hooks that I know of. If you need customization, please add
> configure parameters. We haven't added any only because nobody has
> needed such customization yet.

Ok, I'll see if I come up with configuration parameters for the Mac properties. 
But I'll do it in a different change.

> > In any case, if it comes to a push of this change, I'd then let you do it to
> coordinate with your internal infrastructure 
> 
> OK, I can do that if needed, but if we find a solution where it's not,
> I'm happy to let you push yourself. :)

Good. Then we are back at my latest webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8221880.1/

Anything you want me to change? Otherwise, please do your preparations and let 
me know when and how this is going to be committed 

Thanks
Christoph

Re: RFR (S): 8221880: Better customization for Windows RC properties FileDescription and ProductName

[Erik Joelsson]

Hello,

On 2019-04-04 00:10, Langer, Christoph wrote:

Hi Erik,


Looking closer at what we are doing, we are actually overriding
JDK_RC_PLATFORM_NAME as well, and there are a couple of direct
references to that variable in our custom makefiles. So I will still
need to update those if this goes in.

Ok, but I think then we can/will keep JDK_RC_PLATFORM_NAME, no?
That would work, I think. We could then consider reworking our internal 
usage on our own time, if we wanted to. That would be the least intrusive.

In OpenJDK builds, the current strings evaluate to "OpenJDK Platform"
and for Oracle builds "Java(TM) Platform SE". It makes me curious as to
what you need to modify the string to?

We want to print "SapMachine" there, see this commit:
https://github.com/SAP/SapMachine/commit/e70a2bcb8a813bfca7adf82590a4427114af88a6#diff-00a92a44400757c2a70383c1b51ab8fa

I would also be beneficial, if we could add a configure option for 
MACOSX_BUNDLE_NAME_BASE and MACOSX_BUNDLE_ID_BASE. We have a diff there as well:
https://github.com/SAP/SapMachine/commit/d5df3b2c65fd8b833989375886640433ee9fa0f1#diff-ac0146eb7428c83f99766e2a13ff1b17
This makes me wonder why you don't also want to change the PRODUCT_NAME 
to SapMachine. Wouldn't it make more sense to have it all match? Or are 
you worried about compatibility issues if users expect the value 
OpenJDK? At least Oracle and OpenJDK builds have different values there 
already.

Or are there maybe other hooks to customize these kinds of properties?
No other hooks that I know of. If you need customization, please add 
configure parameters. We haven't added any only because nobody has 
needed such customization yet.

In any case, if it comes to a push of this change, I'd then let you do it to 
coordinate with your internal infrastructure 


OK, I can do that if needed, but if we find a solution where it's not, 
I'm happy to let you push yourself. :)


/Erik


Thanks
Christoph


On 2019-04-03 14:33, Langer, Christoph wrote:

Hi Erik,

please see this new webrev:

http://cr.openjdk.java.net/~clanger/webrevs/8221880.1/

I would now add a new configure flag --with-jdk-rc-name. By default, it is

unset and JDK_RC_NAME would be set to $PRODUCT_NAME
$JDK_RC_PLATFORM_NAME. I think this change would not create the need
for any modification to current build calls.

One additional point that I was thinking about: Shouldn't we maybe

remove JDK_RC_PLATFORM_NAME from version-numbers at all and hard
code the value "Platform" in make/autoconf/jdk-version.m4?

/Christoph



-Original Message-
From: Langer, Christoph
Sent: Mittwoch, 3. April 2019 16:25
To: 'Erik Joelsson' ; build-

d...@openjdk.java.net

Subject: RE: RFR (S): 8221880: Better customization for Windows RC
properties FileDescription and ProductName

Hi Erik,

thanks for the information. Now I also understand your constraints 

I think I'll then try to come up with some configure flag for setting
JDK_RC_NAME. And I'll see if I can do it in a way that you would not need

to

change something in your internal setup.

/Christoph


-Original Message-
From: Erik Joelsson 
Sent: Mittwoch, 3. April 2019 16:18
To: Langer, Christoph ; build-
d...@openjdk.java.net
Subject: Re: RFR (S): 8221880: Better customization for Windows RC
properties FileDescription and ProductName

Hello Christoph,

I understand your problem, but a complicating factor here is that the
version-numbers file is currently formatted as a properties file and we
do consume it as such in other places. While we don't specifically look
for this property there, I think it sets a bad precedent if we let it
become a shell script instead. Could you find a solution without
variable references in version-numbers?

We do override PRODUCT_NAME for our builds, but we do not do it by
patching the version-numbers file. We do it through the custom

extension

hooks in configure. With your change here, that would no longer work
unless we override this new JDK_RC_NAME variable explicitly.

I guess I would be OK with JDK_RC_NAME="OpenJDK Platform", but we

will

need a corresponding internal fix very quickly, so please keep me
updated when such a change is pushed.

/Erik

On 2019-04-03 01:06, Langer, Christoph wrote:

Hi,

In our downstream build, I'd like to be able to set/customize the value

for

the Windows RC properties "ProductName" and "FileDescription" via the
version-numbers file. These values manifest in Windows executable
properties.

During the build ProductName gets set to "OpenJDK Platform 13" and

FileDescription will be "OpenJDK Platform binary". This value is obtained

by

concatenating \$(PRODUCT_NAME) \$(JDK_RC_PLATFORM_NAME) in

flags-

other.m4. Both variables get set in version-numbers. So, if I was to

customize

the properties, I could change PRODUCT_NAME and
JDK_RC_PLATFORM_NAME in version-numbers. However, modifying

the

former is no good idea since it is used ubiquitously and has unwanted

side

effects. On the other hand, I could make an 

Re: RFR: 8221907: make reconfigure broken with "build/jmh/jars does not exist or is not a directory"

[Erik Joelsson]

Hello David,

On 2019-04-03 18:49, David Holmes wrote:

Hi Erik,

On 4/04/2019 1:33 am, Erik Joelsson wrote:

Hello Jie,

This issue applies not only to --with-jmh, but to any configure 
parameter given with a relative path. I think the proper fix would be 
to record the current working directory when configure is launched 
and cd to that directory when running reconfigure. Here is my 
suggested patch:


http://cr.openjdk.java.net/~erikj/8221907/webrev.01/index.html

The relevant parts are exporting the variable from configure and 
using it in Init.gmk. The rest is just renaming the variable since 
CURDIR would clash with the pre defined make variable CURDIR.


I see how the change fixes the issue with existing relative paths, but 
it indicates that OUTPUTDIR is different to CONFIGURE_START_DIR - so 
what happens with generated output now we have a different cwd? Is it 
all controlled by absolute paths and so will still go to the place(s) 
regardless?


All output is controlled with absolute paths yes. One of the first 
things that happens when configure is run (in basics.m4) is figuring out 
which dir is the workspace root (TOPDIR), which is the OUTPUTDIR etc. 
From then on, everything is defined as absolute paths using those basic 
variables. Today you can run configure either from the root or from the 
output dir. With this change, any reconfigure will match the original 
cwd, so should get the same behavior as the original run of configure 
(which is the idea of reconfigure).


/Erik


Thanks,
David


/Erik

On 2019-04-03 05:28, Jie Fu wrote:

Hi all,

JBS: https://bugs.openjdk.java.net/browse/JDK-8221907

For more info (e.g. the symptom & how to reproduce), please see the 
JBS.


It can be fixed by
-
diff -r 3326be37cd9a make/autoconf/lib-tests.m4
--- a/make/autoconf/lib-tests.m4    Tue Apr 02 17:27:48 2019 -0700
+++ b/make/autoconf/lib-tests.m4    Wed Apr 03 19:56:24 2019 +0800
@@ -73,6 +73,10 @@
   else
 # Path specified
 JMH_HOME="$with_jmh"
+    if test "x${JMH_HOME:0:1}" != x/; then
+  JMH_HOME="$TOPDIR/$JMH_HOME"
+    fi
+
 if test ! -d [$JMH_HOME]; then
   AC_MSG_RESULT([no, error])
   AC_MSG_ERROR([$JMH_HOME does not exist or is not a directory])
-

Could you please review it?
Thanks a lot.

Best regards,
Jie

Re: RFR: 8221894: Add comments for docker tests in the test doc

[Erik Joelsson]

Hello Jie,

Looks good, just some grammatical notes on the first paragraph. Here is 
my suggestion:


Docker tests with default parameters may fail on OS versions newer than 
oraclelinux 7.6.
For example, they pass on Ubuntu 16.04 but fail on Ubuntu 18.04 if run 
like this:


/Erik

On 2019-04-03 20:10, Jie Fu wrote:


Hi Erik,

Thank you for your review.


Hello Jie,

I think this kind of information would fit better under its own new 
heading to make it more explicit. Then we could continue filling in 
other similar notes for other tests there. At the bottom, something 
like:


## Notes for Specific Tests

### Docker Tests




What do you think?


Very good suggestions.
Here is the updated version: 
http://cr.openjdk.java.net/~jiefu/8221894/webrev.01/

Please review.
Thanks a lot.

Best regards,
Jie

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Erik Joelsson]

Hello Kim,

On 2019-04-03 16:34, Kim Barrett wrote:

On Apr 3, 2019, at 9:51 AM, Erik Joelsson  wrote:
On 2019-04-02 16:02, Kim Barrett wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8221851

Webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.01/index.html

/Erik

Here's an alternative approach that seems simpler. […]

This kind of solution will also work. It's worth noting that your fix will work 
like basename and just print the filename (as the current state in hotspot does 
for exceptions if compiled with GCC). I think printing the complete relative 
path to the workspace root is an improvement over this, and I think the only 
reason it's currently printing just the filename is that that's what appeared 
available as an alternative. The issue with truncated paths is mostly happening 
on our internal Mach5 builds where the workspace root is usually located in an 
extremely deep path hierarchy.

I disagree that it's mostly a problem on our internal Mach5 builds.
There are paths in my development repos that exceed the clipped size
in the examples in that bug, and I don't think I have a very deep
hierarchy. Using a workspace-relative path in exception messages might
be enough to still address JDK-8204551 though.

Right, even more reason to find a solution then.

Additionally, I would like to replace all (or at least most) instances of 
__FILE__ with my new THIS_FILE, and I suspect some other places would be more 
performance sensitive. Do you see any problem with doing that substitution?

Given that our build system is mostly unhappy with basename collisions
(other than open/custom overrides), I don't think there's much of a
usage problem with using a basename-like approach everywhere either.


It's also allowed, though perhaps not used, to override less specific 
files with more specific, as in OS specific file foo.cpp would override 
shared foo.cpp.


I won't update the other references in this patch. I will see if anyone 
else thinks it's a good enough idea to give it a go. That kind of change 
would not affect the build, nor the full paths in binaries, just 
(hopefully) improve readability of error/log messages.



While researching what various platforms do in this area, I ran across
a suggestion that debug builds should maybe use full paths and release
builds use shortened paths.  That seems like an interesting idea, though
we still want short paths in the exceptions.hpp case.
If you really think this is worth it, I could implement it, but I would 
rather keep it the same if possible.

I tend to expect (modern) compilers to be pretty good at obvious
optimizations, though admit I'm occasionally sadly disappointed.


This doesn't eliminate the full pathname string embedded in the
executable, but I don't think the proposed FILE_MACRO_OFFSET will do
that either.  Those get fixed by -fmacro-prefix-map=.

Correct. While solving this universally would be nice, it wasn't the goal of 
this bug. The main goal was to get precompiled headers to work with GCC again.

I agree that either approach does that.  I looked at the webrev and
it's a bunch of build system changes that I don't feel qualified to
review, so I suggested an alternative that I understand.  We each have
our preferred tools.  I don't object to the build-system-based
approach, just can't give an actual thumbs-up.


Right, and I appreciate the input! But could you at least review the 
hotspot part, so I can get someone else to review the build part?


/Erik

Re: RFR: JDK-8221766: Load-reference barriers for Shenandoah

[Roman Kennke]

The main difference is that instead of ensuring correct invariant when
we store anything into the heap (e.g. read-barrier before reads,
write-barrier before writes, plus a bunch of other stuff), we ensure the
strong invariance on objects when they get loaded, by employing what is
currently our write-barrier.


OK, so how does this work? Sure, the OOP load promotes an object to
tospace, but how do you ensure that the OOP doesn't become stale when
a later phase occurs?


Whenever we start an evacuation phase, we pre-evacuate everything that's 
referenced by stack or registers, and update those stack slots and 
registers. (We did that with the old barrier-scheme too.)


Roman

Re: RFR: JDK-8221766: Load-reference barriers for Shenandoah

[Andrew Haley]

On 4/2/19 10:12 PM, Roman Kennke wrote:
> The main difference is that instead of ensuring correct invariant when
> we store anything into the heap (e.g. read-barrier before reads,
> write-barrier before writes, plus a bunch of other stuff), we ensure the
> strong invariance on objects when they get loaded, by employing what is
> currently our write-barrier.

OK, so how does this work? Sure, the OOP load promotes an object to
tospace, but how do you ensure that the OOP doesn't become stale when
a later phase occurs?

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

RE: RFR (S): 8221880: Better customization for Windows RC properties FileDescription and ProductName

[Langer, Christoph]

Hi Erik,

> Looking closer at what we are doing, we are actually overriding
> JDK_RC_PLATFORM_NAME as well, and there are a couple of direct
> references to that variable in our custom makefiles. So I will still
> need to update those if this goes in.

Ok, but I think then we can/will keep JDK_RC_PLATFORM_NAME, no?

> In OpenJDK builds, the current strings evaluate to "OpenJDK Platform"
> and for Oracle builds "Java(TM) Platform SE". It makes me curious as to
> what you need to modify the string to?

We want to print "SapMachine" there, see this commit:
https://github.com/SAP/SapMachine/commit/e70a2bcb8a813bfca7adf82590a4427114af88a6#diff-00a92a44400757c2a70383c1b51ab8fa

I would also be beneficial, if we could add a configure option for 
MACOSX_BUNDLE_NAME_BASE and MACOSX_BUNDLE_ID_BASE. We have a diff there as well:
https://github.com/SAP/SapMachine/commit/d5df3b2c65fd8b833989375886640433ee9fa0f1#diff-ac0146eb7428c83f99766e2a13ff1b17

Or are there maybe other hooks to customize these kinds of properties?

In any case, if it comes to a push of this change, I'd then let you do it to 
coordinate with your internal infrastructure 

Thanks
Christoph

> On 2019-04-03 14:33, Langer, Christoph wrote:
> > Hi Erik,
> >
> > please see this new webrev:
> http://cr.openjdk.java.net/~clanger/webrevs/8221880.1/
> >
> > I would now add a new configure flag --with-jdk-rc-name. By default, it is
> unset and JDK_RC_NAME would be set to $PRODUCT_NAME
> $JDK_RC_PLATFORM_NAME. I think this change would not create the need
> for any modification to current build calls.
> >
> > One additional point that I was thinking about: Shouldn't we maybe
> remove JDK_RC_PLATFORM_NAME from version-numbers at all and hard
> code the value "Platform" in make/autoconf/jdk-version.m4?
> >
> > /Christoph
> >
> >
> >> -Original Message-
> >> From: Langer, Christoph
> >> Sent: Mittwoch, 3. April 2019 16:25
> >> To: 'Erik Joelsson' ; build-
> d...@openjdk.java.net
> >> Subject: RE: RFR (S): 8221880: Better customization for Windows RC
> >> properties FileDescription and ProductName
> >>
> >> Hi Erik,
> >>
> >> thanks for the information. Now I also understand your constraints 
> >>
> >> I think I'll then try to come up with some configure flag for setting
> >> JDK_RC_NAME. And I'll see if I can do it in a way that you would not need
> to
> >> change something in your internal setup.
> >>
> >> /Christoph
> >>
> >>> -Original Message-
> >>> From: Erik Joelsson 
> >>> Sent: Mittwoch, 3. April 2019 16:18
> >>> To: Langer, Christoph ; build-
> >>> d...@openjdk.java.net
> >>> Subject: Re: RFR (S): 8221880: Better customization for Windows RC
> >>> properties FileDescription and ProductName
> >>>
> >>> Hello Christoph,
> >>>
> >>> I understand your problem, but a complicating factor here is that the
> >>> version-numbers file is currently formatted as a properties file and we
> >>> do consume it as such in other places. While we don't specifically look
> >>> for this property there, I think it sets a bad precedent if we let it
> >>> become a shell script instead. Could you find a solution without
> >>> variable references in version-numbers?
> >>>
> >>> We do override PRODUCT_NAME for our builds, but we do not do it by
> >>> patching the version-numbers file. We do it through the custom
> extension
> >>> hooks in configure. With your change here, that would no longer work
> >>> unless we override this new JDK_RC_NAME variable explicitly.
> >>>
> >>> I guess I would be OK with JDK_RC_NAME="OpenJDK Platform", but we
> >> will
> >>> need a corresponding internal fix very quickly, so please keep me
> >>> updated when such a change is pushed.
> >>>
> >>> /Erik
> >>>
> >>> On 2019-04-03 01:06, Langer, Christoph wrote:
>  Hi,
> 
>  In our downstream build, I'd like to be able to set/customize the value
> for
> >>> the Windows RC properties "ProductName" and "FileDescription" via the
> >>> version-numbers file. These values manifest in Windows executable
> >>> properties.
>  During the build ProductName gets set to "OpenJDK Platform 13" and
> >>> FileDescription will be "OpenJDK Platform binary". This value is obtained
> by
> >>> concatenating \$(PRODUCT_NAME) \$(JDK_RC_PLATFORM_NAME) in
> >> flags-
> >>> other.m4. Both variables get set in version-numbers. So, if I was to
> >> customize
> >>> the properties, I could change PRODUCT_NAME and
> >>> JDK_RC_PLATFORM_NAME in version-numbers. However, modifying
> the
> >>> former is no good idea since it is used ubiquitously and has unwanted
> side
> >>> effects. On the other hand, I could make an adaption to flags-other.m4,
> but
> >>> that diff would be hidden and not in a central place where I'd expect
> such
> >>> customizing diffs.
>  So, please review this small fix, which allows for modifying these RC
> >>> properties in version-numbers. The default behavior won't be changed.
>  Bug: https://bugs.openjdk.java.net/browse/JDK-8221880
>  Webrev: 

Re: RFR: 8221907: make reconfigure broken with "build/jmh/jars does not exist or is not a directory"

[Jie Fu]

Hi Eric and David,

Thank you for your review and suggestions.

Now I think it's not a good practice to assign a configure parameter 
with a relative path.
I had assigned "--with-jmh=build/jmh/jars" just because the doc[1] told 
me to do so.


Even "make configure" would break with a relative path in a particular 
case (e.g., in a symbolic link topdir).


So I will prefer absolute paths for configure parameters (except 
--with-jmh).

Thanks.

Best regards,
Jie

[1] http://hg.openjdk.java.net/jdk/jdk/file/5c7418757bad/doc/testing.md#l44


On 2019/4/4 上午9:49, David Holmes wrote:

Hi Erik,

On 4/04/2019 1:33 am, Erik Joelsson wrote:

Hello Jie,

This issue applies not only to --with-jmh, but to any configure 
parameter given with a relative path. I think the proper fix would be 
to record the current working directory when configure is launched 
and cd to that directory when running reconfigure. Here is my 
suggested patch:


http://cr.openjdk.java.net/~erikj/8221907/webrev.01/index.html

The relevant parts are exporting the variable from configure and 
using it in Init.gmk. The rest is just renaming the variable since 
CURDIR would clash with the pre defined make variable CURDIR.


I see how the change fixes the issue with existing relative paths, but 
it indicates that OUTPUTDIR is different to CONFIGURE_START_DIR - so 
what happens with generated output now we have a different cwd? Is it 
all controlled by absolute paths and so will still go to the place(s) 
regardless?


Thanks,
David


/Erik

On 2019-04-03 05:28, Jie Fu wrote:

Hi all,

JBS: https://bugs.openjdk.java.net/browse/JDK-8221907

For more info (e.g. the symptom & how to reproduce), please see the 
JBS.


It can be fixed by
-
diff -r 3326be37cd9a make/autoconf/lib-tests.m4
--- a/make/autoconf/lib-tests.m4    Tue Apr 02 17:27:48 2019 -0700
+++ b/make/autoconf/lib-tests.m4    Wed Apr 03 19:56:24 2019 +0800
@@ -73,6 +73,10 @@
   else
 # Path specified
 JMH_HOME="$with_jmh"
+    if test "x${JMH_HOME:0:1}" != x/; then
+  JMH_HOME="$TOPDIR/$JMH_HOME"
+    fi
+
 if test ! -d [$JMH_HOME]; then
   AC_MSG_RESULT([no, error])
   AC_MSG_ERROR([$JMH_HOME does not exist or is not a directory])
-

Could you please review it?
Thanks a lot.

Best regards,
Jie