Request for Review: Support build-infra output directory in langtools test
There are several similarities between jdk/test/Makefile and langtools/test/Makefile. This makes it possible to run tests from either one in build-infra (the new build system) by using the target "test". In jdk/test/Makefile there is a test for an external variable ALT_OUTPUTDIR, which directs where the test results should be stored. The build-infra framework uses this to keep the test output in the corresponding build directory. However, while the code is very similar in langtools/test/Makefile, this exact feature is not available. This results in langtools tests being hardcoded to end up in a directory named as in the old build system. Here is a rather trivial patch that will add the ALT_OUTPUTDIR functionality to langtools as well. http://cr.openjdk.java.net/~ihse/support-build-infra-output-in-langtools-test/webrev.00/ I'm assuming someone in the langtools group should review this as well, and that it probably should go in via the -tl forest. I couldn't figure out what mailing list to reach the langtools group on though, so I'd appreaciate some help with forwarding this to the right persons. /Magnus
Request for Review: Add hotspot support for building with mingw/msys on build-infra (the new build)
The following patch will allow build-infra to perform builds on MinGW/MSys. It also updates the case for cygwin, with the two changes: 1) It replaces a $(subst) from / to \ which acted on output from cygpath -m (mixed mode), with just cygpath -w (windows mode, i.e. mixed mode but with \ instead of /). 2) In analogy with the msys case, we only do path convertion of the first word (i.e. the executable), not any possibly following arguments. In reality, there are no argument in the cygwin case (in contrary to the msys case, where the build infra solution requires passing additional arguments to the binaries). If these are problematic, they can be dropped. The important part, from build-infras point of view, is the mingw case. http://cr.openjdk.java.net/~ihse/hotspot-build-infra-msys-support/webrev.00/ /Magnus
Request for Review: Integrating build-infra changes into build forest
Here is a webrev of the latest changes in build-infra that we want to push to the build forest. http://cr.openjdk.java.net/~ihse/build-infra-integ/ For jdk, langtools, corba, jaxp and jaxws, all changes are in the makefiles directory. No patches from hotspot are needed at this time. (See separate mail on this.) In the top level, all changes are in the common directory (currently used exclusively by build-infra), except for a couple of changes at the root directory. This includes addition of a new, top-level configure script and the nbprojects directory. (Kelly, do you still want to push this?) /Magnus
Re: Request for Review: Support build-infra output directory in langtools test
This change is not right. We should not use the name ALT_ anything in this makefile, it is an independent Makefile. For better or worse, it does not share the build make logic at all. The names ALT_OUTPUTDIR and ABS_OUTPUTDIR could just be something like TEST_OUTPUT_ROOT or something without the ALT or OUTPUTDIR names. If these var names were changed I'd be fine with this basic change. But at the same time, there are also the test/Makefiles in jdk, hotspot, and the root. I would think we should at least verify those don't need the same change. -kto On Oct 25, 2012, at 4:42 AM, Magnus Ihse Bursie wrote: > There are several similarities between jdk/test/Makefile and > langtools/test/Makefile. This makes it possible to run tests from either one > in build-infra (the new build system) by using the target "test". In > jdk/test/Makefile there is a test for an external variable ALT_OUTPUTDIR, > which directs where the test results should be stored. The build-infra > framework uses this to keep the test output in the corresponding build > directory. > > However, while the code is very similar in langtools/test/Makefile, this > exact feature is not available. This results in langtools tests being > hardcoded to end up in a directory named as in the old build system. > > Here is a rather trivial patch that will add the ALT_OUTPUTDIR functionality > to langtools as well. > > http://cr.openjdk.java.net/~ihse/support-build-infra-output-in-langtools-test/webrev.00/ > > I'm assuming someone in the langtools group should review this as well, and > that it probably should go in via the -tl forest. I couldn't figure out what > mailing list to reach the langtools group on though, so I'd appreaciate some > help with forwarding this to the right persons. > > /Magnus
Re: Make File Changes For CR4239752
Thank you, Eric. I have updated my change and tested it locally. Everything works well. Here is the latest webrev, please review the change, http://cr.openjdk.java.net/~dxu/4239752/webrev/. Thanks! -Dan On Wed 24 Oct 2012 04:55:46 AM PDT, Erik Joelsson wrote: Alan seems to prefer explicit generation for this class. In that case the correct place to add it is in the list I referred to earlier: in jdk/makefiles/CompileJavaClasses.gmk: JDK_BASE_HEADER_CLASSES:=java.lang.Integer \ java.lang.Long \ java.net.SocketOptions \ sun.nio.ch.IOStatus \ java.io.FileSystem <- Add this and the backslash on the previous line /Erik On 2012-10-24 13:13, Alan Bateman wrote: On 23/10/2012 19:47, Dan Xu wrote: Thank you for all your good comments. After adding the @GenerateNativeHeader annotation, I am able to build the jdk using the new build. But where can I check whether this class is part of the base module in jigsaw? Or should I just use the annotation as the concerns will go away soon? -Dan I think Erik or Magnus should be able to advise on where is the right place to explicitly generate the headers for the core classes. If there isn't any easy way to do that then I wouldn't object to adding @GenerateNativeHeader, on the assumption that it will be replaced soon. -Alan.
Re: Request for Review: Support build-infra output directory in langtools test
I note that there are some notes on the OpenJDK website about running tests. See here: http://openjdk.java.net/jtreg/#makefile In particular, it includes this text: When using these targets, it is recommended to set the following variables, either as environment variables or on the "make" command line: * |JT_HOME|: the jtreg installation directory * |PRODUCT_HOME|: the JDK build to be tested: this is typically a file path ending in /j2sdk-image/ If someone makes any changes such that this is no longer true, please let me know so that I can update these notes. -- Jon On 10/25/2012 01:40 PM, Kelly O'Hair wrote: This change is not right. We should not use the name ALT_ anything in this makefile, it is an independent Makefile. For better or worse, it does not share the build make logic at all. The names ALT_OUTPUTDIR and ABS_OUTPUTDIR could just be something like TEST_OUTPUT_ROOT or something without the ALT or OUTPUTDIR names. If these var names were changed I'd be fine with this basic change. But at the same time, there are also the test/Makefiles in jdk, hotspot, and the root. I would think we should at least verify those don't need the same change. -kto On Oct 25, 2012, at 4:42 AM, Magnus Ihse Bursie wrote: There are several similarities between jdk/test/Makefile and langtools/test/Makefile. This makes it possible to run tests from either one in build-infra (the new build system) by using the target "test". In jdk/test/Makefile there is a test for an external variable ALT_OUTPUTDIR, which directs where the test results should be stored. The build-infra framework uses this to keep the test output in the corresponding build directory. However, while the code is very similar in langtools/test/Makefile, this exact feature is not available. This results in langtools tests being hardcoded to end up in a directory named as in the old build system. Here is a rather trivial patch that will add the ALT_OUTPUTDIR functionality to langtools as well. http://cr.openjdk.java.net/~ihse/support-build-infra-output-in-langtools-test/webrev.00/ I'm assuming someone in the langtools group should review this as well, and that it probably should go in via the -tl forest. I couldn't figure out what mailing list to reach the langtools group on though, so I'd appreaciate some help with forwarding this to the right persons. /Magnus
Re: Request for Review: Add hotspot support for building with mingw/msys on build-infra (the new build)
It has been my experience that using cygpath -w created problems with shell escape logic, and that is why cygpath -m with a make level subst change is done, it was less problematic. I don't understand why you would even want to change the cygwin logic at all, if it isn't broken why mess with it? So my suggestion would be to add the msys logic, and leave the old cygwin logic alone. I could have sworn that Tim Bell already did this change, and yes, if I look at: http://hg.openjdk.java.net/jdk8/build/hotspot/file/dccd40de8db1/make/windows/makefiles/defs.make These changes are there already, or something equivalent. Have you talked with Tim Bell about this change? -kto On Oct 25, 2012, at 6:49 AM, Magnus Ihse Bursie wrote: > The following patch will allow build-infra to perform builds on MinGW/MSys. > > It also updates the case for cygwin, with the two changes: > > 1) It replaces a $(subst) from / to \ which acted on output from cygpath -m > (mixed mode), with just cygpath -w (windows mode, i.e. mixed mode but with \ > instead of /). > > 2) In analogy with the msys case, we only do path convertion of the first > word (i.e. the executable), not any possibly following arguments. In reality, > there are no argument in the cygwin case (in contrary to the msys case, where > the build infra solution requires passing additional arguments to the > binaries). > > If these are problematic, they can be dropped. The important part, from > build-infras point of view, is the mingw case. > > http://cr.openjdk.java.net/~ihse/hotspot-build-infra-msys-support/webrev.00/ > > /Magnus
Re: Makefile changes
If these changes are integrated and on their way to jdk8/jdk8 already, it's water under the bridge, we need to see them earlier in the future. I appreciate the information though, I looked at these changesets and they look fine, but it is extremely hard to review makefile changes and know that they are correct, as you well know. :( I don't think we need to see copyright changes. What I will observe is that there appears to be many extremely minor, maybe cosmetic changes being made to makefiles that probably are unnecessary. What I will suggest from now on is that developers should strive to NOT change any makefiles unless the changes have been deemed critical or necessary for some kind of bug fix. Cleanups and misc format changes to makefiles would not be a good idea right now. Thanks for the heads up on these changes. -kto On Oct 18, 2012, at 11:40 AM, Chris Gruszka wrote: > I saw your other email, that it does apply to closed repos. > > In our current JDK8 PIT (b62/b63?), we have makefile/.gmk file changes in: > 7184404: MacOS AU needs to support a scheduled update check > > 7197469: update copyright year to match last edit in jdk8 install repository > > 7198150: install inclusion of JavaFX cobundle must fail if cobundle isn't > available in ALT_JAVAFX_ZIP_DIR > > 7200833: After JavaFX truly cobundle, DS NEXTVER builds fail > 7196306: After JavaFX truly cobundle, JavaFX_VERSION in > ds_build_versions is wrong > 7195616: JavaFX files are not signed during DS build > > 8000536: [Mac] JavaFX missing from install (JDK and private JRE) > > 7200140: Install repo for JDK8 has SUNWj7* directories > > 7199651: 7u10 win64 build failed at rebase process by including a signed > 7192948: bin/installer.dll has wrong versions and is not signed > > -ChrisG > > On 10/18/2012 2:23 PM, Chris Gruszka wrote: >> >> Kelly, >> Does this include closed repositories like install? >> My understanding was that no changes had been made for closed repos. >> >> -ChrisG >> >> On 10/18/2012 1:47 PM, Kelly O'Hair wrote: >>> If you are making any changes to the Makefiles or build process in jdk8, >>> please let us know in the build group >>> build-dev@openjdk.java.net. We will need to review and approve ALL Makefile >>> and build changes from now >>> until we change over to the new build-infra Makefiles. >>> The build-infra team can help you determine the equivalent change needed in >>> the new build-infra Makefiles. >>> >>> -kto >>>
Re: Request for Review: Support build-infra output directory in langtools test
As I said, the name ALT_OUTPUTDIR is already used in jdk/test/Makefile. That is the reason I used it. Of course it could be called anything, but if it is not called the same thing across repos, we get a hard time for no good reason. The only important thing for integration with build-infra is that the langtools test makefile gets a customizable output directory, so it is not hard-coded to the old output directory. The build-infra test integration currently calls test/Makefile in the root, and that works fine. I do not remeber if it is because it already has this variable, or because it does not output anything, but I suspect thr latter. I was not aware that the hotspot repo also shared this test logic, so I haven't checked there. /Magnus 25 okt 2012 kl. 22:40 skrev Kelly O'Hair : > > This change is not right. > > We should not use the name ALT_ anything in this makefile, it is an > independent Makefile. > For better or worse, it does not share the build make logic at all. > > The names ALT_OUTPUTDIR and ABS_OUTPUTDIR could just be something like > TEST_OUTPUT_ROOT > or something without the ALT or OUTPUTDIR names. > > If these var names were changed I'd be fine with this basic change. But at > the same time, there are also the test/Makefiles > in jdk, hotspot, and the root. I would think we should at least verify those > don't need the same change. > > -kto > > On Oct 25, 2012, at 4:42 AM, Magnus Ihse Bursie wrote: > >> There are several similarities between jdk/test/Makefile and >> langtools/test/Makefile. This makes it possible to run tests from either one >> in build-infra (the new build system) by using the target "test". In >> jdk/test/Makefile there is a test for an external variable ALT_OUTPUTDIR, >> which directs where the test results should be stored. The build-infra >> framework uses this to keep the test output in the corresponding build >> directory. >> >> However, while the code is very similar in langtools/test/Makefile, this >> exact feature is not available. This results in langtools tests being >> hardcoded to end up in a directory named as in the old build system. >> >> Here is a rather trivial patch that will add the ALT_OUTPUTDIR functionality >> to langtools as well. >> >> http://cr.openjdk.java.net/~ihse/support-build-infra-output-in-langtools-test/webrev.00/ >> >> I'm assuming someone in the langtools group should review this as well, and >> that it probably should go in via the -tl forest. I couldn't figure out what >> mailing list to reach the langtools group on though, so I'd appreaciate some >> help with forwarding this to the right persons. >> >> /Magnus >
Re: Request for Review: Integrating build-infra changes into build forest
On 26/10/2012 12:51 AM, Magnus Ihse Bursie wrote: Here is a webrev of the latest changes in build-infra that we want to push to the build forest. http://cr.openjdk.java.net/~ihse/build-infra-integ/ For jdk, langtools, corba, jaxp and jaxws, all changes are in the makefiles directory. I've looked at jdk and top-level. No major comments. The proof of all this is in the building and the sooner we get jdk8/jdk8 up to date with this the sooner build-infra can wind up as a distinct project :) In version.numbers I'd still like to see these entries deleted: 30 JDK_BUILD_NUMBER= 31 MILESTONE=internal so that they can be overridden through the environment. David - No patches from hotspot are needed at this time. (See separate mail on this.) In the top level, all changes are in the common directory (currently used exclusively by build-infra), except for a couple of changes at the root directory. This includes addition of a new, top-level configure script and the nbprojects directory. (Kelly, do you still want to push this?) /Magnus
