Re: RFR: 8026062 : (s) webrev.ksh: fix bug title web scraping, remove teamware, sac, "open bug" and wxfile support

[Vadim Pakhnushev]

You probably either supply a list file as a parameter, or pipe with '-' 
as a parameter.

At least file name as a parameter works for me, haven't tried '-' with pipe.

Thanks,
Vadim

On 11.10.2013 4:20, Weijun Wang wrote:



On 10/10/13 1:13 PM, Mike Duigou wrote:


On Oct 9 2013, at 22:11 , Weijun Wang wrote:

Some of us still use wxfile to generate a single webrev for code 
changes in multiple repos. Is there another way to do it?


Yes, you can pipe a simple file list to webrev to have it use that 
list of files. I removed wxfile support in part because it's not 
documented anywhere and there are alternatives. If you can confirm 
that the alternatives are adequate I would very much appreciate being 
able to go ahead with removing wxfile support.


I tried to create a file list and pipe it to "webrev.ksh -N -r -1" but 
seems not working. Am I using the wrong options?


Thanks
Max

Re: RFR: 8026062 : (s) webrev.ksh: fix bug title web scraping, remove teamware, sac, "open bug" and wxfile support

[Weijun Wang]

On 10/10/13 1:13 PM, Mike Duigou wrote:


On Oct 9 2013, at 22:11 , Weijun Wang wrote:


Some of us still use wxfile to generate a single webrev for code changes in 
multiple repos. Is there another way to do it?


Yes, you can pipe a simple file list to webrev to have it use that list of 
files. I removed wxfile support in part because it's not documented anywhere 
and there are alternatives. If you can confirm that the alternatives are 
adequate I would very much appreciate being able to go ahead with removing 
wxfile support.


I tried to create a file list and pipe it to "webrev.ksh -N -r -1" but 
seems not working. Am I using the wrong options?


Thanks
Max

Re: JDK7u40: build issue with a french VS2010 Express

[Ludovic HOCHET]

Hello,
Coming back to this:
"But that does not work neither because when one download VS2010 Express
from a French territory, one gets the French version EVEN if the English
version was explicitly requested (I made it twice, thinking I missed to
select the English version)."
Sadly I see the same.
I got hold of the proper link if that can help:
http://go.microsoft.com/?linkid=9709949

Ludovic

On Thu, Oct 3, 2013 at 2:56 PM, Francis ANDRE <
francis.andre.kampb...@orange.fr> wrote:

> Hi
>
> In the 7u40/jdk/make/common/shared/**Compiler-msvc.gmk, the code for
> computing the CC_VER does work with a French VS2010 Express compiler
>
> CC_VER  := $(shell $(CC) 2>&1 | $(HEAD) -n 1 | $(SED)
> 's/.*\(Version.*\)/\1/' | $(NAWK) '{print $$2}')
>
> because the cl command returns
>
> Compilateur d'optimisation Microsoft (R) 32 bits C/C++ version
> 16.00.30319.01 pour 80x86
> Copyright (C) Microsoft Corporation. Tous droits réservés.
>
> utilisation : cl [ option... ] nom de fichier... [ /link linkoption... ]
>
> So, soneone from the build team  would say: Hey guy, pick up the English
> version and it will work!. But that does not work neither because when one
> download VS2010 Express from a French territory, one gets the French
> version EVEN if the English version was explicitly requested (I made it
> twice, thinking I missed to select the English version).
>
>
> The problem has already been solved in the hotspot build by using the
> externals variables FORCE_MSC_VER and FORCE_LINK_VERSION in this file
> /hotspot/make/windows/get_msc_**ver.sh
>
> I can build a fix for this issue but I think it is better than someone
> from the build team fix this issue because of the overall responsibility of
> making the build correct on the WXP/VS2010 as well as others platforms
> which I do not have.
>
> Francis
>



-- 
Ludovic
-

"Les formes qui differencient les etres importent peu
 si leur pensees s'unissent pour batir un univers..."
 Yoko Tsuno (in 'Les titans' by Roger Leloup)
 [The shapes that differenciate beings are not important
 if their thoughts unite to build a universe]

Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access

[Tim Bell]

On 10/10/13 10:45 AM, Sean Mullan wrote:

On 10/10/2013 05:57 AM, Erik Joelsson wrote:

Adding makefiles to make/tools is not needed for the new build. Either
remove those changes or make a complete port to the old build. I'm not
pushing for porting this to the old build at this point since missing
this will not cause the old build to stop working, it will just diverge
the resulting bits a bit more.


I have ported the changes to the old build, please review.

New webrev: http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.02/

The only modified file is make/java/security/Makefile


Makefile looks good to me.

As for when the old makefiles are removed, we don't know for sure yet, 
but the time is getting closer as there are fewer stopper issues with 
the removal.


Tim

Re: RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac

[Tim Bell]

On 10/10/13 04:12 AM, Magnus Ihse Bursie wrote:

On 2013-10-10 12:10, Michael McMahon wrote:

Can I get the following change for jdk 8 reviewed please?

It's a simple build change to enable compilation of the dummy
SCTP API layer on macosx, plus the dummy implementation
used on windows.

The existing jdk_sctp tests cover this.

http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/


The build part looks fine to me.

/Magnus


Makefile changes look good to me as well.

Tim

Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access

[Sean Mullan]

On 10/10/2013 05:57 AM, Erik Joelsson wrote:

Adding makefiles to make/tools is not needed for the new build. Either
remove those changes or make a complete port to the old build. I'm not
pushing for porting this to the old build at this point since missing
this will not cause the old build to stop working, it will just diverge
the resulting bits a bit more.


I have ported the changes to the old build, please review.

New webrev: http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.02/

The only modified file is make/java/security/Makefile

Thanks,
Sean

Re: code review round 0 for Full Debug Symbols on MacOS X hotspot (7165611)

[Daniel D. Daugherty]

Thanks for confirmation.

Dan


On 10/10/13 3:09 AM, David Holmes wrote:

I'm fine with not adding untestable minimal VM support.

Thanks,
David

On 10/10/2013 1:03 AM, Daniel D. Daugherty wrote:

Replies also inline...


On 10/9/13 6:02 AM, David Holmes wrote:

inline ...

On 9/10/2013 8:59 AM, Daniel D. Daugherty wrote:

On 10/1/13 8:52 PM, David Holmes wrote:

- hotspot/make/Makefile

+ $(EXPORT_CLIENT_DIR)/%.dSYM: $(MINIMAL1_BUILD_DIR)/%.dSYM

EXPORT_CLIENT_DIR should be EXPORT_MINIMAL_DIR.

For fun you can try building minimal on OSX, but I don't know how far
you will get:

--with-jvm-variants=client,server,minimal1

I'll point out obvious issues with minimal VM support anyway.


Since you pointed out in a later email that minimal1 isn't
supported on MacOS X, I'm going to drop those changes. I'm
going to leave the Client VM support since there are folks
out in OpenJDK trying to get the Client VM working on MacOS X.

It does look like someone added minimal1 support for other
non-Linux platforms, but I'm not planning to clean that up.


Yes I did that when adding minimal support. Seemed better than leaving
someone to have to rediscover what needed to be implemented if/when
minimal support was expanded.


Thanks for the history. Are you still OK if I leave out the
FDS Minimal1 support?



Someday I hope to remove all the duplicated EXPORT_* stuff and
generate it based on the requested JDK_VARIANT_* values. And do it in
a platform indpendent way - once! :)


Well it can be mostly platform independent, but there will be
minor differences in what is exported by the different platforms.





Note that the whole jdk6_or_earlier logic is defunct as this will
never be used for jdk6 or earlier. But best to clean all that up at
the one time.


Agreed that we (Oracle) currently don't have plans to drop HSX-25 into
JDK6 or OpenJDK6, but I don't want to rule out future insanity.


I do! We need to cut ties with historical baggage.


Good sentiment, but not for this changeset. I'll file a bug to track
your idea of "best to clean all that up at one time".





- make/bsd/makefiles/universal.gmk

I did not understand the additional logic here:

  63 # Copy built non-universal binaries in place
  64 $(UNIVERSAL_COPY_LIST):
  65 BUILT_COPY_FILES="`find
$(EXPORT_JRE_LIB_DIR)/{i386,amd64}/$(subst 
$(EXPORT_JRE_LIB_DIR)/,,$@)

2>/dev/null`"; \
  66 if [ -n "$${BUILT_COPY_FILES}" ]; then \
  67   for i in $${BUILT_COPY_FILES}; do \
  68 if [ -f $${i} ]; then \
  69   $(MKDIR) -p $(shell dirname $@); \
  70   $(CP) -R $${i} $@; \
  71 fi; \
  72 if [ -d $${i} ]; then \
  73   $(MKDIR) -p $@; \
  74 fi; \
  75   done; \
  76 fi

until I realized that foo.dSYM is a directory not a file! Even so
don't you want to copy the contents of the directory across ?


BUILT_COPY_FILES includes both files and directories so everything
should get copied across. I've added some commments to the rule
header and reorganized the logic a bit to hopefully be more clear.


Doesn't this assume that the directory will appear before the files
within it? Is that guaranteed?


The way find works is that it lists the directory prior to listing
the files within the directory. However, even if find didn't, the
containing directory would be created via line 69 above. The one
non-obvious feature of lines 72-74 is that an empty directory named
in the BUILD_COPY_FILES list would get 'copied' to the destination.

Please check out the latest version when I get it out for review.



Which leads me to ask why we have cp -R for copying files?


MacOS X strongly discourages use of 'cp -r' and recommends 'cp -R'
instead because 'cp -R' properly copies non-directory and non-file
file system objects, e.g., symlinks. So if BUILD_COPY_FILES happens
to contain a symlink, then the symlink is copied to the destination.





- jdk/makefiles/Tools.gmk

Not sure about this one. Logically is seems correct but up to now 
this
has been defined for everything without it seeming to cause a 
problem.

So why do we need to change it and what impact will it have?


It just seemed wrong to define something that should never be used
on non-Solaris platforms. My control build of the entire forest
didn't have an issue with this change so I'm planning to keep it.


Ok.

Thanks,
David


Again, thanks for the thorough reviews.

Dan





Dan





David
-

On 21/09/2013 1:36 PM, Daniel D. Daugherty wrote:

Greetings,

I have the initial support for Full Debug Symbols (FDS) on MacOS X
done
and ready for review:

 7165611 implement Full Debug Symbols on MacOS X hotspot
 https://bugs.openjdk.java.net/browse/JDK-7165611

Here is the JDK8/HSX-25 webrev URL:

OpenJDK:
http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/0-jdk8/
Internal:
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/0-jdk8/ 




This webrev includes changes for the follo

Re: RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac

[Magnus Ihse Bursie]

On 2013-10-10 12:10, Michael McMahon wrote:

Can I get the following change for jdk 8 reviewed please?

It's a simple build change to enable compilation of the dummy
SCTP API layer on macosx, plus the dummy implementation
used on windows.

The existing jdk_sctp tests cover this.

http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/


The build part looks fine to me.

/Magnus

hg: jdk8/build: 8023611: Win32 and win64: Remove all the WARNINGS in JDK 8 builds for Windows 2008 and MSVS 2010 SP1

[tim . bell]

Changeset: ec48d637778a
Author:tbell
Date:  2013-10-09 18:51 -0700
URL:   http://hg.openjdk.java.net/jdk8/build/rev/ec48d637778a

8023611: Win32 and win64: Remove all the WARNINGS in JDK 8 builds for Windows 
2008 and MSVS 2010 SP1
Reviewed-by: erikj

! common/autoconf/generated-configure.sh
! common/autoconf/toolchain.m4

Re: RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac

[Michael McMahon]

On 10/10/13 12:01, Alan Bateman wrote:

On 10/10/2013 11:10, Michael McMahon wrote:

Can I get the following change for jdk 8 reviewed please?

It's a simple build change to enable compilation of the dummy
SCTP API layer on macosx, plus the dummy implementation
used on windows.

The existing jdk_sctp tests cover this.

http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/
This looks okay to me (I didn't look at the Sctp* sources closely as I 
assume they are just a copy of the dummy implementation from 
src/windows).




Yes. That's what I meant to say. The sources are copied from the windows 
tree. They just throw
UnsupportedOperationException, but it means that Sctp tests can be 
compiled on all the reference platforms now.


A minor comment (a question) is whether there is a convention to sort 
the values of EXFILES or not. If there is then you might have to 
re-order them.




Right. I'll defer to someone from build on that question.

Thanks
Michael

Re: RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac

[Alan Bateman]

On 10/10/2013 11:10, Michael McMahon wrote:

Can I get the following change for jdk 8 reviewed please?

It's a simple build change to enable compilation of the dummy
SCTP API layer on macosx, plus the dummy implementation
used on windows.

The existing jdk_sctp tests cover this.

http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/
This looks okay to me (I didn't look at the Sctp* sources closely as I 
assume they are just a copy of the dummy implementation from src/windows).


A minor comment (a question) is whether there is a convention to sort 
the values of EXFILES or not. If there is then you might have to 
re-order them.


-Alan

hg: jdk8/build/jdk: 2 new changesets

[tim . bell]

Changeset: 86df2e879eca
Author:tbell
Date:  2013-10-09 18:50 -0700
URL:   http://hg.openjdk.java.net/jdk8/build/jdk/rev/86df2e879eca

8023611: Win32 and win64: Remove all the WARNINGS in JDK 8 builds for Windows 
2008 and MSVS 2010 SP1
Reviewed-by: erikj

! make/common/shared/Compiler-msvc.gmk
! make/common/shared/Defs-versions.gmk
! make/common/shared/Sanity.gmk

Changeset: 98d98ec01f07
Author:tbell
Date:  2013-10-09 23:19 -0700
URL:   http://hg.openjdk.java.net/jdk8/build/jdk/rev/98d98ec01f07

Merge

Re: RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac

[Chris Hegarty]

Thank you for doing this Michael.

The changes look good to me, but someone from the build group should 
cast an eye on the makefile changes.


-Chris.

On 10/10/2013 11:10 AM, Michael McMahon wrote:

Can I get the following change for jdk 8 reviewed please?

It's a simple build change to enable compilation of the dummy
SCTP API layer on macosx, plus the dummy implementation
used on windows.

The existing jdk_sctp tests cover this.

http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/

Thanks,
Michael.

RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac

[Michael McMahon]

Can I get the following change for jdk 8 reviewed please?

It's a simple build change to enable compilation of the dummy
SCTP API layer on macosx, plus the dummy implementation
used on windows.

The existing jdk_sctp tests cover this.

http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/

Thanks,
Michael.

Re: RFR: 8026144: Missing mkdir in Images.gmk

[Magnus Ihse Bursie]

On 2013-10-10 10:45, Volker Simonis wrote:

I just want to add that parallel "mkdir -p " commands can fail on AIX
if  is on an NFS share. While knowing that AIX is not one of the main
OpenJDK platforms I would nevertheless prefer the conceptually cleaner way
of having a separate rule for the path to prevent parallel mkdirs for the
same directory.



Just as a side comment, having the build output directory is not 
recommended. It will typically affect the build speed tremendously, and 
if configure manages to figure out that this is the situation, it prints 
a warning about it.


I remember something about Solaris having problems with parallel mkdirs 
as well, even on non-NFS paths.


/Magnus

Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access

[Erik Joelsson]

Adding makefiles to make/tools is not needed for the new build. Either 
remove those changes or make a complete port to the old build. I'm not 
pushing for porting this to the old build at this point since missing 
this will not cause the old build to stop working, it will just diverge 
the resulting bits a bit more.


/Erik

On 2013-10-09 19:19, Sean Mullan wrote:
Updated webrev: 
http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.01/


Let me know if there are any more comments, otherwise I will plan to 
push tomorrow.


Thanks,
Sean

On 10/09/2013 09:20 AM, Sean Mullan wrote:

On 10/09/2013 05:14 AM, Erik Joelsson wrote:

Hello Sean,

On 2013-10-09 06:33, David Holmes wrote:

Hi Sean,

Not a full review.

On 9/10/2013 5:52 AM, Sean Mullan wrote:

Please review the fix for the following bug:

   https://bugs.openjdk.java.net/browse/JDK-8007292

This bug requires build changes and a new build tool to add 
additional

restricted packages to the java.security file which are not part of
OpenJDK. These packages are only added when doing a build 
including the

open and closed sources.

The restricted packages and new test are in the closed sources and 
will

be reviewed separately.

webrev: http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.00/


Based on your description and the ifndef OPENJDK it sounds to me that
this doesn't belong in the OpenJDK makefiles.


I agree in principle, but the pattern on how to handle this isn't well
established yet and something we need to improve on. In this case we
would need to make changes on the open side anyway to provide hooks for
overrides of the behavior. I'm willing to accept this for now.


Thanks. Also, keep in mind that this hook allows each vendor,etc to add
additional proprietary or internal packages to the restricted packages
properties for their own build. So I think it is useful in general in
that respect.


That aside I would think the CP+RM could be changed to a MV.

Agreed.


Right. Will do.


I would prefer the TOOL_ADDTO... line to be moved to
jdk/makefiles/Tools.gmk with the other similar definitions, even if it
is only used here atm.


Ok.


In the tool this code doesn't show correct use of try-with-resources:

51 try (BufferedReader br = new BufferedReader(new
FileReader(args[0]));
  52  BufferedWriter bw = new BufferedWriter(new
FileWriter(args[1]))) {

The FileReader and FileWriter should also be covered by TWR:

  try (FileReader fr = new FileReader(args[0]);
   BufferedReader br = new BufferedReader(fr);
   FileWriter fw = new FileWriter(args[1]);
   BufferedWriter bw = new BufferedWriter(fw)) {


I'm not familiar with the try-with-resources, but calling close on a
BufferedReader/writer will close the underlying reader/writer so 
nothing

will be left open, will it not?


That's what I thought as well. David?


Finally do we still use make/tools/Makefile in the new build?


No, we don't. If you want to add old build support for this, you would
also need to add usage of the tool to the correct old makefile.


I don't think it's necessary to add this to the old build at this point.

I'll post another webrev later in the day with these updates.

Thanks,
Sean

Re: code review round 0 for Full Debug Symbols on MacOS X hotspot (7165611)

[David Holmes]

I'm fine with not adding untestable minimal VM support.

Thanks,
David

On 10/10/2013 1:03 AM, Daniel D. Daugherty wrote:

Replies also inline...


On 10/9/13 6:02 AM, David Holmes wrote:

inline ...

On 9/10/2013 8:59 AM, Daniel D. Daugherty wrote:

On 10/1/13 8:52 PM, David Holmes wrote:

- hotspot/make/Makefile

+ $(EXPORT_CLIENT_DIR)/%.dSYM: $(MINIMAL1_BUILD_DIR)/%.dSYM

EXPORT_CLIENT_DIR should be EXPORT_MINIMAL_DIR.

For fun you can try building minimal on OSX, but I don't know how far
you will get:

--with-jvm-variants=client,server,minimal1

I'll point out obvious issues with minimal VM support anyway.


Since you pointed out in a later email that minimal1 isn't
supported on MacOS X, I'm going to drop those changes. I'm
going to leave the Client VM support since there are folks
out in OpenJDK trying to get the Client VM working on MacOS X.

It does look like someone added minimal1 support for other
non-Linux platforms, but I'm not planning to clean that up.


Yes I did that when adding minimal support. Seemed better than leaving
someone to have to rediscover what needed to be implemented if/when
minimal support was expanded.


Thanks for the history. Are you still OK if I leave out the
FDS Minimal1 support?



Someday I hope to remove all the duplicated EXPORT_* stuff and
generate it based on the requested JDK_VARIANT_* values. And do it in
a platform indpendent way - once! :)


Well it can be mostly platform independent, but there will be
minor differences in what is exported by the different platforms.





Note that the whole jdk6_or_earlier logic is defunct as this will
never be used for jdk6 or earlier. But best to clean all that up at
the one time.


Agreed that we (Oracle) currently don't have plans to drop HSX-25 into
JDK6 or OpenJDK6, but I don't want to rule out future insanity.


I do! We need to cut ties with historical baggage.


Good sentiment, but not for this changeset. I'll file a bug to track
your idea of "best to clean all that up at one time".





- make/bsd/makefiles/universal.gmk

I did not understand the additional logic here:

  63 # Copy built non-universal binaries in place
  64 $(UNIVERSAL_COPY_LIST):
  65 BUILT_COPY_FILES="`find
$(EXPORT_JRE_LIB_DIR)/{i386,amd64}/$(subst $(EXPORT_JRE_LIB_DIR)/,,$@)
2>/dev/null`"; \
  66 if [ -n "$${BUILT_COPY_FILES}" ]; then \
  67   for i in $${BUILT_COPY_FILES}; do \
  68 if [ -f $${i} ]; then \
  69   $(MKDIR) -p $(shell dirname $@); \
  70   $(CP) -R $${i} $@; \
  71 fi; \
  72 if [ -d $${i} ]; then \
  73   $(MKDIR) -p $@; \
  74 fi; \
  75   done; \
  76 fi

until I realized that foo.dSYM is a directory not a file! Even so
don't you want to copy the contents of the directory across ?


BUILT_COPY_FILES includes both files and directories so everything
should get copied across. I've added some commments to the rule
header and reorganized the logic a bit to hopefully be more clear.


Doesn't this assume that the directory will appear before the files
within it? Is that guaranteed?


The way find works is that it lists the directory prior to listing
the files within the directory. However, even if find didn't, the
containing directory would be created via line 69 above. The one
non-obvious feature of lines 72-74 is that an empty directory named
in the BUILD_COPY_FILES list would get 'copied' to the destination.

Please check out the latest version when I get it out for review.



Which leads me to ask why we have cp -R for copying files?


MacOS X strongly discourages use of 'cp -r' and recommends 'cp -R'
instead because 'cp -R' properly copies non-directory and non-file
file system objects, e.g., symlinks. So if BUILD_COPY_FILES happens
to contain a symlink, then the symlink is copied to the destination.





- jdk/makefiles/Tools.gmk

Not sure about this one. Logically is seems correct but up to now this
has been defined for everything without it seeming to cause a problem.
So why do we need to change it and what impact will it have?


It just seemed wrong to define something that should never be used
on non-Solaris platforms. My control build of the entire forest
didn't have an issue with this change so I'm planning to keep it.


Ok.

Thanks,
David


Again, thanks for the thorough reviews.

Dan





Dan





David
-

On 21/09/2013 1:36 PM, Daniel D. Daugherty wrote:

Greetings,

I have the initial support for Full Debug Symbols (FDS) on MacOS X
done
and ready for review:

 7165611 implement Full Debug Symbols on MacOS X hotspot
 https://bugs.openjdk.java.net/browse/JDK-7165611

Here is the JDK8/HSX-25 webrev URL:

OpenJDK:
http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/0-jdk8/
Internal:
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/0-jdk8/


This webrev includes changes for the follow repos:

 jdk8
 jdk8/hotspot
 jdk8/jdk
 jdk8/jdk/make/closed

Onc

Re: Trivial RFR: 8026232: Move libnpt from profile compact1 to compact3

[David Holmes]

Thanks Alan and Mandy.

David

On 10/10/2013 5:48 PM, Alan Bateman wrote:

On 10/10/2013 01:47, David Holmes wrote:

webrev:

http://cr.openjdk.java.net/~dholmes/8026232/webrev/

libnpt.so is used for debugging and profiling but was mistakenly
placed in profile compact1. It should be in compact3 along with the
hprof agent that uses it (other users are in full JRE).


Looks okay to me and helps reduce compact1 and compact2 another tiny bit.

-Alan.

Re: RFR: 8026144: Missing mkdir in Images.gmk

[Volker Simonis]

On Thu, Oct 10, 2013 at 10:08 AM, Erik Joelsson wrote:

>
> On 2013-10-10 03:02, David Holmes wrote:
>
>> On 9/10/2013 11:30 PM, Erik Joelsson wrote:
>>
>>> I just hit this race when building in jprt. Adding the standard mkdir -p
>>> lines should fix it.
>>>
>>> https://bugs.openjdk.java.net/**browse/JDK-8026144
>>>
>>> http://cr.openjdk.java.net/~**erikj/8026144/webrev.jdk.01/
>>>
>>
>> Hmmm. Isn't the real problem that there is a missing dependency on the
>> prior unzipping target (that already does the mkdir). To be pedantic I
>> suppose all three targets should have a dependency on a "prepare" target
>> that does the mkdir.
>>
>> Adding mkdir to all of them does avoid the problem though.
>>
>>  Of those options, I would say a separate rule for the directory would be
> more correct. There is no need to depend on the actual unzipping. It would
> also be complicated by the unzipping rule being a pattern rule, making it
> trickier to follow the dependency chain when reading the code.
>
> It's a matter of design choice I suppose. It's slightly more efficient to
> declare a rule for the directory alone that does the mkdir. At least on
> cygwin where execs are expensive. There are instances of this design in the
> build, but in most places we use the $(install-file) or similar macros in
> the recipe that does mkdir -p, just because it's simpler, especially for
> generated rules. In this case I opted for the smaller change and more
> common pattern.
>
> /Erik
>

I just want to add that parallel "mkdir -p " commands can fail on AIX
if  is on an NFS share. While knowing that AIX is not one of the main
OpenJDK platforms I would nevertheless prefer the conceptually cleaner way
of having a separate rule for the path to prevent parallel mkdirs for the
same directory.

Regards,
Volker

Re: RFR: 8026144: Missing mkdir in Images.gmk

[Erik Joelsson]

On 2013-10-10 03:02, David Holmes wrote:

On 9/10/2013 11:30 PM, Erik Joelsson wrote:

I just hit this race when building in jprt. Adding the standard mkdir -p
lines should fix it.

https://bugs.openjdk.java.net/browse/JDK-8026144

http://cr.openjdk.java.net/~erikj/8026144/webrev.jdk.01/


Hmmm. Isn't the real problem that there is a missing dependency on the 
prior unzipping target (that already does the mkdir). To be pedantic I 
suppose all three targets should have a dependency on a "prepare" 
target that does the mkdir.


Adding mkdir to all of them does avoid the problem though.

Of those options, I would say a separate rule for the directory would be 
more correct. There is no need to depend on the actual unzipping. It 
would also be complicated by the unzipping rule being a pattern rule, 
making it trickier to follow the dependency chain when reading the code.


It's a matter of design choice I suppose. It's slightly more efficient 
to declare a rule for the directory alone that does the mkdir. At least 
on cygwin where execs are expensive. There are instances of this design 
in the build, but in most places we use the $(install-file) or similar 
macros in the recipe that does mkdir -p, just because it's simpler, 
especially for generated rules. In this case I opted for the smaller 
change and more common pattern.


/Erik

Re: Trivial RFR: 8026232: Move libnpt from profile compact1 to compact3

[Alan Bateman]

On 10/10/2013 01:47, David Holmes wrote:

webrev:

http://cr.openjdk.java.net/~dholmes/8026232/webrev/

libnpt.so is used for debugging and profiling but was mistakenly 
placed in profile compact1. It should be in compact3 along with the 
hprof agent that uses it (other users are in full JRE).



Looks okay to me and helps reduce compact1 and compact2 another tiny bit.

-Alan.

Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access

[Magnus Ihse Bursie]

On 2013-10-09 15:20, Sean Mullan wrote:



Finally do we still use make/tools/Makefile in the new build?


No, we don't. If you want to add old build support for this, you would
also need to add usage of the tool to the correct old makefile.


I don't think it's necessary to add this to the old build at this point.


I don't really get this.

You have made some changes to the old build system (changes in 
make/tools/Makefile and the addition of 
make/tools/addtorestrictedpkgs/Makefile), but you are not actually using 
this.


I think you should either drop these changes as well, or fix it so that 
the old build is usable too. Doing it half-way seems just confusing.


Also, are we -- in general -- okay with making changes only to the new 
build system? As long as both systems are still there, I thought the 
idea was to keep them in sync. Even though we want to remove the old 
build, we don't know when that happens, and we *don't* want the systems 
do diverge. Or are there precedence for this kind of changes?


/Magnus