Re: Comments on jpackage (JEP 343)
> but I'd concede it to be "." as a default On second thoughts I am not sure about that either. I find it much cleaner to know what was generated by looking in a new directory rather than hunting in my current directory, especially for the default app-image case which will dump a bunch of unfamiliar files. -phil. On 9/16/19, 8:13 PM, Philip Race wrote: I've been thinking about this. output is nicely symmetrical with input and in the case of a default app-image it is more than a single item and personally I'd much rather it not clutter my working directory and again you get symmetry with input which you really want to specify but I'd concede it to be "." as a default over changing the name to dest ... There is also precedent : jlink uses -output and these two tools are going to be frequently used together. So I would like to see the status quo. -phil. On 9/3/19, 11:58 AM, mark.reinh...@oracle.com wrote: - The `--output`/`-o` option is confusing. It doesn’t name the output itself, but rather a directory into which the single item of output will be placed. Typing `-o .` all the time is just annoying. It’d be more logical to rename this option to `--dest`/`-d` and to make it optional, with a default value of `.`.
Re: Comments on jpackage (JEP 343)
I've been thinking about this. output is nicely symmetrical with input and in the case of a default app-image it is more than a single item and personally I'd much rather it not clutter my working directory and again you get symmetry with input which you really want to specify but I'd concede it to be "." as a default over changing the name to dest ... There is also precedent : jlink uses -output and these two tools are going to be frequently used together. So I would like to see the status quo. -phil. On 9/3/19, 11:58 AM, mark.reinh...@oracle.com wrote: - The `--output`/`-o` option is confusing. It doesn’t name the output itself, but rather a directory into which the single item of output will be placed. Typing `-o .` all the time is just annoying. It’d be more logical to rename this option to `--dest`/`-d` and to make it optional, with a default value of `.`.
Re: RFR: 8229773: Resolve permissions for code source URLs lazily
Hi Alan, Your suspicion is correct. :) Thanks for the leads, I'll look into it further. Currently the policy implementation finds policy url's in system properties, "java.security.policy" and numbered policy locations with the prefix "policy.url." if the "java.security.policy" property doesn't begin with "=" (which represents java.security.policy==). Cheers, Peter. On 15/09/2019 10:58 PM, Alan Bateman wrote: On 14/09/2019 21:21, Peter Firmstone wrote: Hi Alan, We've got a bunch of very old policy files in our test suite, so they still had policy grants using the extension directory property. The grant for the extension directory property was followed by a forward slash and asterix. Oddly when the property was missing the grant became a wildcard URL. Note this isn't the sun PolicyFile implementation, but our policy provider also augments, rather than replace, maybe there's a new policy file our provider isn't aware of? From memory there was something special about the way the extension directory property was treated by the policy provider, but I don't recall the details, the same problems don't appear to exist when other properties in policy files cannot be resolved. Modules that required permissions, seem to be service providers: In jdk/jdk repo, the following policy files are merged in the build to create the default policy: src/java.base/windows/lib/security/default.policy src/java.base/solaris/lib/security/default.policy src/java.base/share/lib/security/default.policy The default policy goes into a JDK internal location in the run-time image and used by the PolicyFile implementation. If you look in there you should see the permissions that are granted to the modules that map to the platform class loader. The intention is that deployments that are setting their own policy files don't need to be concerned about the permissions of modules in the run-time image. I suspect you are looking for a custom PolicyFile implementation to make use of these defaults to avoid needing to be concerned with the specific permissions that the modules in the run-time image. -Alan
Re: RFR [14/java.xml] 8230814: Enable SAX ContentHandler to handle XML Declaration
On 9/14/19 1:08 AM, Alan Bateman wrote: On 13/09/2019 21:50, Joe Wang wrote: : It can be said that the SAX project, in terms of the API work, was dead a long time ago. There hasn't been any updates/changes since SAX 2.0.2 released in 2004[1]. SAX is in public domain [2]. Sun/Oracle incorporated SAX2 in Java SE with a GPL license. I assume we're free to make changes. Please let me know if you think otherwise. I'm not objecting to notifying the content handler of declarations. I'm also not discussing licenses. I'm mostly concerned that ContentHandler and all the other classes in this API point the reader to the SAX project as the place to go for documentation and more information. Has there been any effort to find a contact for the SAX project on soucreforge and get them to put an EOL notice on the main page? Alternatively, if the SAX API in Java SE is getting a second wind then maybe the links to the SAX project could be reduced to a historical note. I deliberately left the javadoc in the SAX package as they are. But I agree it may be worth it to address this aspect of the document. I suggest we do so with a separate doc-only request[1] to clarify the relationship with the SAX project, likely adding a short note in the package description and removing all other references. If you are okay, I'll keep this changeset the way it is, limiting it to the new method. [1] https://bugs.openjdk.java.net/browse/JDK-8231083 -Joe -Alan
Re: [PATCH] Add *.iml to .hgignore and .gitignore
Intellij configuration files may appear in directories other than /.idea/. For example the script suggested by AdoptOpenJDK generates such in the root directory. пн, 16 сент. 2019 г., 9:33 Alan Bateman : > > I think the .ignore files are maintained on build-dev. Note that .idea > is already excluded and it contains the .iml and other workspace files > are created for the IntelliJ project, maybe your setup might be different? > > -Alan. > > > On 15/09/2019 21:22, JARvis PROgrammer wrote: > > This is a small patch to disable tracking of configuration files of > > Intellij-based IDEs (.iml) > > Diff: > > diff -r a6f653312b19 .gitignore > > --- a/.gitignoreSun Sep 15 08:41:48 2019 +0200 > > +++ b/.gitignoreSun Sep 15 21:11:13 2019 +0100 > > @@ -1,6 +1,7 @@ > > /build/ > > /dist/ > > /.idea/ > > +*.iml > > nbproject/private/ > > /webrev > > /.src-rev > > diff -r a6f653312b19 .hgignore > > --- a/.hgignore Sun Sep 15 08:41:48 2019 +0200 > > +++ b/.hgignore Sun Sep 15 21:11:13 2019 +0100 > > @@ -1,6 +1,7 @@ > > ^build/ > > ^dist/ > > ^.idea/ > > +*.iml > > nbproject/private/ > > ^webrev > > ^.src-rev$ > > Hosted diff-file: > > https://gist.github.com/JarvisCraft/d016d39a0342d09ce3a0a22a1893841d > > > > PS: if it is an unneeded patch or a wrong mailing list, please inform me > > > > Thanks, > > Peter > >
[jpackage] Customize msi installer
Hi, after experimenting with jpackage for creating a msi installer, I would like to suggest the following improvements: 1. Set the ARPPRODUCTICON property, that controls the icon displayed in Add/Remove Programs for your application. https://wixtoolset.org/documentation/manual/v3/howtos/ui_and_localization/co nfigure_arp_appearance.html e.g: (btw: it's strange that the icon ids end on exe) 2. Make it possible to customize the appearance of the installer. At least make it possible to specify WixUIBannerBmp and WixUIDialogBmp. I guess most people want to replace the default icons with their own ones. https://wixtoolset.org//documentation/manual/v3/wixui/wixui_customizations.h tml Best Tobias
RE: 8229471: Calendar under a specific timezone changes HOUR field when MILLISECOND field is changed
Hi Naoto,
Thank you for replying. I understand that this behavior is expected.
I will consider the existing behavior.
Regards,
Masanori Yano
> -Original Message-
> From: naoto.s...@oracle.com [mailto:naoto.s...@oracle.com]
> Sent: Friday, September 13, 2019 5:25 AM
> To: Yano, Masanori ;
> 'core-libs-dev@openjdk.java.net'
> Cc: i18n-...@openjdk.java.net
> Subject: Re: 8229471: Calendar under a specific timezone changes HOUR field
> when MILLISECOND field is changed
>
> Hi Masanori,
>
> Thank you for looking at the issue and your contribution. I am also
> investigating it (as an assignee of the bug), and looking at this old
> issue:
>
> https://bugs.openjdk.java.net/browse/JDK-4177484
>
> The comment suggests that the existing behavior is the expected one. In
> fact, your fix would break some regression test cases in
> jdk/java/util/Calendar/CalendarRegression.java.
>
> I will need some more archaeological investigation, but I am inclined to
> close it as not an issue at the moment.
>
> Naoto
>
> On 9/12/19 12:16 AM, Yano, Masanori wrote:
> > Hello.
> >
> > I think JDK-8229471 occurs because a change of TimeZone is not considered
> in getTime().
> > To resolve this problem, isTimeSet flag must be set to false when
> setTimeZone() is called.
> >
> > Please review the following change.
> >
> > diff -r e1269de19aa5
> src/java.base/share/classes/java/util/Calendar.java
> > --- a/src/java.base/share/classes/java/util/Calendar.java Thu Aug
> 22 14:09:36 2019 -0700
> > +++ b/src/java.base/share/classes/java/util/Calendar.java Thu Sep
> 12 11:45:37 2019 +0900
> > @@ -2901,7 +2901,7 @@
> >* generally, a call to setTimeZone() affects calls to set()
> BEFORE AND
> >* AFTER it up to the next call to complete().
> >*/
> > -areAllFieldsSet = areFieldsSet = false;
> > +isTimeSet = areAllFieldsSet = areFieldsSet = false;
> > }
> >
> > /**
> > diff -r e1269de19aa5 test/jdk/java/util/Calendar/Bug8229471.java
> > --- /dev/null Thu Jan 01 00:00:00 1970 +
> > +++ b/test/jdk/java/util/Calendar/Bug8229471.java Thu Sep 12
> 11:45:37 2019 +0900
> > @@ -0,0 +1,22 @@
> > +/*
> > + *@test
> > + *@bug 8229471
> > + *@summary test for recompute when Calendar.setTimeZone() */
> > +
> > +import java.util.Calendar;
> > +import java.util.Date;
> > +import java.util.Locale;
> > +import java.util.TimeZone;
> > +
> > +public class Bug8229471 {
> > +public static void main(String[] args) throws Exception {
> > +Calendar calendar = Calendar.getInstance(new Locale("en",
> "US"));
> > +Date date1 = calendar.getTime();
> > +
> calendar.setTimeZone(TimeZone.getTimeZone("Europe/Budapest"));
> > +Date date2 = calendar.getTime();
> > +if (date1.equals(date2)) {
> > +throw new RuntimeException("Bug8229471: failed. TimeZone
> is not applied.");
> > +}
> > +}
> > +}
> >
> > Regards,
> > Masanori Yano
> >
Re: [PATCH] Add *.iml to .hgignore and .gitignore
Hello, The .ignore file is currently in regexp format so new additions should probably be in that format. I don't object to ignoring .iml files. /Erik On 2019-09-15 23:33, Alan Bateman wrote: I think the .ignore files are maintained on build-dev. Note that .idea is already excluded and it contains the .iml and other workspace files are created for the IntelliJ project, maybe your setup might be different? -Alan. On 15/09/2019 21:22, JARvis PROgrammer wrote: This is a small patch to disable tracking of configuration files of Intellij-based IDEs (.iml) Diff: diff -r a6f653312b19 .gitignore --- a/.gitignore Sun Sep 15 08:41:48 2019 +0200 +++ b/.gitignore Sun Sep 15 21:11:13 2019 +0100 @@ -1,6 +1,7 @@ /build/ /dist/ /.idea/ +*.iml nbproject/private/ /webrev /.src-rev diff -r a6f653312b19 .hgignore --- a/.hgignore Sun Sep 15 08:41:48 2019 +0200 +++ b/.hgignore Sun Sep 15 21:11:13 2019 +0100 @@ -1,6 +1,7 @@ ^build/ ^dist/ ^.idea/ +*.iml nbproject/private/ ^webrev ^.src-rev$ Hosted diff-file: https://gist.github.com/JarvisCraft/d016d39a0342d09ce3a0a22a1893841d PS: if it is an unneeded patch or a wrong mailing list, please inform me Thanks, Peter
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
On 9/16/2019 6:06 AM, Alan Bateman wrote: On 16/09/2019 13:56, Kevin Rushforth wrote: In that case, it may be better to issue any warnings about it being experimental directly from jpackage instead of relying on the incubating modules. Incubating modules are incubating non-final APIs so I don't think it's the right solution here. I suspect what is needed is a combination of a warning message (from the tool) and documentation to make it clear that that the feature and CLI options aren't yet final. Yes, that's what I was thinking as well, now that it's clear that the incubating module approach isn't the right one to use. -- Kevin
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
On 16/09/2019 13:56, Kevin Rushforth wrote: In that case, it may be better to issue any warnings about it being experimental directly from jpackage instead of relying on the incubating modules. Incubating modules are incubating non-final APIs so I don't think it's the right solution here. I suspect what is needed is a combination of a warning message (from the tool) and documentation to make it clear that that the feature and CLI options aren't yet final. -Alan.
Re: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.
Hi Goetz, not a full review, just a small suggestion. In jvm.cpp you could just access ss->base() instead of ss->as_string() since the internal buffer is already NULL terminated and result_string does not outlive the stringStream object. Also it misses including ostream.hpp. Cheers, Thomas On Tue, Sep 10, 2019 at 4:46 PM Lindenmaier, Goetz < goetz.lindenma...@sap.com> wrote: > Hi, > > the subject should mention 8218628. (Not 8218626). > Sorry for this! > > Best regards, > Goetz. > > From: Lindenmaier, Goetz > Sent: Dienstag, 10. September 2019 11:48 > To: 'Hotspot dev runtime' ; Java > Core Libs > Subject: RFR (L, final): 8218626: Add detailed message to > NullPointerException describing what is null. > > Hi, > > this is the implementation of JEP 358: Helpful NullPointerExceptions. > > The JEP is in status "Candidate". Coleen (many, many thanks!) ran > it through the Oracle-internal processes. Now I please need final reviews > for this change so that I can propose it to target jdk 14. > > JEP: https://bugs.openjdk.java.net/browse/JDK-8220715 > Enhancement: https://bugs.openjdk.java.net/browse/JDK-8218628 > webrev: http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/16/ > > The change ran through a lot of testing, all jtreg and jck tests to name > only some. The webrev points to patch > > http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/16/enable_NPE_message.patch > that enables the change by default, which was useful for testing to > assure the code is used in the tests. > I just pushed the change to jdk/submit once more. > > Please review. > > Thanks! > Goetz. >
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
In that case, it may be better to issue any warnings about it being experimental directly from jpackage instead of relying on the incubating modules. -- Kevin On 9/15/2019 1:05 PM, Andy Herrick wrote: yes - result of this change is as you suggest, everything shows this warning, so we will need further discussions as to what it might mean to make jpackage "experimental". /Andy On 9/15/2019 12:35 PM, Alan Bateman wrote: On 15/09/2019 14:58, Andy Herrick wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This is the at least the first small set of changes that need to be make to make jpackage an experimental package. - Add flags to the build so the module jdk.jpackage is built as an experimental module. - Modify test programs to tolerate the warning emitted when jpackage is run. I think this change will need discussion. Can you provide a summary on what you mean by "experimental package"? I remember seeing Mark's comment go by where he suggested that the tool should be an experimental feature but I'm not sure if this translates to a warning or a configure option. I see the JIRA issue references the JEP for Incubating Modules but I'm not sure that it makes sense here as jdk.jpackage doesn't export an API and will eagerly participate in service binding because it `provides java.util.spi.ToolProvider`. There are subtle issues around incubating modules that want to provide services that were not worked out in the JDK 9 time frame. In this case, java.base uses ToolProvider so jdk.jpackage will be resolved when it is observable. I assum `java -version` will print a warning and that will not be welcomed. -Alan
Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo
Hi Brent, The StackWalker benchmark look good to me. However I have some doubts about the the logging benchmark. Is the benchmark run in a single thread? If not then there doesn't seem any guarantee that each call to publish() will be immediately followed by a call to reset(), but instead, if you have two threads, you could see things like: T1: publish(); T2: publish(); => T1 record would be lost here T1: reset(); T2: reset(); => reset() would return null here best regards, -- daniel On 13/09/2019 23:07, Brent Christian wrote: Hi, Please review these StackWalker and Throwable benchmarks for addition into the JDK microbenchmarks. Bug: https://bugs.openjdk.java.net/browse/JDK-8221623 Webrev: http://cr.openjdk.java.net/~bchristi/8221623/webrev07/ The StackWalker benchmarks use StackWalker's forEach(), walk(), and Options to measure retrieval of various types of information from the call stack. The Throwable benchmarks do corresponding exercises; there are also a couple of Logging benchmarks. A JMH @Param is used to test a variety of call stack depths. In the future, we might consider a benchmark for Reflection.getCallerClass(). (It is more involved today to benchmark that method than at the time these benchmarks were originally written, so that one's commented out.) See JDK-8230976. Thanks, -Brent 1. https://bugs.openjdk.java.net/browse/JDK-8230976
