Re: RFR 8030011: Update Hotspot version string output
On 11/04/2014 1:42 AM, Alejandro E Murillo wrote: Hi David, thanks for the feedback, see below On 4/9/2014 8:38 PM, David Holmes wrote: Hi Alejandro, Given we have to maintain the JDK version information in two places (top level repo and hotspot repo) wouldn't it have been simpler to keep hotspot_version file and HOTSPOT_RELEASE_VERSION and simply set the major/minor/build values to match those of the JDK version? The file is still used, just renamed as only jdk info is in there. HOTSPOT_RELEASE_VERSION was also kept, we just don't get the major,minor,micro and build number from it anymore, mainly it is now set very similar to the JDK_RELEASE_VERSION (plus other values), and that format it's not fixed as it used to be for hotspot express. Those values (major,minor,micro and build number) are already defined in the makefiles, so we just pass them to vm_version.cpp, so no more parsing in it. I still think the overall changes could have been much smaller but okay. Thanks, David - That aside, in jdk_version file hotspot copyright should now be 2014 will fix that */vm.make: This line is way too long. ! VM_VER_DEFS = -DHOTSPOT_RELEASE_VERSION="\"$(HS_BUILD_VER)\"" -DJRE_RELEASE_VERSION="\"$(JRE_RELEASE_VER)\"" -DJDK_MAJOR_VERSION="\"$(JDK_MAJOR_VERSION)\"" -DJDK_MINOR_VERSION="\"$(JDK_MINOR_VERSION)\"" -DJDK_MICRO_VERSION="\"$(JDK_MICRO_VERSION)\"" -DJDK_BUILD_NUMBER="\"$(JDK_BUILD_NUMBER)\"" ok Not clear why we suddenly need defines for all the component pieces either. You could have kept the HS_XXX variables, just adding the micro part. as mentioned above, the micro part was actually added to HOTSPOT_RELEASE_VERSION, we just don't get those values by parsing it, so we just pass those values to the vm_version.cpp, since they are already defined in the makefiles. The format of the JDK version is not that fixed. Thanks Alejandro David On 10/04/2014 10:15 AM, Alejandro E Murillo wrote: Please review this change to make the hotspot related output produced by "java -version" match the corresponding JDK output: webrev: http://cr.openjdk.java.net/~amurillo/9/8030011/ Bug: https://bugs.openjdk.java.net/browse/JDK-8030011 Note that we initially wanted to obtain more information from the repo being built and add it to the hotspot version output, but that will require changes in the JPRT, so we have decided to split the change in 2 bugs. One to make the version match (above webrev) and another one, an RFE, to add additional information extracted from the repo. Note that in the current version of vm_version.cpp, there is no error checking in product mode, I was suggested to make that explicit. Release Engineering did some testing and I also ran several cases with full and just hotspot JPRT builds. Here is a summary of how the new output compares to the old one: (1) Release Engineering builds (9-dev): from jdk9-dev build: java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00) Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00, mixed mode) compared with what we currently have java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h247-20140326-b06-b00) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (2) Release Engineering builds (jdk9): java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) Server VM (build 1.9.0-ea-b07, mixed mode) compared with what we currently have java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (3) JPRT Full builds: java version "1.9.0-internal" # Java(TM) SE Runtime Environment (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00, mixed mode) (4) JPRT hotspot only builds: java version "1.9.0-ea-fastdebug" # Java(TM) SE Runtime Environment (build 1.9.0-ea-fastdebug-b06) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404031820.amurillo.jdk9-hs-ver-str-HS-fastdebug, mixed mode) in this one the built VM is inserted into a promoted build bundle, since we do not have the JDK build number info in the hotspot repo, we can't match the build number in the JDK portion. With the RFE mentioned above, we can extract the build info from the repo and add it to the hotspot portion. I want to mention, that this may change once the new JDK version change is implemented but we don't know when that will be implemented yet, so we need to go ahead with this to get rid of the old hotspot express output. And most of these changes will still have to be done anyways BTW, john Coomes and Dan Daugherty provided feedback in some pieces of this webrev, but I need full reviews now. Thanks
Re: RFR: 8039411 : Add environment variable support to fixpath
On Apr 8 2014, at 23:26 , Erik Joelsson wrote: > Indentation look weird around line 460 and 470 (tab vs space?), otherwise > it's ok to me. Corrected before push. Some text editor I was using was allowing tabs to sneak in. Mike > > /Erik > > On 2014-04-08 21:31, Mike Duigou wrote: >> I have made the changes Tim suggested along with other cleanups, additional >> diagnostics and refinements. >> >> http://cr.openjdk.java.net/~mduigou/JDK-8039411/1/webrev/ >> >> I have not tested this change on mingw/msys and probably won't attempt to >> unless it is required. >> >> Mike >> >> >> On Apr 8 2014, at 08:10 , Tim Bell wrote: >> >>> Hi Mike >>> >>> Looks good - one thing to pick on is line 402 - I'd like to see what was in >>> var if the setting fails. >>> >>> Tim >>> >>> On 04/08/14 08:35, Erik Joelsson wrote: Hello Mike, My C is a bit rusty, but I think it looks good in general. If you are able to test it on mingw/msys I think that would be good since it's a pretty big change. /Erik On 2014-04-07 23:53, Mike Duigou wrote: > Hello all; > > Currently the fixpath utility used in windows builds expects that the > first parameter it is passed will be the path of the executable. In some > cases it's desirable to define environment variables which will apply > during the execution of that executable. This change adds support for > defining environment variables. The variables appear before the > executable. Currently the command line parsing assumes that all arguments > containing "=" before the command path are environment variables. (This > precludes the executable having '=' in it's path, which is unlikely > anyway). > > The remainder of the changes were lint warnings suggested by Visual C. > (mostly const) > > https://bugs.openjdk.java.net/browse/JDK-8039411 > http://cr.openjdk.java.net/~mduigou/JDK-8039411/0/webrev/ > > Mike >
Re: RFR [9] sequential operation option for hgforest
On 10 Apr 2014, at 20:03, Mike Duigou wrote:
> Looks good to me and useful. (Other than that I had hoped to claim "-s" for
> a future change I planned ;-) (no worries, I will use "-r" for that))
Thanks for the review Mike. If you want to keep ‘-s’ we can come up with
something else, “—np|—non-parallel” ?? Or anything, I’m happy once I can
operate in non parallel mode.
-Chris.
> Mike
>
> On Apr 10 2014, at 08:54 , Chris Hegarty wrote:
>
>> Sometimes I get a little confused/nervous when trying to push/status/in
>> using the hgforest.sh script. The output can be a little confusing as it
>> runs several jobs in parallel.
>>
>> I would like to add an option to support sequential operation of commands.
>> It is off by default. The more nervous of us that push using this script can
>> opt in, if you like!
>>
>> diff --git a/common/bin/hgforest.sh b/common/bin/hgforest.sh
>> --- a/common/bin/hgforest.sh
>> +++ b/common/bin/hgforest.sh
>> @@ -29,6 +29,7 @@
>> status_output="/dev/stdout"
>> qflag="false"
>> vflag="false"
>> +sflag="false"
>> while [ $# -gt 0 ]
>> do
>> case $1 in
>> @@ -43,6 +44,10 @@
>> global_opts="${global_opts} -v"
>> ;;
>>
>> +-s | --sequential )
>> + sflag="true"
>> + ;;
>> +
>>'--' ) # no more options
>> shift; break
>> ;;
>> @@ -63,7 +68,7 @@
>> command_args="$@"
>>
>> usage() {
>> - echo "usage: $0 [-q|--quiet] [-v|--verbose] [--]
>> [commands...]" > ${status_output}
>> + echo "usage: $0 [-q|--quiet] [-v|--verbose] [-s|--sequential] [--]
>> [commands...]" > ${status_output}
>> exit 1
>> }
>>
>> @@ -243,11 +248,15 @@
>> ) 2>&1 | sed -e "s@^@${reponame}: @" > ${status_output}
>>) &
>>
>> -if [ `expr ${n} '%' ${at_a_time}` -eq 0 ] ; then
>> +if [ `expr ${n} '%' ${at_a_time}` -eq 0 -a "${sflag}" = "false" ] ; then
>> sleep 2
>> echo "Waiting 5 secs before spawning next background command." >
>> ${status_output}
>> sleep 3
>>fi
>> +
>> +if [ "${sflag}" = "true" ] ; then
>> +wait
>> +fi
>> done
>> fi
>>
>> -Chris.
>>
>
Re: RFR [9] sequential operation option for hgforest
Looks good to me and useful. (Other than that I had hoped to claim "-s" for a
future change I planned ;-) (no worries, I will use "-r" for that))
Mike
On Apr 10 2014, at 08:54 , Chris Hegarty wrote:
> Sometimes I get a little confused/nervous when trying to push/status/in using
> the hgforest.sh script. The output can be a little confusing as it runs
> several jobs in parallel.
>
> I would like to add an option to support sequential operation of commands. It
> is off by default. The more nervous of us that push using this script can opt
> in, if you like!
>
> diff --git a/common/bin/hgforest.sh b/common/bin/hgforest.sh
> --- a/common/bin/hgforest.sh
> +++ b/common/bin/hgforest.sh
> @@ -29,6 +29,7 @@
> status_output="/dev/stdout"
> qflag="false"
> vflag="false"
> +sflag="false"
> while [ $# -gt 0 ]
> do
> case $1 in
> @@ -43,6 +44,10 @@
> global_opts="${global_opts} -v"
> ;;
>
> +-s | --sequential )
> + sflag="true"
> + ;;
> +
> '--' ) # no more options
> shift; break
> ;;
> @@ -63,7 +68,7 @@
> command_args="$@"
>
> usage() {
> - echo "usage: $0 [-q|--quiet] [-v|--verbose] [--]
> [commands...]" > ${status_output}
> + echo "usage: $0 [-q|--quiet] [-v|--verbose] [-s|--sequential] [--]
> [commands...]" > ${status_output}
> exit 1
> }
>
> @@ -243,11 +248,15 @@
> ) 2>&1 | sed -e "s@^@${reponame}: @" > ${status_output}
> ) &
>
> -if [ `expr ${n} '%' ${at_a_time}` -eq 0 ] ; then
> +if [ `expr ${n} '%' ${at_a_time}` -eq 0 -a "${sflag}" = "false" ] ; then
> sleep 2
> echo "Waiting 5 secs before spawning next background command." >
> ${status_output}
> sleep 3
> fi
> +
> +if [ "${sflag}" = "true" ] ; then
> +wait
> +fi
> done
> fi
>
> -Chris.
>
Re: CFV: Nomination of Tim Bell as the new Build Group Lead
Vote: yes -phil.
RFR [9] sequential operation option for hgforest
Sometimes I get a little confused/nervous when trying to push/status/in using
the hgforest.sh script. The output can be a little confusing as it runs several
jobs in parallel.
I would like to add an option to support sequential operation of commands. It
is off by default. The more nervous of us that push using this script can opt
in, if you like!
diff --git a/common/bin/hgforest.sh b/common/bin/hgforest.sh
--- a/common/bin/hgforest.sh
+++ b/common/bin/hgforest.sh
@@ -29,6 +29,7 @@
status_output="/dev/stdout"
qflag="false"
vflag="false"
+sflag="false"
while [ $# -gt 0 ]
do
case $1 in
@@ -43,6 +44,10 @@
global_opts="${global_opts} -v"
;;
+-s | --sequential )
+ sflag="true"
+ ;;
+
'--' ) # no more options
shift; break
;;
@@ -63,7 +68,7 @@
command_args="$@"
usage() {
- echo "usage: $0 [-q|--quiet] [-v|--verbose] [--]
[commands...]" > ${status_output}
+ echo "usage: $0 [-q|--quiet] [-v|--verbose] [-s|--sequential] [--]
[commands...]" > ${status_output}
exit 1
}
@@ -243,11 +248,15 @@
) 2>&1 | sed -e "s@^@${reponame}: @" > ${status_output}
) &
-if [ `expr ${n} '%' ${at_a_time}` -eq 0 ] ; then
+if [ `expr ${n} '%' ${at_a_time}` -eq 0 -a "${sflag}" = "false" ] ; then
sleep 2
echo "Waiting 5 secs before spawning next background command." >
${status_output}
sleep 3
fi
+
+if [ "${sflag}" = "true" ] ; then
+wait
+fi
done
fi
-Chris.
Re: RFR 8030011: Update Hotspot version string output
Hi David, thanks for the feedback, see below On 4/9/2014 8:38 PM, David Holmes wrote: Hi Alejandro, Given we have to maintain the JDK version information in two places (top level repo and hotspot repo) wouldn't it have been simpler to keep hotspot_version file and HOTSPOT_RELEASE_VERSION and simply set the major/minor/build values to match those of the JDK version? The file is still used, just renamed as only jdk info is in there. HOTSPOT_RELEASE_VERSION was also kept, we just don't get the major,minor,micro and build number from it anymore, mainly it is now set very similar to the JDK_RELEASE_VERSION (plus other values), and that format it's not fixed as it used to be for hotspot express. Those values (major,minor,micro and build number) are already defined in the makefiles, so we just pass them to vm_version.cpp, so no more parsing in it. That aside, in jdk_version file hotspot copyright should now be 2014 will fix that */vm.make: This line is way too long. ! VM_VER_DEFS = -DHOTSPOT_RELEASE_VERSION="\"$(HS_BUILD_VER)\"" -DJRE_RELEASE_VERSION="\"$(JRE_RELEASE_VER)\"" -DJDK_MAJOR_VERSION="\"$(JDK_MAJOR_VERSION)\"" -DJDK_MINOR_VERSION="\"$(JDK_MINOR_VERSION)\"" -DJDK_MICRO_VERSION="\"$(JDK_MICRO_VERSION)\"" -DJDK_BUILD_NUMBER="\"$(JDK_BUILD_NUMBER)\"" ok Not clear why we suddenly need defines for all the component pieces either. You could have kept the HS_XXX variables, just adding the micro part. as mentioned above, the micro part was actually added to HOTSPOT_RELEASE_VERSION, we just don't get those values by parsing it, so we just pass those values to the vm_version.cpp, since they are already defined in the makefiles. The format of the JDK version is not that fixed. Thanks Alejandro David On 10/04/2014 10:15 AM, Alejandro E Murillo wrote: Please review this change to make the hotspot related output produced by "java -version" match the corresponding JDK output: webrev: http://cr.openjdk.java.net/~amurillo/9/8030011/ Bug: https://bugs.openjdk.java.net/browse/JDK-8030011 Note that we initially wanted to obtain more information from the repo being built and add it to the hotspot version output, but that will require changes in the JPRT, so we have decided to split the change in 2 bugs. One to make the version match (above webrev) and another one, an RFE, to add additional information extracted from the repo. Note that in the current version of vm_version.cpp, there is no error checking in product mode, I was suggested to make that explicit. Release Engineering did some testing and I also ran several cases with full and just hotspot JPRT builds. Here is a summary of how the new output compares to the old one: (1) Release Engineering builds (9-dev): from jdk9-dev build: java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00) Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00, mixed mode) compared with what we currently have java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h247-20140326-b06-b00) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (2) Release Engineering builds (jdk9): java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) Server VM (build 1.9.0-ea-b07, mixed mode) compared with what we currently have java version "1.9.0-ea" Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (3) JPRT Full builds: java version "1.9.0-internal" # Java(TM) SE Runtime Environment (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00, mixed mode) (4) JPRT hotspot only builds: java version "1.9.0-ea-fastdebug" # Java(TM) SE Runtime Environment (build 1.9.0-ea-fastdebug-b06) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404031820.amurillo.jdk9-hs-ver-str-HS-fastdebug, mixed mode) in this one the built VM is inserted into a promoted build bundle, since we do not have the JDK build number info in the hotspot repo, we can't match the build number in the JDK portion. With the RFE mentioned above, we can extract the build info from the repo and add it to the hotspot portion. I want to mention, that this may change once the new JDK version change is implemented but we don't know when that will be implemented yet, so we need to go ahead with this to get rid of the old hotspot express output. And most of these changes will still have to be done anyways BTW, john Coomes and Dan Daugherty provided feedback in some pieces of this webrev, but I need full reviews now. Thanks -- Alejandro
