Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v5]
On Wed, 9 Dec 2020 07:22:51 GMT, Bernhard Urban-Forster wrote: >> This adds the cross-compiled build only, as no Windows+Arm64 machines are >> available on GitHub Action that we could use to run the tests. >> >> Due to cross-compilation a build JDK is required. Initially I added EA >> builds to be downloaded from https://jdk.java.net/16/ and used for that, but >> then I saw how @shipiliv attempted it for the linux cross-compilation builds >> in https://github.com/openjdk/jdk/pull/1147. That is using the JDK image >> produced by the x64 variant. This however add more stress to the "critical >> path", as now two more jobs depend on the x64 build first. >> >> Let's see how it works out in the long-run. A Windows+AArch64 build takes >> 40-50min. > > Bernhard Urban-Forster has updated the pull request with a new target base > due to a merge or a rebase. The pull request now contains 12 commits: > > - remove gtest and jtreg > - Merge remote-tracking branch 'upstream/master' into > 8256657-win-arm64-gh-submit-workflow > - merge mistakes > - change default target to hotspot and align with updated x64 bits > - remove devkit usage on win-aarch64 > - Merge remote-tracking branch 'upstream/master' into > 8256657-win-arm64-gh-submit-workflow > - todo note for caching devkit > - remove release build > - move windows_aarch64_build next to other builds > - Merge remote-tracking branch 'upstream/master' into > 8256657-win-arm64-gh-submit-workflow > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/9ce3d806...c07f5d72 Thanks, updates look fine to me. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v5]
> This adds the cross-compiled build only, as no Windows+Arm64 machines are > available on GitHub Action that we could use to run the tests. > > Due to cross-compilation a build JDK is required. Initially I added EA builds > to be downloaded from https://jdk.java.net/16/ and used for that, but then I > saw how @shipiliv attempted it for the linux cross-compilation builds in > https://github.com/openjdk/jdk/pull/1147. That is using the JDK image > produced by the x64 variant. This however add more stress to the "critical > path", as now two more jobs depend on the x64 build first. > > Let's see how it works out in the long-run. A Windows+AArch64 build takes > 40-50min. Bernhard Urban-Forster has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - remove gtest and jtreg - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - merge mistakes - change default target to hotspot and align with updated x64 bits - remove devkit usage on win-aarch64 - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - todo note for caching devkit - remove release build - move windows_aarch64_build next to other builds - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - ... and 2 more: https://git.openjdk.java.net/jdk/compare/9ce3d806...c07f5d72 - Changes: https://git.openjdk.java.net/jdk/pull/1379/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1379=04 Stats: 92 lines in 1 file changed: 91 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1379.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1379/head:pull/1379 PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v4]
On Tue, 8 Dec 2020 06:59:23 GMT, Aleksey Shipilev wrote: >> Minor nits. > > Also merge from master to get the clean workflow run everywhere? Thank you @shipilev for your comments, I've updated the PR. - PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8257450: Start of release updates for JDK 17 [v6]
> Start of JDK 17 updates. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 14 additional commits since the last revision: - Get in JEP 390 changes. - Merge branch 'master' into JDK-8257450 - Merge branch 'master' into JDK-8257450 - Update symbol files for JDK 16b27. - Merge branch 'master' into JDK-8257450 - Merge branch 'master' into JDK-8257450 - Merge branch 'master' into JDK-8257450 - Update tests. - Merge branch 'master' into JDK-8257450 - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into JDK-8257450 - ... and 4 more: https://git.openjdk.java.net/jdk/compare/823053e1...57ba7b64 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1531/files - new: https://git.openjdk.java.net/jdk/pull/1531/files/ff9b78ec..57ba7b64 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1531=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1531=04-05 Stats: 1178 lines in 148 files changed: 571 ins; 195 del; 412 mod Patch: https://git.openjdk.java.net/jdk/pull/1531.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1531/head:pull/1531 PR: https://git.openjdk.java.net/jdk/pull/1531
Re: RFR: 8257450: Start of release updates for JDK 17 [v5]
On Tue, 8 Dec 2020 17:10:30 GMT, Joe Darcy wrote: >> Start of JDK 17 updates. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 12 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8257450 > - Update symbol files for JDK 16b27. > - Merge branch 'master' into JDK-8257450 > - Merge branch 'master' into JDK-8257450 > - Merge branch 'master' into JDK-8257450 > - Update tests. > - Merge branch 'master' into JDK-8257450 > - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into > JDK-8257450 > - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into > JDK-8257450 > - JDK-8257450 > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/d1c29ce3...ff9b78ec Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1531
Re: RFR: JDK-8256950: Add record attribute support to symbol generator CreateSymbols [v5]
On Thu, 3 Dec 2020 09:22:18 GMT, Jan Lahoda wrote: >> Adding support for record classes in the historical data for ct.sym. This >> includes a few changes not strictly needed for the change: >> -updating and moving tests into test/langtools, so that it is easier to run >> them. >> -fixing Record attribute reading in javac's ClassReader (used for tests, but >> seems like the proper thing to do anyway). >> -fixing the -Xprint annotation processor to print record component >> annotations. >> >> Changes to jdk.jdeps' classfile library are needed so that the ct.sym >> creation works. > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Blank lines do not count for the text block indentation, removing them. Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1480
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v12]
> This is an update to javac and javadoc, to introduce support for Preview > APIs, and generally improve javac and javadoc behavior to more closely adhere > to JEP 12. > > The notable changes are: > > * adding support for Preview APIs (javac until now supported primarily only > preview language features, and APIs associated with preview language > features). This includes: > * the @PreviewFeature annotation has boolean attribute "reflective", > which should be true for reflective Preview APIs, false otherwise. This > replaces the existing "essentialAPI" attribute with roughly inverted meaning. > * the preview warnings for preview APIs are auto-suppressed as described > in the JEP 12. E.g. the module that declares the preview API is free to use > it without warnings > * improving error/warning messages. Please see [1] for a list of > cases/examples. > * class files are only marked as preview if they are using a preview > feature. [1] also shows if a class file is marked as preview or not. > * the PreviewFeature annotation has been moved to jdk.internal.javac package > (originally was in the jdk.internal package). > * Preview API in JDK's javadoc no longer needs to specify @preview tag in > the source files. javadoc will auto-generate a note for @PreviewFeature > elements, see e.g. [2] and [3] (non-reflective and reflective API, > respectively). A summary of preview elements is also provided [4]. Existing > uses of @preview have been updated/removed. > * non-JDK javadoc is also enhanced to auto-generate preview notes for uses > of Preview elements, and for declaring elements using preview language > features [5]. > > Please also see the CSR [6] for more information. > > [1] http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html > [2] > http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html > [3] > http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html > [4] > http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html > [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/ > [6] https://bugs.openjdk.java.net/browse/JDK-8250769 Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 55 commits: - Merging recent master changes into JDK-8250768 - Fixing navigator for the PREVIEW page. - Fixing typo. - Removing obsolette @PreviewFeature. - Merging master into JDK-8250768 - Removing unnecessary property keys. - Cleanup - removing unnecessary code. - Merging master into JDK-8250768-dev4 - Reflecting review comments. - Removing trailing whitespace. - ... and 45 more: https://git.openjdk.java.net/jdk/compare/044616bd...0c1c4d57 - Changes: https://git.openjdk.java.net/jdk/pull/703/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=703=11 Stats: 3711 lines in 132 files changed: 2726 ins; 692 del; 293 mod Patch: https://git.openjdk.java.net/jdk/pull/703.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/703/head:pull/703 PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 17:33:16 GMT, Alan Bateman wrote: >> The mapping and nr tables, and the *-X.java.template files in >> make/data/charsetmapping are used to generate source code for the java.base >> and jdk.charsets modules. The stdcs-$OS files configure the package and >> module that each charset go into. If the tables used to generate the source >> files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk >> will probably need a new home too. > >> @AlanBateman The process of modularization was not fully completed with >> Project Jigsaw, and a few ugly warts remained. I was under the impression >> that these should be addressed in follow-up fixes, but this was >> unfortunately never done. Charsets and cldrconverter were both split between >> a core portion in java.base and the rest in jdk.charsets and jdk.localedata, >> respectively, but the split was never handled properly, but just "duct >> taped" in place. > > This is a complicated area of the build, not really a Project Jigsaw issue. > It's complicated because the source code for the charsets is generated at > build time and the set of non-standard charsets included in java.base varies > by platform, e.g. there's are several IBMxxx charsets in java.base when > building on AIX that are not interesting to include in java.base on other > platforms. This means we can't split up the mapping tables in > make/data/charsetmapping and put them in different directories. If you are > moving them into the src tree then src/java.base (as you have it) is best but > will still have the ugly wart that some of these mapping tables will be used > to generate code for the jdk.charsets module. @AlanBateman @mlchung @naotoj I can certainly anticipate follow-up cleanups on this patch. In fact, I think this patch has the potential to be a catalyst for such change, since the data that has been "hidden away" in `make` now becomes more visible to the teams that are capable of doing something about it. (With that said, of course the build team will assist in helping to clear up messy code structure, as we always do.) But I think we should be restrictive in trying too hard to make everything right in this single patch; it's better to get things approximately right and then go through the "warts" one by one and solve them properly. @AlanBateman What I meant by Jigsaw was that when the monolithic source code were modularized, the separation of concern between java.base and jdk.charsets/jdk.localedata was not complete, compared to (more or less) all other modules. It might very well be the case that we will never be able to make such a separation; but, prior to Jigsaw, this was not a "wart". It only became a code hygiene issue when some of the charsets and localedata was extraced from java.base. @mlchung My gut reaction is that we should not change idea.sh. First of all, kind of the purpose of this move is to make it clear to module developers the full set of materials their module consists of. That purpose would be sort of defeated, if we were to hide this in a popular IDE configuration. Secondly, I doubt this will add any performance difference. Listing additional files in the workspace is unlikely to do much, unless you actively open and/or interact with these files. But if you are worried, please fetch the PR (Skara adds instructions in the body) and try it out yourself! - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 17:33:16 GMT, Alan Bateman wrote: >> The mapping and nr tables, and the *-X.java.template files in >> make/data/charsetmapping are used to generate source code for the java.base >> and jdk.charsets modules. The stdcs-$OS files configure the package and >> module that each charset go into. If the tables used to generate the source >> files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk >> will probably need a new home too. > >> @AlanBateman The process of modularization was not fully completed with >> Project Jigsaw, and a few ugly warts remained. I was under the impression >> that these should be addressed in follow-up fixes, but this was >> unfortunately never done. Charsets and cldrconverter were both split between >> a core portion in java.base and the rest in jdk.charsets and jdk.localedata, >> respectively, but the split was never handled properly, but just "duct >> taped" in place. > > This is a complicated area of the build, not really a Project Jigsaw issue. > It's complicated because the source code for the charsets is generated at > build time and the set of non-standard charsets included in java.base varies > by platform, e.g. there's are several IBMxxx charsets in java.base when > building on AIX that are not interesting to include in java.base on other > platforms. This means we can't split up the mapping tables in > make/data/charsetmapping and put them in different directories. If you are > moving them into the src tree then src/java.base (as you have it) is best but > will still have the ugly wart that some of these mapping tables will be used > to generate code for the jdk.charsets module. > @AlanBateman The process of modularization was not fully completed with > Project Jigsaw, and a few ugly warts remained. I was under the impression > that these should be addressed in follow-up fixes, but this was unfortunately > never done. Charsets and cldrconverter were both split between a core portion > in java.base and the rest in jdk.charsets and jdk.localedata, respectively, > but the split was never handled properly, but just "duct taped" in place. > > I chose to put the data files used for both java.base and the "additional" > modules in java.base, based on the comment that Naoto made in > https://mail.openjdk.java.net/pipermail/build-dev/2020-March/027044.html: > > > As to charsetmapping and cldrconverter, I believe they can reside in > > java.base, as jdk.charsets and jdk.localedata modules depend on it. > > Of course it would be preferable to make a proper split, but that requires > work done by the component teams to break the modules apart. > > Specifically for make/modules/jdk.charsets/Gensrc.gmk; the code in that file > is more or less duplicated in > make/modules/java.base/gensrc/GensrcCharsetMapping.gmk, since the same data > set is processed twice, once for java.base and once for jdk.charsets. I don't > think that means that make/modules/jdk.charsets/Gensrc.gmk should move to any > other place. I still stand by what I wrote above. It's best to put data in java.base for charsets/localedata. Otherwise we would have to duplicate some in each modules source directory. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257620: Do not use objc_msgSend_stret to get macOS version
On Thu, 3 Dec 2020 06:50:11 GMT, Anton Kozlov wrote: >> Filed https://bugs.openjdk.java.net/browse/JDK-8257633 > > Thanks for taking care of those issues. To be clear, there is no real need to > bump the version for this PR, 10.9 is fine. This PR just proposes another way > to implement the workaround for 10.9. Hi, could I get review of the patch? - PR: https://git.openjdk.java.net/jdk/pull/1569
Re: RFR: 8257450: Start of release updates for JDK 17 [v5]
On Tue, 8 Dec 2020 17:10:30 GMT, Joe Darcy wrote: >> Start of JDK 17 updates. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 12 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8257450 > - Update symbol files for JDK 16b27. > - Merge branch 'master' into JDK-8257450 > - Merge branch 'master' into JDK-8257450 > - Merge branch 'master' into JDK-8257450 > - Update tests. > - Merge branch 'master' into JDK-8257450 > - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into > JDK-8257450 > - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into > JDK-8257450 > - JDK-8257450 > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/c3d62067...ff9b78ec Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1531
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 18:16:15 GMT, Mandy Chung wrote: >> @wangweij Moving build tools is a related, but separate, question, which is >> addressed by https://bugs.openjdk.java.net/browse/JDK-8241463. >> >> I send out a review earlier this year (see >> https://mail.openjdk.java.net/pipermail/build-dev/2020-March/027038.html), >> but there were differing opinions on what the proper placement of these >> tools should be, and the discussion kind of fizzled out without reaching a >> conclusion. > > @magicus @erikj79 it's now clearly stated that you want everything under > `make` owned by the build team, which is fair. You are right that `make` has > been easily considered as a dumping ground and it's time to define a clean > structure. > > I reviewed this patch and this looks okay to me. Some follow-up questions > and follow-on cleanup for example "should jdwp.spec belong to `specs` > directory vs `data`? There are platform-specific data (such as charsets) > which has been special-case in the makefile and they need follow-on clean up. > I agree that this should be cleaned up by the component teams and not part > of this PR. With these new files showing up in under `src` directory, should `bin/idea.sh` be changed to exclude `data` to avoid incurring costs in loading JDK project from IDE that many of us use for development? - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 16:19:05 GMT, Magnus Ihse Bursie wrote: >> Is there a plan to move make/jdk/src/classes/build/tools/intpoly into >> java.base as well? >> >> Update: I see all subdirs in tools are still there. > > @wangweij Moving build tools is a related, but separate, question, which is > addressed by https://bugs.openjdk.java.net/browse/JDK-8241463. > > I send out a review earlier this year (see > https://mail.openjdk.java.net/pipermail/build-dev/2020-March/027038.html), > but there were differing opinions on what the proper placement of these tools > should be, and the discussion kind of fizzled out without reaching a > conclusion. @magicus @erikj79 it's now clearly stated that you want everything under `make` owned by the build team, which is fair. You are right that `make` has been easily considered as a dumping ground and it's time to define a clean structure. I reviewed this patch and this looks okay to me. Some follow-up questions and follow-on cleanup for example "should jdwp.spec belong to `specs` directory vs `data`? There are platform-specific data (such as charsets) which has been special-case in the makefile and they need follow-on clean up. I agree that this should be cleaned up by the component teams and not part of this PR. - PR: https://git.openjdk.java.net/jdk/pull/1611
Integrated: 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments
On Tue, 8 Dec 2020 14:43:00 GMT, Magnus Ihse Bursie wrote: > When calling a Windows tool that needs an argument on the form > `-option/a:/home/ihse/myjdk/...`, fixpath gets confused and thinks > `/a:/home/ihse/myjdk` is a unix path, notices that it does not exists, and > stops trying to convert the path. > > Instead it needs to recheck for paths after a separator such as `:` or `=`. This pull request has now been integrated. Changeset: 264feb35 Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/264feb35 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/1698
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 12:15:38 GMT, Alan Bateman wrote: >> Also, to clarify, for me there is a fundamental difference between >> `src/$MODULE` and `make/modules/$MODULE`. The former is the home of files >> that are part of the module, owned by the content team, and the `$MODULE` >> part is essential to delineate this content. The latter is owned by the >> build team, and is just a convenient way to organize the build system within >> the `make` directory. So it's clearly a no-no to put anything but `.gmk` >> files in `make/modules/$MODULE`. > > The mapping and nr tables, and the *-X.java.template files in > make/data/charsetmapping are used to generate source code for the java.base > and jdk.charsets modules. The stdcs-$OS files configure the package and > module that each charset go into. If the tables used to generate the source > files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk > will probably need a new home too. > @AlanBateman The process of modularization was not fully completed with > Project Jigsaw, and a few ugly warts remained. I was under the impression > that these should be addressed in follow-up fixes, but this was unfortunately > never done. Charsets and cldrconverter were both split between a core portion > in java.base and the rest in jdk.charsets and jdk.localedata, respectively, > but the split was never handled properly, but just "duct taped" in place. This is a complicated area of the build, not really a Project Jigsaw issue. It's complicated because the source code for the charsets is generated at build time and the set of non-standard charsets included in java.base varies by platform, e.g. there's are several IBMxxx charsets in java.base when building on AIX that are not interesting to include in java.base on other platforms. This means we can't split up the mapping tables in make/data/charsetmapping and put them in different directories. If you are moving them into the src tree then src/java.base (as you have it) is best but will still have the ugly wart that some of these mapping tables will be used to generate code for the jdk.charsets module. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments [v2]
On Tue, 8 Dec 2020 16:43:14 GMT, Magnus Ihse Bursie wrote: >> When calling a Windows tool that needs an argument on the form >> `-option/a:/home/ihse/myjdk/...`, fixpath gets confused and thinks >> `/a:/home/ihse/myjdk` is a unix path, notices that it does not exists, and >> stops trying to convert the path. >> >> Instead it needs to recheck for paths after a separator such as `:` or `=`. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > The same fix is also needed for the $DRIVEPREFIX regex Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1698
Re: RFR: 8257450: Start of release updates for JDK 17 [v5]
> Start of JDK 17 updates. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision: - Merge branch 'master' into JDK-8257450 - Update symbol files for JDK 16b27. - Merge branch 'master' into JDK-8257450 - Merge branch 'master' into JDK-8257450 - Merge branch 'master' into JDK-8257450 - Update tests. - Merge branch 'master' into JDK-8257450 - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into JDK-8257450 - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into JDK-8257450 - JDK-8257450 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/e5462611...ff9b78ec - Changes: - all: https://git.openjdk.java.net/jdk/pull/1531/files - new: https://git.openjdk.java.net/jdk/pull/1531/files/342c8650..ff9b78ec Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1531=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1531=03-04 Stats: 1427 lines in 38 files changed: 1106 ins; 191 del; 130 mod Patch: https://git.openjdk.java.net/jdk/pull/1531.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1531/head:pull/1531 PR: https://git.openjdk.java.net/jdk/pull/1531
Re: RFR: 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments [v2]
> When calling a Windows tool that needs an argument on the form > `-option/a:/home/ihse/myjdk/...`, fixpath gets confused and thinks > `/a:/home/ihse/myjdk` is a unix path, notices that it does not exists, and > stops trying to convert the path. > > Instead it needs to recheck for paths after a separator such as `:` or `=`. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: The same fix is also needed for the $DRIVEPREFIX regex - Changes: - all: https://git.openjdk.java.net/jdk/pull/1698/files - new: https://git.openjdk.java.net/jdk/pull/1698/files/ae993372..b678804e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1698=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1698=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1698.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1698/head:pull/1698 PR: https://git.openjdk.java.net/jdk/pull/1698
Re: RFR: 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments [v2]
On Tue, 8 Dec 2020 16:40:26 GMT, Magnus Ihse Bursie wrote: >> When calling a Windows tool that needs an argument on the form >> `-option/a:/home/ihse/myjdk/...`, fixpath gets confused and thinks >> `/a:/home/ihse/myjdk` is a unix path, notices that it does not exists, and >> stops trying to convert the path. >> >> Instead it needs to recheck for paths after a separator such as `:` or `=`. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > The same fix is also needed for the $DRIVEPREFIX regex Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1698
Re: RFR: JDK-8257539: tools/jpackage/windows/WinL10nTest.java unpack.bat failed with Exit code: 1618
On Tue, 8 Dec 2020 14:56:20 GMT, Andy Herrick wrote: >> Looks like test failed due to: >> [Fatal Error] b.wxl:3:1: XML document structures must start and end within >> the same entity. >> So, I am not sure how increased repeat will help. Do we know why it failed >> to parse XML? > >> >> >> Looks like test failed due to: >> [Fatal Error] b.wxl:3:1: XML document structures must start and end within >> the same entity. >> So, I am not sure how increased repeat will help. Do we know why it failed >> to parse XML? > > If you scroll down from that error - you can see that that is the expected > error and the return code from jpackage is asserted to be 1, and it is > asserted that the resulting WinL10NTest.msi does not exist. > > the @Parameters for "data" cause instance of this test to be run 8 times with > different args. This instances is expected to fail since allWxlFilesValid is > false. > later in the log > (https://mach5.us.oracle.com:10060/api/v1/results/mach5-one-jdk-16+26-1740-tier2-20201124-1335-16116386-open_test_jdk_tier2_part2-windows-x64-125-1606226621-1556/log) > you can see the timeout, where unpack.bat is run three times with 3 second > delay and returns 1618 each time: > **13:58:30.303] TRACE: exec: Execute [cmd /c > .\\test.3563aceb\\unpacked-msi\\unpack.bat](3); discard I/O... > [13:58:33.469] TRACE: exec: Done. Exit code: 1618 > [13:58:36.492] TRACE: exec: Execute [cmd /c > .\\test.3563aceb\\unpacked-msi\\unpack.bat](3); discard I/O... > [13:58:39.636] TRACE: exec: Done. Exit code: 1618 > [13:58:42.652] TRACE: exec: Execute [cmd /c > .\\test.3563aceb\\unpacked-msi\\unpack.bat](3); discard I/O... > [13:58:45.803] TRACE: exec: Done. Exit code: 1618 > [13:58:48.832] ERROR: Expected [0]. Actual [1618]: Check command [cmd /c > .\\test.3563aceb\\unpacked-msi\\unpack.bat](3) exited with 0 code > [13:58:48.832] [ FAILED ] WinL10nTest([name=a.wxl; culture=fr, name=b.wxl; > culture=fr](length=2), fr;en-us, null).test; checks=17 > [13:58:48.832] [ RUN ] WinL10nTest([name=a.wxl; culture=fr, name=b.wxl; > culture=it, name=c.wxl; culture=fr, name=d.wxl; culture=it](length=4), > fr;it;en-us,** > > running unpack.bat with msiexec command in it succeed for one test instance > after the expected failure, then got 1618 return on the second test instance > after the expected failure with the above timeout. I converted to draft because somehow merging with master caused a mess - since is simple change in one test file I may create a new branch and new pull request cleanly again. - PR: https://git.openjdk.java.net/jdk/pull/1676
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 15:25:47 GMT, Weijun Wang wrote: >> The mapping and nr tables, and the *-X.java.template files in >> make/data/charsetmapping are used to generate source code for the java.base >> and jdk.charsets modules. The stdcs-$OS files configure the package and >> module that each charset go into. If the tables used to generate the source >> files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk >> will probably need a new home too. > > Is there a plan to move make/jdk/src/classes/build/tools/intpoly into > java.base as well? > > Update: I see all subdirs in tools are still there. @wangweij Moving build tools is a related, but separate, question, which is addressed by https://bugs.openjdk.java.net/browse/JDK-8241463. I send out a review earlier this year (see https://mail.openjdk.java.net/pipermail/build-dev/2020-March/027038.html), but there were differing opinions on what the proper placement of these tools should be, and the discussion kind of fizzled out without reaching a conclusion. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 12:15:38 GMT, Alan Bateman wrote: >> Also, to clarify, for me there is a fundamental difference between >> `src/$MODULE` and `make/modules/$MODULE`. The former is the home of files >> that are part of the module, owned by the content team, and the `$MODULE` >> part is essential to delineate this content. The latter is owned by the >> build team, and is just a convenient way to organize the build system within >> the `make` directory. So it's clearly a no-no to put anything but `.gmk` >> files in `make/modules/$MODULE`. > > The mapping and nr tables, and the *-X.java.template files in > make/data/charsetmapping are used to generate source code for the java.base > and jdk.charsets modules. The stdcs-$OS files configure the package and > module that each charset go into. If the tables used to generate the source > files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk > will probably need a new home too. @AlanBateman The process of modularization was not fully completed with Project Jigsaw, and a few ugly warts remained. I was under the impression that these should be addressed in follow-up fixes, but this was unfortunately never done. Charsets and cldrconverter were both split between a core portion in java.base and the rest in jdk.charsets and jdk.localedata, respectively, but the split was never handled properly, but just "duct taped" in place. I chose to put the data files used for both java.base and the "additional" modules in java.base, based on the comment that Naoto made in https://mail.openjdk.java.net/pipermail/build-dev/2020-March/027044.html: > As to charsetmapping and cldrconverter, I believe they can reside in java.base, as jdk.charsets and jdk.localedata modules depend on it. Of course it would be preferable to make a proper split, but that requires work done by the component teams to break the modules apart. Specifically for make/modules/jdk.charsets/Gensrc.gmk; the code in that file is more or less duplicated in make/modules/java.base/gensrc/GensrcCharsetMapping.gmk, since the same data set is processed twice, once for java.base and once for jdk.charsets. I don't think that means that make/modules/jdk.charsets/Gensrc.gmk should move to any other place. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 12:15:38 GMT, Alan Bateman wrote: >> Also, to clarify, for me there is a fundamental difference between >> `src/$MODULE` and `make/modules/$MODULE`. The former is the home of files >> that are part of the module, owned by the content team, and the `$MODULE` >> part is essential to delineate this content. The latter is owned by the >> build team, and is just a convenient way to organize the build system within >> the `make` directory. So it's clearly a no-no to put anything but `.gmk` >> files in `make/modules/$MODULE`. > > The mapping and nr tables, and the *-X.java.template files in > make/data/charsetmapping are used to generate source code for the java.base > and jdk.charsets modules. The stdcs-$OS files configure the package and > module that each charset go into. If the tables used to generate the source > files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk > will probably need a new home too. Is there a plan to move make/jdk/src/classes/build/tools/intpoly into java.base as well? - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8257539: tools/jpackage/windows/WinL10nTest.java unpack.bat failed with Exit code: 1618 [v2]
> increase retries and timeout increasing in runMsiexecWithRetries Andy Herrick has updated the pull request incrementally with 18 additional commits since the last revision: - 8256149: Weird AST structure for incomplete member select Reviewed-by: vromero - 8257194: Add 'foreign linker API' in 'jdk.incubator.foreign' module desc/summary Reviewed-by: jvernee, shade - 8257848: -XX:CompileCommand=blackhole,* should be diagnostic Reviewed-by: vlivanov - 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input Reviewed-by: alanb - 8254733: HotSpot Style Guide should permit using range-based for loops Reviewed-by: dholmes, pliden, jrose, dcubed, iklam, eosterlund, tschatzl, kvn - 8253644: C2: assert(skeleton_predicate_has_opaque(iff)) failed: unexpected Reviewed-by: roland, kvn - 8256411: Based anonymous classes have a weird end position Reviewed-by: vromero - 8257813: [redo] C2: Filter type in PhiNode::Value() for induction variables of trip-counted integer loops Reviewed-by: chagedorn, kvn - 8257769: Cipher.getParameters() throws NPE for ChaCha20-Poly1305 Reviewed-by: mullan, valeriep - 8257855: Example SafeVarargsNotApplicableToRecordAccessors breaks test tools/javac/diags/CheckExamples.java Reviewed-by: jjg - ... and 8 more: https://git.openjdk.java.net/jdk/compare/7c4743c5...5d065497 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1676/files - new: https://git.openjdk.java.net/jdk/pull/1676/files/7c4743c5..5d065497 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1676=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1676=00-01 Stats: 1938 lines in 49 files changed: 1465 ins; 276 del; 197 mod Patch: https://git.openjdk.java.net/jdk/pull/1676.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1676/head:pull/1676 PR: https://git.openjdk.java.net/jdk/pull/1676
RFR: 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments
When calling a Windows tool that needs an argument on the form `-option/a:/home/ihse/myjdk/...`, fixpath gets confused and thinks `/a:/home/ihse/myjdk` is a unix path, notices that it does not exists, and stops trying to convert the path. Instead it needs to recheck for paths after a separator such as `:` or `=`. - Commit messages: - 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments Changes: https://git.openjdk.java.net/jdk/pull/1698/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1698=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8257905 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1698.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1698/head:pull/1698 PR: https://git.openjdk.java.net/jdk/pull/1698
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On 2020-12-08 00:30, Magnus Ihse Bursie wrote:
On Tue, 8 Dec 2020 02:40:43 GMT, Mandy Chung wrote:
I have reviewed all lines in the patch file with or near instances of
`jdk.compiler`
Hi Magnus,
I see the motivation of moving these build files for better identification of
ownership. Placing them under
`src/$MODULE/{share,$OS}/data` is one option. Given that skara will
automatically determine appropriate mailing lists of a PR, it seems that
`make/modules/$MODULE/data` can be another alternative that skara can include
this pattern in the mailing list configuration?? I don't yet have a strong
preference while I don't consider everything under `make` must be owned by the
build team though. Do you see one option is better than the other?
@mlchung If you don't have any strong preference, I implore you to accept this
change. I **vastly** prefer to move the data out of `make`!
This is not just about Skara tooling. When working with make files, autoconf and shell scripts,
there is no fancy IDE support, so you are stuck with simple text editors and tools like `grep`.
I've lost count on how many times I've had my grep searches blow up, since I happened to find e.g.
a string in `tzdata` and get hundreds or more of hits. :-( And I do believe we will get a better
code structure if the build team "owns" `make`; or at least has a vested interest in
what's in that directory. We still suffer a lot of the old "I don't know where to put this
file, so I'll just put it in make cause nobody cares about it anyway" mentality, but I've been
working for quite some time to make that list of misplaced files shorter and shorter.
I strongly agree with Magnus for all the same reasons. To me, the data
files are clearly a form of source code that should be considered owned
by the component teams. I'm honestly perplexed over why this is being
argued against.
/Erik
-
PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 08:32:28 GMT, Magnus Ihse Bursie wrote: >> @mlchung If you don't have any strong preference, I implore you to accept >> this change. I **vastly** prefer to move the data out of `make`! >> >> This is not just about Skara tooling. When working with make files, autoconf >> and shell scripts, there is no fancy IDE support, so you are stuck with >> simple text editors and tools like `grep`. I've lost count on how many times >> I've had my grep searches blow up, since I happened to find e.g. a string in >> `tzdata` and get hundreds or more of hits. :-( And I do believe we will get >> a better code structure if the build team "owns" `make`; or at least has a >> vested interest in what's in that directory. We still suffer a lot of the >> old "I don't know where to put this file, so I'll just put it in make cause >> nobody cares about it anyway" mentality, but I've been working for quite >> some time to make that list of misplaced files shorter and shorter. > > Also, to clarify, for me there is a fundamental difference between > `src/$MODULE` and `make/modules/$MODULE`. The former is the home of files > that are part of the module, owned by the content team, and the `$MODULE` > part is essential to delineate this content. The latter is owned by the build > team, and is just a convenient way to organize the build system within the > `make` directory. So it's clearly a no-no to put anything but `.gmk` files in > `make/modules/$MODULE`. The mapping and nr tables, and the *-X.java.template files in make/data/charsetmapping are used to generate source code for the java.base and jdk.charsets modules. The stdcs-$OS files configure the package and module that each charset go into. If the tables used to generate the source files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk will probably need a new home too. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 08:27:16 GMT, Magnus Ihse Bursie wrote:
>> Hi Magnus,
>>
>> I see the motivation of moving these build files for better identification
>> of ownership. Placing them under
>> `src/$MODULE/{share,$OS}/data` is one option. Given that skara will
>> automatically determine appropriate mailing lists of a PR, it seems that
>> `make/modules/$MODULE/data` can be another alternative that skara can
>> include this pattern in the mailing list configuration?? I don't yet have
>> a strong preference while I don't consider everything under `make` must be
>> owned by the build team though. Do you see one option is better than the
>> other?
>
> @mlchung If you don't have any strong preference, I implore you to accept
> this change. I **vastly** prefer to move the data out of `make`!
>
> This is not just about Skara tooling. When working with make files, autoconf
> and shell scripts, there is no fancy IDE support, so you are stuck with
> simple text editors and tools like `grep`. I've lost count on how many times
> I've had my grep searches blow up, since I happened to find e.g. a string in
> `tzdata` and get hundreds or more of hits. :-( And I do believe we will get a
> better code structure if the build team "owns" `make`; or at least has a
> vested interest in what's in that directory. We still suffer a lot of the old
> "I don't know where to put this file, so I'll just put it in make cause
> nobody cares about it anyway" mentality, but I've been working for quite some
> time to make that list of misplaced files shorter and shorter.
Also, to clarify, for me there is a fundamental difference between
`src/$MODULE` and `make/modules/$MODULE`. The former is the home of files that
are part of the module, owned by the content team, and the `$MODULE` part is
essential to delineate this content. The latter is owned by the build team, and
is just a convenient way to organize the build system within the `make`
directory. So it's clearly a no-no to put anything but `.gmk` files in
`make/modules/$MODULE`.
-
PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Tue, 8 Dec 2020 02:40:43 GMT, Mandy Chung wrote:
>> I have reviewed all lines in the patch file with or near instances of
>> `jdk.compiler`
>
> Hi Magnus,
>
> I see the motivation of moving these build files for better identification of
> ownership. Placing them under
> `src/$MODULE/{share,$OS}/data` is one option. Given that skara will
> automatically determine appropriate mailing lists of a PR, it seems that
> `make/modules/$MODULE/data` can be another alternative that skara can include
> this pattern in the mailing list configuration?? I don't yet have a strong
> preference while I don't consider everything under `make` must be owned by
> the build team though. Do you see one option is better than the other?
@mlchung If you don't have any strong preference, I implore you to accept this
change. I **vastly** prefer to move the data out of `make`!
This is not just about Skara tooling. When working with make files, autoconf
and shell scripts, there is no fancy IDE support, so you are stuck with simple
text editors and tools like `grep`. I've lost count on how many times I've had
my grep searches blow up, since I happened to find e.g. a string in `tzdata`
and get hundreds or more of hits. :-( And I do believe we will get a better
code structure if the build team "owns" `make`; or at least has a vested
interest in what's in that directory. We still suffer a lot of the old "I don't
know where to put this file, so I'll just put it in make cause nobody cares
about it anyway" mentality, but I've been working for quite some time to make
that list of misplaced files shorter and shorter.
-
PR: https://git.openjdk.java.net/jdk/pull/1611
