Re: RFR: 8284209: Replace remaining usages of 'a the' in source code

[Daniel Fuchs]

On Wed, 18 May 2022 14:46:42 GMT, Alexey Ivanov  wrote:

> Replaces usages of articles that follow each other in all combinations: 
> a/the, an?/an?, the/the…
> 
> It's the last issue in the series, and it still touches different areas of 
> the code.

Logging/JNDI OK

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8771

Re: RFR: 8286744: failure_handler: dmesg command on macos fails to collect data due to permission issues [v2]

[Daniel Fuchs]

On Fri, 13 May 2022 14:36:02 GMT, Jaikiran Pai  wrote:

>> Can I please get a review of this change which proposes to fix the failure 
>> handler command `dmesg` on macOS?
>> 
>> As noted in the JBS issue, the command currently fails with permission 
>> error. The commit in this PR uses `sudo` as suggested in the man pages of 
>> that command.
>> 
>> Tested locally on macOS by running:
>> 
>> 
>> cd test/failure_handler
>> make clean
>> make test
>> 
>> Then checked the generated environment.html files for the dmesg command 
>> output. Looks fine after this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   copyright year

LGTM

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8703

Re: RFR: 8285915: failure_handler: gather the contents of /etc/hosts file

[Daniel Fuchs]

On Fri, 29 Apr 2022 11:28:32 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which addresses 
> https://bugs.openjdk.java.net/browse/JDK-8285915?
> 
> With this change, the environment details collected by the failure handler 
> will now include the contents of the `/etc/hosts/` file, which can be useful 
> in certain cases when debugging network tests that fail.
> 
> Testing done (on macOS):
> 
> 
> cd test/failure_handler 
> make test
> 
> Then verified that the generated environment.html had the `/etc/hosts` file 
> content

Thanks for doing this Jaikiran! That should be helpful. Please get approval 
from someone from build-dev before integrating.

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8466

Re: RFR: 8284890: Support for Do not fragment IP socket options [v8]

[Daniel Fuchs]

On Wed, 20 Apr 2022 14:30:21 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> Could I get the following PR review please? It adds a new JDK specific 
>> extended socket option
>> called IP_DONTFRAGMENT, which disables IP packet fragmentation in both IPv4 
>> and IPv6
>> UDP sockets (NIO DatagramChannels). For IPv4 in particular, it sets the DF 
>> (Dont Fragment) bit
>> in the IP header. There is no equivalent in the IPv6 packet header as 
>> fragmentation is implemented
>> exclusively by the sending and receiving nodes.
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   test update

Marked as reviewed by dfuchs (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8245

Re: RFR: 8284890: Support for Do not fragment IP socket options [v5]

[Daniel Fuchs]

On Tue, 19 Apr 2022 15:54:02 GMT, Michael McMahon  wrote:

>> test/jdk/jdk/net/ExtendedSocketOption/DontFragmentTest.java line 44:
>> 
>>> 42: StandardProtocolFamily fam = args[0].equals("ipv4") ? INET : 
>>> INET6;
>>> 43: System.out.println("Family = " + fam);
>>> 44: testDatagramChannel(args, fam);
>> 
>> Shouldn't there be a testcase for when DatagramChannel is opened using the 
>> no arg factory method `DatagramChannel.open()`?
>
> I'm not sure there is value in testing all of these permutations. 
> Distinguishing DatagramChannel and DatagramSocket probably made sense, but 
> it's all the same implementation under the hood.

1. `DatagramChannel.open()` => opens a dual socket unless 
`-Djava.net.preferIPv4Stack=true`, in which case it should be equivalent to 
`DatagramChannel.open(StandardProtocolFamily.INET)`
2. `DatagramChannel.open(StandardProtocolFamily.INET)` => opens an IPv4 socket
3. `DatagramChannel.open(StandardProtocolFamily.INET6)` => opens an IPv6 socket

So I believe it makes sense to test the no-arg constructor since that's the 
only way to open a dual socket.

-

PR: https://git.openjdk.java.net/jdk/pull/8245

Re: RFR: 8284890: Support for Do not fragment IP socket options [v5]

[Daniel Fuchs]

On Tue, 19 Apr 2022 14:47:01 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> Could I get the following PR review please? It adds a new JDK specific 
>> extended socket option
>> called IP_DONTFRAGMENT, which disables IP packet fragmentation in both IPv4 
>> and IPv6
>> UDP sockets (NIO DatagramChannels). For IPv4 in particular, it sets the DF 
>> (Dont Fragment) bit
>> in the IP header. There is no equivalent in the IPv6 packet header as 
>> fragmentation is implemented
>> exclusively by the sending and receiving nodes.
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   fix whitespace

src/jdk.net/windows/native/libextnet/WindowsSocketOptions.c line 112:

> 110: return optval;
> 111: }
> 112: handleError(env, rv, "get option IP_DONTFRAGMENT failed");

Is there some indentation issue here?

test/jdk/jdk/net/ExtendedSocketOption/DontFragmentTest.java line 44:

> 42: StandardProtocolFamily fam = args[0].equals("ipv4") ? INET : 
> INET6;
> 43: System.out.println("Family = " + fam);
> 44: testDatagramChannel(args, fam);

Shouldn't there be a testcase for when DatagramChannel is opened using the no 
arg factory method `DatagramChannel.open()`?

test/jdk/jdk/net/ExtendedSocketOption/DontFragmentTest.java line 47:

> 45: try (DatagramSocket c = new DatagramSocket()) {
> 46: testDatagramSocket(c);
> 47: }

Can't you test `MulticastSocket` in exactly the same way? Why is there a 
specific test method for `MulticastSocket`?

-

PR: https://git.openjdk.java.net/jdk/pull/8245

Re: RFR: 8284890: Support for Do not fragment IP socket options

[Daniel Fuchs]

On Thu, 14 Apr 2022 16:04:22 GMT, Michael McMahon  wrote:

> Hi,
> 
> Could I get the following PR review please? It adds a new JDK specific 
> extended socket option
> called IP_DONTFRAGMENT, which disables IP packet fragmentation in both IPv4 
> and IPv6
> UDP sockets (NIO DatagramChannels). For IPv4 in particular, it sets the DF 
> (Dont Fragment) bit
> in the IP header. There is no equivalent in the IPv6 packet header as 
> fragmentation is implemented
> exclusively by the sending and receiving nodes.
> 
> Thanks,
> Michael

test/jdk/jdk/net/ExtendedSocketOption/DontFragmentTest.java line 40:

> 38: 
> 39: public class DontFragmentTest {
> 40: 

Should we have a similar test for DatagramSocket / MulticastSocket / 
DatagramChannel.socket() ?

-

PR: https://git.openjdk.java.net/jdk/pull/8245

Re: RFR: 8277459: Add jwebserver tool [v5]

[Daniel Fuchs]

On Mon, 29 Nov 2021 13:57:13 GMT, Julia Boes  wrote:

>> This change introduces jwebserver, a dedicated JDK tool for the Simple Web 
>> Server. 
>> 
>> A description is provided in a follow-up comment.
>
> Julia Boes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update comment about exit code

LGTM!

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6497

Re: RFR: 8277459: Add jwebserver tool [v3]

[Daniel Fuchs]

On Wed, 24 Nov 2021 17:29:40 GMT, Julia Boes  wrote:

>> This change introduces jwebserver, a dedicated JDK tool for the Simple Web 
>> Server. 
>> 
>> A description is provided in a follow-up comment.
>
> Julia Boes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix whitespace error, add missing code tag

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/Main.java line 
63:

> 61: System.exit(ec);
> 62: }  // otherwise the server has been started successfully and runs 
> in
> 63:// another non-daemon thread.

I wonder if we should add:


// another non-daemon thread, or -h or -version have been passed and
// the main thread will simply exit normally.

-

PR: https://git.openjdk.java.net/jdk/pull/6497

Re: RFR: 8277847: Support toolGuide tag in class-level documentation

[Daniel Fuchs]

On Fri, 26 Nov 2021 14:31:04 GMT, Julia Boes  wrote:

>> make/jdk/src/classes/build/tools/taglet/ToolGuide.java line 159:
>> 
>>> 157: .toString()
>>> 158: .replace('.', '/')
>>> 159: .replaceAll("[^/]+", "..");
>> 
>> If the class is in a module don't you have to get one step higher to get the 
>> root?
>> I am not familiar with this code, so I'm just reasoning by induction here - 
>> trying to match with what the case for PACKAGE seems to be doing...
>
> Same here, I initially applied the same pattern as for PACKAGE but that does 
> not produce the correct path (it includes 1 ".." too much.):
> 
> String typePart = te.getQualifiedName()
> .toString()
> .replace('.', '/')
> .replaceAll("[^/]+", "..");
> return te.getEnclosingElement().getEnclosingElement() != null
> ? "../" + typePart
> : typePart;

I see - class has a class name which is a file - so `foo.Bar` produces `../..` 
relative to the directory foo (foo/../..) - so it already has an additional 
`..` compared to package - where `a.b` would produce ../.. relative to 
*directory* b which would lead to `b/../..` and not `a/../..` which is what we 
need.
The difference comes from the fact that `Bar` is a file whereas `b` is a 
directory.

-

PR: https://git.openjdk.java.net/jdk/pull/6566

Re: RFR: 8277847: Support toolGuide tag in class-level documentation

[Daniel Fuchs]

On Thu, 25 Nov 2021 15:58:58 GMT, Julia Boes  wrote:

> This change adds support for the `@toolGuide` tag in class-level API 
> documentation. 
> 
> (A use case is the jwebserver tool, where the 
> com.sun.net.httpserver.SimpleFileServer class provides API points for 
> extension and customization of the underlying server and its components. 
> Linking to the tool's man page in the class-level documentation would be 
> beneficial.)

make/jdk/src/classes/build/tools/taglet/ToolGuide.java line 159:

> 157: .toString()
> 158: .replace('.', '/')
> 159: .replaceAll("[^/]+", "..");

If the class is in a module don't you have to get one step higher to get the 
root?
I am not familiar with this code, so I'm just reasoning by induction here - 
trying to match with what the case for PACKAGE seems to be doing...

-

PR: https://git.openjdk.java.net/jdk/pull/6566

Re: RFR: 8277459: Add jwebserver tool [v2]

[Daniel Fuchs]

On Wed, 24 Nov 2021 17:15:35 GMT, Julia Boes  wrote:

>> This change introduces jwebserver, a dedicated JDK tool for the Simple Web 
>> Server. 
>> 
>> A description is provided in a follow-up comment.
>
> Julia Boes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   address PR comments:
>   * update module and package summary
>   * add -version / --version option to usage
>   * fix markdown formatting

src/jdk.httpserver/share/classes/module-info.java line 39:

> 37:  * The {@link com.sun.net.httpserver.spi} package specifies a Service 
> Provider
> 38:  * Interface (SPI) for locating HTTP server implementations based on the
> 39:  * com.sun.net.httpserver API.

Nit: maybe this should be ` * {@code com.sun.net.httpserver} API.`

-

PR: https://git.openjdk.java.net/jdk/pull/6497

Re: RFR: 8277459: Add jwebserver tool

[Daniel Fuchs]

On Mon, 22 Nov 2021 09:43:19 GMT, Julia Boes  wrote:

> This change introduces jwebserver, a dedicated JDK tool for the Simple Web 
> Server. 
> 
> A description is provided in a follow-up comment.

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/resources/simpleserver.properties
 line 45:

> 43: -p, --port- Port to listen on. Default: 8000.\n\
> 44: -h, -?, --help- Print this help message.\n\
> 45: -version, --version   - Print version information.\n\

Should this be " - Print version information and exit."

-

PR: https://git.openjdk.java.net/jdk/pull/6497

Re: RFR: 8277459: Add jwebserver tool

[Daniel Fuchs]

On Mon, 22 Nov 2021 09:43:19 GMT, Julia Boes  wrote:

> This change introduces jwebserver, a dedicated JDK tool for the Simple Web 
> Server. 
> 
> A description is provided in a follow-up comment.

src/jdk.httpserver/share/classes/com/sun/net/httpserver/SimpleFileServer.java 
line 110:

> 108:  *
> 109:  * A simple HTTP file server implementation is provided via the
> 110:  * {@code jwebserver} tool.

Maybe an `@toolGuide jwebserver` javadoc tag could also be added here too, at 
line 111?

src/jdk.httpserver/share/classes/module-info.java line 30:

> 28:  * for running a minimal HTTP server.
> 29:  *
> 30:  * The com.sun.net.httpserver package defines a high-level API for 
> building

Maybe that should be " The {@link com.sun.net.httpserver} package defines 
..."

src/jdk.httpserver/share/classes/module-info.java line 34:

> 32:  * simple HTTP-only file server intended for testing, development and 
> debugging
> 33:  * purposes. A default implementation is provided via the {@code 
> jwebserver} tool
> 34:  * and the main entry point of the module, which can be invoked with

should we say "which can also be invoked ..." ?

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/SimpleFileServerImpl.java
 line 74:

> 72:  * @param  writer the writer to which output should be written
> 73:  * @param  args the command line options
> 74:  * @param launcher the launcher the server is started from

should this be aligned like the other `@param` tags?

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/resources/simpleserver.properties
 line 30:

> 28: \  [-o none|info|verbose] [-h to show options]
> 29: 
> 30: usage.jwebserver=\

should usage also show `-version, --version` in both usage strings, and in the 
usage summary of the man page below as well?

-

PR: https://git.openjdk.java.net/jdk/pull/6497

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v8]

[Daniel Fuchs]

On Wed, 29 Sep 2021 11:02:05 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update output for all interfaces

Marked as reviewed by dfuchs (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v7]

[Daniel Fuchs]

On Tue, 28 Sep 2021 10:08:29 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 20 commits:
> 
>  - use ipv4/ipv6 specific loopback address and add add how-to output for all 
> interfaces
>  - Merge branch 'master' into simpleserver
>  - change default bind address from anylocal to loopback
>  - address PR comments
>  - Merge branch 'master' into simpleserver
>  - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
>  - Merge branch 'master' into simpleserver
>  - refactor isHidden,isReadable,isSymlink checks and cleanup tests
>  - Merge branch 'master' into simpleserver
>  - check isHidden, isSymlink, isReadable for all path segments 
>  - ... and 10 more: 
> https://git.openjdk.java.net/jdk/compare/87998565...7f994476

Thanks Julia, the new changes look good to me.

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v6]

[Daniel Fuchs]

On Wed, 22 Sep 2021 15:26:33 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - change default bind address from anylocal to loopback
>  - address PR comments

When the -b option is not explicitly specified on the command line it would be 
good to print a message that says that the server is bound to the loopback by 
default, and print the command line that would be needed to bind to all 
interfaces instead (or instruct the user to call `java -m jdk.httpserver 
--help` to learn how to bind to all interfaces). I don't think your latest 
changes include that.

src/jdk.httpserver/share/classes/module-info.java line 55:

> 53:  *  [-o none|info|verbose] [-h to show 
> options]
> 54:  *Options:
> 55:  *-b, --bind-address- Address to bind to. Default: 127.0.0.1 
> (loopback).

This assumes that the machine on which the server is run has IPv4 configured. 
It might not be the case. It can also depend on whether 
-Djdk.net.preferIPv6Addresses=true is specified on the java command line. Maybe 
this should be: `127.0.0.1 (or ::1), (loopback).`

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/SimpleFileServerImpl.java
 line 52:

> 50:  *
> 51:  *  Unless specified as arguments, the default values are:
> 52:  * bind address: 127.0.0.1 (loopback)

maybe this should say: `127.0.01 (or ::1), (loopback)`

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/resources/simpleserver.properties
 line 32:

> 30: options=\
> 31: Options:\n\
> 32: -b, --bind-address- Address to bind to. Default: 127.0.0.1 
> (loopback).\n\

This assumes that the machine on which the server is run has IPv4 configured. 
It might not be the case. It can also depend on whether 
-Djdk.net.preferIPv6Addresses=true is specified on the java command line. The 
actual address of the loopback (as returned by 
InetAddress.getLoopackeAddress()) should therefore preferably be passed as 
parameter to any message that talks about the loopback. It is not such an issue 
for the wildcard - because AFAIU there's no difference between 0.0.0.0 and ::0 
at the OS level

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/resources/simpleserver.properties
 line 42:

> 40: opt.bindaddress=\
> 41: \-b, --bind-address- Address to bind to. Default: 127.0.0.1 
> (loopback).\n\
> 42: \ For 0.0.0.0 (all interfaces) use -b 0.0.0.0 or 
> -b ::0.

is `opt.bindaddress` used somewhere? I couldn't find it.
Same for the other `opt.*` properties below.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]

[Daniel Fuchs]

On Tue, 21 Sep 2021 16:03:28 GMT, Hannes Wallnöfer  wrote:

>> Hmm... When printing messages on the console don't we want a localized date? 
>> Or are you referring to other usages of the date formatter?  Worth double 
>> checking in any case.
>
> The problem I was referring to was not about printing to the console. I 
> hadn't thought about that, I agree the default locale should be used there. I 
> was referring to `Last-modified` HTTP headers with a non-English date value, 
> e.g.:
> 
> Last-modified: Di., 21 Sep. 2021 09:56:53 GMT
> 
> I think browsers will get confused by this. It probably isn't a big deal 
> since I think the server doesn't implement conditional GETs (i.e. use the 
> last-modified date in subsequent requests), but the HTTP spec is [quite 
> strict ][1] about date formats.
> 
> [1]: https://datatracker.ietf.org/doc/html/rfc2616#section-3.3.1

Thanks for clarifying. Yes the date should be in GMT and should not be 
localized when sent in a `Last-Modified` / `Date` headers. Good catch!

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]

[Daniel Fuchs]

On Tue, 21 Sep 2021 14:09:54 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 16 commits:
> 
>  - Merge branch 'master' into simpleserver
>  - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
>  - Merge branch 'master' into simpleserver
>  - refactor isHidden,isReadable,isSymlink checks and cleanup tests
>  - Merge branch 'master' into simpleserver
>  - check isHidden, isSymlink, isReadable for all path segments 
>  - add checks for all path segments
>  - Merge branch 'master' into componentcheck
>  - Merge branch 'master' into simpleserver
>  - improve output on startup
>  - ... and 6 more: 
> https://git.openjdk.java.net/jdk/compare/6d91a3eb...fe059131

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java
 line 314:

> 312: + "\n");
> 313: try (var paths = Files.list(path)) {
> 314: paths.filter(p -> !isHiddenOrSymLink(p))

Shouldn't we filter paths that are not readable here too? There's no point in 
printing a link that will result in 404, is there?

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]

[Daniel Fuchs]

On Tue, 21 Sep 2021 15:06:09 GMT, Hannes Wallnöfer  wrote:

>> test/jdk/com/sun/net/httpserver/simpleserver/SecurityManagerTest.java line 
>> 198:
>> 
>>> 196: 
>>> 197: static final DateTimeFormatter HTTP_DATE_FORMATTER =
>>> 198: DateTimeFormatter.ofPattern("EEE, dd MMM  HH:mm:ss v");
>> 
>> I think this and other usages of `DateTimeFormatter.ofPattern` need to be 
>> called with `Locale.US` (or something similar) as second argument, otherwise 
>> the current default locale will be used. I noticed because on my laptop the 
>> `Last-modified` header contains a german date.
>
> I just realized I commented on a test file, while the actual culprit is in 
> `FileServerHandler.java`. But I guess it applies to all usages of this class 
> and method.

Hmm... When printing messages on the console don't we want a localized date? Or 
are you referring to other usages of the date formatter?  Worth double checking 
in any case.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v4]

[Daniel Fuchs]

On Mon, 20 Sep 2021 16:09:14 GMT, Daniel Fuchs  wrote:

>> Julia Boes has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 12 commits:
>> 
>>  - Merge branch 'master' into simpleserver
>>  - check isHidden, isSymlink, isReadable for all path segments 
>>  - add checks for all path segments
>>  - Merge branch 'master' into componentcheck
>>  - Merge branch 'master' into simpleserver
>>  - improve output on startup
>>  - correct path handling
>>  - small spec rewording
>>  - add module main class to symbolgenerator
>>  - remove UnmodifiableHeaders constant
>>  - ... and 2 more: 
>> https://git.openjdk.java.net/jdk/compare/4d95a5d6...10523290
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java
>  line 340:
> 
>> 338: }
>> 339: }
>> 340: return false;
> 
> This will start checking from the root of the file system. I believe we want 
> to start checking from the root of the FileServerHandler, root excluded.

Maybe these checks should be made in `mapToPath` instead since you already walk 
the path there - and IIRC returning null from `mapToPath` will cause HTTP 404.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v4]

[Daniel Fuchs]

On Mon, 20 Sep 2021 15:28:05 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 12 commits:
> 
>  - Merge branch 'master' into simpleserver
>  - check isHidden, isSymlink, isReadable for all path segments 
>  - add checks for all path segments
>  - Merge branch 'master' into componentcheck
>  - Merge branch 'master' into simpleserver
>  - improve output on startup
>  - correct path handling
>  - small spec rewording
>  - add module main class to symbolgenerator
>  - remove UnmodifiableHeaders constant
>  - ... and 2 more: 
> https://git.openjdk.java.net/jdk/compare/4d95a5d6...10523290

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java
 line 340:

> 338: }
> 339: }
> 340: return false;

This will start checking from the root of the file system. I believe we want to 
start checking from the root of the FileServerHandler, root excluded.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

[Daniel Fuchs]

On Tue, 14 Sep 2021 16:47:45 GMT, Daniel Fuchs  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java 
> line 129:
> 
>> 127:  * response body bytes are a {@code UTF-8} encoded byte 
>> sequence of
>> 128:  * {@code body}. The response {@linkplain 
>> HttpExchange#sendResponseHeaders(int, long) is sent}
>> 129:  * with the given {@code statusCode} and the body bytes' length. 
>> The body
> 
> That might give the impression that chunked encoding will be used if the body 
> length is 0. I wonder if it should say instead:
> 
> 
> with the given {@code statusCode} and a {@code Content-Length} field set to 
> the body bytes' length.

Or maybe - which would be more accurate:


with the given {@code statusCode} and the body bytes' length (or {@code -1} if 
the body is empty).

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

[Daniel Fuchs]

On Tue, 14 Sep 2021 08:52:37 GMT, Julia Boes  wrote:

> This change implements a simple web server that can be run on the 
> command-line with `java -m jdk.httpserver`.
> 
> This is facilitated by adding an entry point for the `jdk.httpserver` module, 
> an implementation class whose main method is run when the above command is 
> executed. This is the first such module entry point in the JDK.
> 
> The server is a minimal HTTP server that serves the static files of a given 
> directory, similar to existing alternatives on other platforms and convenient 
> for testing, development, and debugging.
> 
> Additionally, a small API is introduced for programmatic creation and 
> customization.
> 
> Testing: tier1-3.

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java line 
129:

> 127:  * response body bytes are a {@code UTF-8} encoded byte 
> sequence of
> 128:  * {@code body}. The response {@linkplain 
> HttpExchange#sendResponseHeaders(int, long) is sent}
> 129:  * with the given {@code statusCode} and the body bytes' length. The 
> body

That might give the impression that chunked encoding will be used if the body 
length is 0. I wonder if it should say instead:


with the given {@code statusCode} and a {@code Content-Length} field set to the 
body bytes' length.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

[Daniel Fuchs]

On Tue, 14 Sep 2021 12:38:19 GMT, Jim Laskey  wrote:

>> src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java 
>> line 128:
>> 
>>> 126:  *  {@code headers} are the effective headers of the response. 
>>> The
>>> 127:  * response body bytes are a {@code UTF-8} encoded byte 
>>> sequence of
>>> 128:  * {@code body}. The response {@linkplain 
>>> HttpExchange#sendResponseHeaders(int, long) is sent}
>> 
>> Not sure what the javadoc looks like here, but it looks like the link is "is 
>> sent". Curious.
>
> Just a rewording, maybe.

Hmm... Maye that should be "The response headers *are sent*". The non-obvious 
technical details is that the response headers are sent before the body - as 
soon as you call  `sendResponseHeaders`. The link is here to provide a better 
understanding of the workings of the new Handler. The headers are sent by 
calling `sendResponseHeaders` on the HttpExchange.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

[Daniel Fuchs]

On Tue, 14 Sep 2021 16:07:21 GMT, Julia Boes  wrote:

>> src/jdk.httpserver/share/classes/com/sun/net/httpserver/Headers.java line 
>> 288:
>> 
>>> 286: }
>>> 287: 
>>> 288: private static final Headers EMPTY = new UnmodifiableHeaders(new 
>>> Headers());
>> 
>> IDEA warns here:
>>>Referencing subclass UnmodifiableHeaders from superclass Headers initializer 
>>>might lead to class loading deadlock
>>>Such references can cause JVM-level deadlocks in multithreaded environment, 
>>>when one thread tries to load the superclass and another thread tries to 
>>>load the subclass at the same time.
>
> Interesting, thanks for noting. We can return a new instance instead on line 
> 307, the only place the constant is used.

Or you could possibly define the constant in UnmodifiableHeaders instead.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

[Daniel Fuchs]

On Tue, 14 Sep 2021 13:19:17 GMT, Andrey Turbanov 
 wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> src/jdk.httpserver/share/classes/com/sun/net/httpserver/Headers.java line 106:
> 
>> 104: var h = headers.entrySet().stream()
>> 105: .collect(Collectors.toUnmodifiableMap(
>> 106: Entry::getKey, e -> new 
>> LinkedList<>(e.getValue(;
> 
> I wonder, what the reason of  `LinkedList` usages here?
> As I know ArrayList is faster in all real-world scenarios.

Hi Andrey, IIRC from when I reviewed this code the current implementation of 
`Headers` already uses `LinkedList` in other places - so this preserves the 
concrete list implementation class that `Headers` uses (see `Headers::add`). 
Not that it matters much - but if we wanted to replace `LinkedList` with 
`ArrayList`  I believe we should do it consistently in this class - and 
probably outside of the realm of this JEP.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8273092: Sort classlist in JDK image [v2]

[Daniel Fuchs]

On Tue, 31 Aug 2021 01:05:05 GMT, Ioi Lam  wrote:

>> When the classlist is generated using build.tools.classlist.HelloClasslist, 
>> its contents may be non-deterministic due to Java thread execution order.
>> 
>> We should sort the generated classlist to make the JDK image's contents more 
>> deterministic.
>> 
>> Tested with Mach5 tier1, tier2, builds-tier5
>
> Ioi Lam has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   @dfuch comments

Thanks Ioi! These changes look good to me.

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5288

Re: RFR: 8273092: Sort classlist in JDK image

[Daniel Fuchs]

On Fri, 27 Aug 2021 23:12:52 GMT, Ioi Lam  wrote:

> When the classlist is generated using build.tools.classlist.HelloClasslist, 
> its contents may be non-deterministic due to Java thread execution order.
> 
> We should sort the generated classlist to make the JDK image's contents more 
> deterministic.
> 
> Tested with Mach5 tier1, tier2, builds-tier5

make/jdk/src/classes/build/tools/classlist/SortClasslist.java line 58:

> 56: String line = scanner.nextLine();
> 57: Matcher m = p.matcher(line);
> 58: if (m.find()) {

Are we sure that a comment line will not match this regexp, or that if it 
matches, it is not a comment line?

-

PR: https://git.openjdk.java.net/jdk/pull/5288

Re: RFR: 8272805: Avoid looking up standard charsets [v2]

[Daniel Fuchs]

On Sun, 22 Aug 2021 23:02:06 GMT, Sergey Bylokhov  wrote:

>> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>> 
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>> 
>> This could be done more efficiently(up to x20 time faster) with use of 
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>> 
>> The later variant also makes the code cleaner, as it is known not to throw 
>> UnsupportedEncodingException in contrary to the former variant.
>> 
>> This change includes:
>>  * demo/utils
>>  * jdk.xx packages
>>  * Some places were missed in the previous changes. I have found it by 
>> tracing the calls to the Charset.forName() by executing tier1,2,3 and 
>> desktop tests.
>> 
>> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>> 
>> Code excluded in this fix: the Xerces library(should be fixed upstream), 
>> J2DBench(should be compatible to 1.4), some code in the network(the change 
>> there are not straightforward, will do it later).
>> 
>> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
>
> Sergey Bylokhov 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:
> 
>  - Update the usage of Files.readAllLines()
>  - Update datatransfer
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Fix related imports
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Cleanup UnsupportedEncodingException
>  - Update PacketStream.java
>  - Rollback TextTests, should be compatible with jdk1.4
>  - Rollback TextRenderTests, should be compatible with jdk1.4
>  - ... and 4 more: 
> https://git.openjdk.java.net/jdk/compare/db47f6e1...e7127644

Changes to http server look good to me.

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5210

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Daniel Fuchs]

On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

src/java.base/share/classes/java/lang/SecurityManager.java line 104:

> 102:  * method will throw an {@code UnsupportedOperationException}). If the
> 103:  * {@systemProperty java.security.manager} system property is set to the
> 104:  * special token "{@code allow}", then a security manager will not be 
> set at

Can/should the `{@systemProperty ...}` tag be used more than once for a given 
system property? I thought it should be used only once, at the place where the 
system property is defined. Maybe @jonathan-gibbons can offer some more 
guidance on this.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8188055: (ref) Add Reference::refersTo predicate [v6]

[Daniel Fuchs]

On Wed, 28 Oct 2020 08:54:31 GMT, Peter Levart  wrote:

>> Some thoughts regarding the parameter type of refersTo. Summary: I think 
>> `refersTo(T)` is fine and that we don't want to change it to 
>> `refersTo(Object)`.
>> 
>> I don't think we have a migration issue similar to generifying collections, 
>> where there was a possibility of changing `contains(Object)` to 
>> `contains(E)`. If that had been done, it would have been a source 
>> compatibility issue, because changing the signature of the method 
>> potentially affects existing code that calls the method. That doesn't apply 
>> here because we're adding a new method.
>> 
>> The question now falls to whether it's preferable to have more convenience 
>> with `refersTo(Object)` or more type-safety with `refersTo(T)`. With the 
>> generic collections issue, the migration issue probably drove the decision 
>> to keep `contains(Object)`, but this has resulted in a continual set of 
>> complaints about the lack of an error when code passes an instance of the 
>> "wrong" type. I think that kind of error is likely to occur with `refersTo`. 
>> Since we don't have a source compatibility issue here, we can choose the 
>> safer API and avoid this kind of problem entirely.
>> 
>> The safer API does raise the possibility of having to add inconvenient 
>> unchecked casts and local variables in certain places, but I think Mandy's 
>> comment about the code already having a reference of the "right" type is 
>> correct. Her prototype webrev linked above shows that having to add 
>> unchecked casts is fairly infrequent.
>
>> Some thoughts regarding the parameter type of refersTo. Summary: I think 
>> `refersTo(T)` is fine and that we don't want to change it to 
>> `refersTo(Object)`.
>> 
> I agree that we don't have a migration problem here that collections had. So 
> let it be `refersTo(T)` then.

I agree as well.

-

PR: https://git.openjdk.java.net/jdk/pull/498

RFR: 8205945 - Revert unintended changes to make/gensrc/Gensrc-jdk.hotspot.agent.gmk

[Daniel Fuchs]

[resending with correct subject]
Hi,

https://bugs.openjdk.java.net/browse/JDK-8205945
8205945: Revert unintended changes to 
make/gensrc/Gensrc-jdk.hotspot.agent.gmk


My fix for  8205397 [1] includes a change to 
make/gensrc/Gensrc-jdk.hotspot.agent.gmk that was not intended:


diff --git a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk 
b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk

--- a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
+++ b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
@@ -49,6 +49,7 @@
   MACH_EXC_SERVER := $(MIG_OUTPUT_DIR)/mach_excServer.c

   $(MACH_EXC_SERVER): $(SDKROOT)/usr/include/mach/mach_exc.defs
+ $(call MakeTargetDir)
$(MIG) -isysroot $(SDKROOT) -server $@ -user $(MACH_EXC_USER) \
-header $(MACH_EXC_HEADER) 
$(SDKROOT)/usr/include/mach/mach_exc.defs


This patch is to revert the unintented change:

diff --git a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk 
b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk

--- a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
+++ b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
@@ -49,7 +49,6 @@
   MACH_EXC_SERVER := $(MIG_OUTPUT_DIR)/mach_excServer.c

   $(MACH_EXC_SERVER): $(SDKROOT)/usr/include/mach/mach_exc.defs
-   $(call MakeTargetDir)
$(MIG) -isysroot $(SDKROOT) -server $@ -user $(MACH_EXC_USER) \
-header $(MACH_EXC_HEADER) $(SDKROOT)/usr/include/mach/mach_exc.defs


My deepest apologies for the mistake :-(

-- daniel

[1] http://hg.openjdk.java.net/jdk/jdk/rev/d21a3d3aa4fb

RFR: Revert unintended changes to make/gensrc/Gensrc-jdk.hotspot.agent.gmk

[Daniel Fuchs]

Hi,

My fix for  8205397 [1] includes a change to 
make/gensrc/Gensrc-jdk.hotspot.agent.gmk that was not intended:


diff --git a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk 
b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk

--- a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
+++ b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
@@ -49,6 +49,7 @@
   MACH_EXC_SERVER := $(MIG_OUTPUT_DIR)/mach_excServer.c

   $(MACH_EXC_SERVER): $(SDKROOT)/usr/include/mach/mach_exc.defs
+ $(call MakeTargetDir)
$(MIG) -isysroot $(SDKROOT) -server $@ -user $(MACH_EXC_USER) \
-header $(MACH_EXC_HEADER) 
$(SDKROOT)/usr/include/mach/mach_exc.defs


This patch is to revert the unintented change:

diff --git a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk 
b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk

--- a/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
+++ b/make/gensrc/Gensrc-jdk.hotspot.agent.gmk
@@ -49,7 +49,6 @@
   MACH_EXC_SERVER := $(MIG_OUTPUT_DIR)/mach_excServer.c

   $(MACH_EXC_SERVER): $(SDKROOT)/usr/include/mach/mach_exc.defs
-   $(call MakeTargetDir)
$(MIG) -isysroot $(SDKROOT) -server $@ -user $(MACH_EXC_USER) \
-header $(MACH_EXC_HEADER) $(SDKROOT)/usr/include/mach/mach_exc.defs


My deepest apologies for the mistake :-(

-- daniel

[1] http://hg.openjdk.java.net/jdk/jdk/rev/d21a3d3aa4fb

Re: RFR: JDK-8204973: Add build support for filtering translations

[Daniel Fuchs]

Hi Erik,

The modifications to the logging test look good to me.
Caveat: I don't speak chinese nor japanese ;-)

cheers,

-- daniel

On 13/06/18 20:47, Erik Joelsson wrote:

Hello,

Oracle will reduce the number of languages that it maintains 
translations of JDK resources for. The current translations will remain 
in the source for now, but we need a way to filter out a set of 
translations at build time so that we only include the ones we support. 
This patch adds such a configuration option. It also changes how Oracle 
builds by using the option to exclude all translations except English, 
Japanese, Simplified Chinese and Traditional Chinese. Anyone else 
building OpenJDK will by default include all translations present in the 
source, just as before.


I added a test that verifies this for builds with the "IMPLEMENTOR" 
field in the release file set to "Oracle Corporation". The test will not 
be run for other OpenJDK builds.


I had to modify an existing test for java.logging which used various 
translations to verify localized log messages to only use translations 
that Oracle chooses to include.


Since this is the second test that specifically verifies build behavior, 
I moved the previous such test together with this new test into a common 
top level test directory "build", under the jdk test root. I put these 
tests in the jdk tier3 test group.


I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically 
looked for tests that use the java.util.Locale class and ran them locally.


Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html

Bug: https://bugs.openjdk.java.net/browse/JDK-8204973

/Erik

Re: RFR: JDK-8190702: JMC packaging causing errors in JDK 10 consolidated repo (macOS)

[Daniel Fuchs]

Thanks for that Erik!
It's been annoying me for a couple of days :-)

I have imported your patch locally in my repo and it seems to
be working fine.

Hopefully someone from the build team will give you a thumbs up!

best regards,

-- daniel


On 14/11/2017 22:35, Erik Joelsson wrote:
When I added support for copying files with spaces in their names, I 
missed a case. On Mac (the only platform where we currently, in the 
Oracle build, have a directory with space in it), when rebuilding images 
after actually touching the source, the build fails with:


cp: 
/Users/danielfuchs/workspaces/jdk/jdk10/build/macosx-x64/images/jdk/lib/missioncontrol/Java 
Mission Control.app/Contents/Eclipse/jmc.ini: No such file or directory


This is caused by the $(call MakeDir, $(@D)) line in the install-file 
macro not actually creating the parent directory before trying to copy 
the file. This in turn was caused by two things:


First, if a target file that contains spaces, that is defined using the 
? wildcard trick, exists when the makefile is parsed, make will resolve 
those wildcards to spaces, so the target file variable ($@) will have 
spaces in it instead of ?. This has the consequence that $(@D), which is 
just a shortcut for $(dir $@) will actually apply the dir call on each 
space separated entity in $@. So the first fix was to make sure the 
input to $(dir ) is actually SpaceEncoded.


Second, the wildcard function seem to be doing some kind of clever 
caching of results and this caching breaks down when there are spaces in 
filenames. Because of this, if the argument of $(wildcard ) existed when 
the makefile was parsed, but was then deleted as part of a prerequisite 
rule, $(wildcard ) will still return the file as existing when called 
from a subsequent recipe line. This only happens if the file argument to 
wildcard contains spaces, so my best guess is that some string matching 
is happening in some caching table and fails (but I haven't actually 
checked the gnu make source). My fix for this is to change the MakeDir 
macro so that it always runs mkdir if the target dir contains ?.


I also have modified the copy-files tests to reproduce this situation 
and verified that the fix solves it there as well.


Bug: https://bugs.openjdk.java.net/browse/JDK-8190702

Webrev: http://cr.openjdk.java.net/~erikj/8190702/webrev.01/

/Erik

Re: Review Request: JDK-8173858: Rename libmanagement_rmi to libmanagement_agent

[Daniel Fuchs]

+1

libmanagement_agent is indeed a better name :-)

best regards,

-- daniel

On 02/02/17 22:27, Mandy Chung wrote:

libmanagement_agent should be the proper library name for jdk.management.agent. 
 It was an oversight with the current name.

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173858/webrev.00/

This patch also takes out the qualified exports of java.base/jdk.internal.vm  
to java.management which is not needed.

Mandy

Heads up: Fwd: hg: jdk9/dev/jdk: 8173607: JMX RMI connector should be in its own module

[Daniel Fuchs]

Hi,

A direct consequence of this change is that you may have
to either blow up your ./build directory, or run
'make clean-java' after pulling this fix.

Otherwise you may see strange build failures like below:

Error occurred during initialization of VM
java.lang.RuntimeException: Package com.sun.jmx.remote.protocol.rmi in 
both module java.management.rmi and module java.management
	at 
jdk.internal.module.ModuleBootstrap.fail(java.base/ModuleBootstrap.java:699)
	at 
jdk.internal.module.ModuleBootstrap.boot(java.base/ModuleBootstrap.java:329)

at java.lang.System.initPhase2(java.base/System.java:1928)


best regards,

-- daniel
--- Begin Message ---
Changeset: db6e995edd0a
Author:dfuchs
Date:  2017-02-02 16:50 +
URL:   http://hg.openjdk.java.net/jdk9/dev/jdk/rev/db6e995edd0a

8173607: JMX RMI connector should be in its own module
Summary: The JMX RMI connector is moved to a new java.management.rmi module.
Reviewed-by: mchung, erikj

- make/rmic/Rmic-java.management.gmk
+ make/rmic/Rmic-java.management.rmi.gmk
! src/java.base/share/classes/module-info.java
+ 
src/java.management.rmi/share/classes/com/sun/jmx/remote/internal/rmi/ProxyRef.java
+ 
src/java.management.rmi/share/classes/com/sun/jmx/remote/internal/rmi/RMIExporter.java
+ 
src/java.management.rmi/share/classes/com/sun/jmx/remote/internal/rmi/Unmarshal.java
+ 
src/java.management.rmi/share/classes/com/sun/jmx/remote/protocol/rmi/ClientProvider.java
+ 
src/java.management.rmi/share/classes/com/sun/jmx/remote/protocol/rmi/ServerProvider.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/NoCallStackClassLoader.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnector.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectorServer.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIIIOPServerImpl.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIJRMPServerImpl.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIServer.java
+ 
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIServerImpl.java
+ src/java.management.rmi/share/classes/javax/management/remote/rmi/package.html
+ src/java.management.rmi/share/classes/module-info.java
! 
src/java.management/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java
- src/java.management/share/classes/com/sun/jmx/remote/internal/ProxyRef.java
- src/java.management/share/classes/com/sun/jmx/remote/internal/RMIExporter.java
- src/java.management/share/classes/com/sun/jmx/remote/internal/Unmarshal.java
- 
src/java.management/share/classes/com/sun/jmx/remote/protocol/rmi/ClientProvider.java
- 
src/java.management/share/classes/com/sun/jmx/remote/protocol/rmi/ServerProvider.java
! 
src/java.management/share/classes/javax/management/remote/JMXConnectorFactory.java
! 
src/java.management/share/classes/javax/management/remote/JMXConnectorServerFactory.java
- 
src/java.management/share/classes/javax/management/remote/rmi/NoCallStackClassLoader.java
- 
src/java.management/share/classes/javax/management/remote/rmi/RMIConnection.java
- 
src/java.management/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
- 
src/java.management/share/classes/javax/management/remote/rmi/RMIConnector.java
- 
src/java.management/share/classes/javax/management/remote/rmi/RMIConnectorServer.java
- 
src/java.management/share/classes/javax/management/remote/rmi/RMIIIOPServerImpl.java
- 
src/java.management/share/classes/javax/management/remote/rmi/RMIJRMPServerImpl.java
- src/java.management/share/classes/javax/management/remote/rmi/RMIServer.java
- 
src/java.management/share/classes/javax/management/remote/rmi/RMIServerImpl.java
- src/java.management/share/classes/javax/management/remote/rmi/package.html
! src/java.management/share/classes/module-info.java
! src/java.rmi/share/classes/module-info.java
! src/java.se/share/classes/module-info.java
! src/jdk.jconsole/share/classes/module-info.java
! src/jdk.management.agent/share/classes/module-info.java
! 
src/jdk.management.agent/share/classes/sun/management/jmxremote/ConnectorBootstrap.java
! test/javax/management/MBeanInfo/NotificationInfoTest.java
! test/javax/management/MBeanServer/ExceptionTest.java
! test/javax/management/MBeanServer/OldMBeanServerTest.java
! test/javax/management/modelmbean/UnserializableTargetObjectTest.java
! test/javax/management/mxbean/GenericArrayTypeTest.java
! test/javax/management/mxbean/MXBeanExceptionHandlingTest.java
! test/javax/management/mxbean/MXBeanInteropTest1.java
! test/javax/management/mxbean/MXBeanInteropTest2.java
! test/javax/management/mxbean/MXBeanNotifTest.java
! test/javax/management/mxbean/MXBeanTest.java
! 

Re: RFR: 8173607: JMX RMI connector should be in its own module

[Daniel Fuchs]

Hi Mandy,

On 02/02/17 02:41, Mandy Chung wrote:

http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.06


Does java.management still depend on java.base/jdk.internal.module?


Well spotted! It doesn't. I have updated module-info.java for java.base.

http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.07


http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.06/java.management.rmi-summary.html



Maybe the first sentence of @provides could be simplified to:

A provider of JMXConnectorProvider for the RMI protocol.
A provider of JMXConnectorServerProvider for the RMI protocol.


Done. But I think you meant A provider of JMXConnector[Server] for the 
RMI protocol.


http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.07/java.management.rmi-summary.html

best regards,

-- daniel



Otherwise looks good.
Mandy

RFR: 8173607: JMX RMI connector should be in its own module

[Daniel Fuchs]

Hi,

Please find below a patch for:

8173607: JMX RMI connector should be in its own module
https://bugs.openjdk.java.net/browse/JDK-8173607

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.05

This patch makes it possible to remove the requires transitive
java.rmi from the java.management module.

For convenience here is the HTML module-level API documentation
produced by javadoc for the new java.management.rmi module:

http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.05/java.management.rmi-summary.html

The patch has some temporary hacks (mainly ConnectorBootstrap.java)
that we will be able to revert when
JDK-8173608 Separate JDK management agent from java.management module
is pushed.

The changes were mainly mechanical except for:

ConnectorBootstrap.java: see above
JMXConnectorFactory.java/JMXConnectorServerFactory.java:
   ServiceLoader is now used to load the default RMI Connector provider
   as a service. There should however be no observable behavior change
   to existing application.
ProxyRef.java, Unmarshal.java: are moved to a new
   com.sun.jmx.remote.internal.rmi package in the new module.
ClientNotifForwarder.java: is modified to no longer depend on
   java.rmi.UnmarshalException - NotSerializableException is used
   instead
RMIConnector.java: fetchNotif is updated to throw
   NotSerializableException instead of UnmarshalException
   The PRef serialization bytes are updated to accomodate the
   new ProxyRef package name.

Some further cleanup of java.base and java.rmi module-info.java
should become possible once JDK-8173608 is in (as well as moving
RMIExporter to java.management.rmi - which is not possible yet).

Further cleanup of @modules in tests might become possible as
well.

Note:
JCK tests for api/javax_management and api/java_lang/management
are passing with this change.

best regards,

-- daniel

Re: Review Request: JDK-8173608: Separate JDK management agent from java.management module

[Daniel Fuchs]

Hi Mandy,

Looks good to me in general.

Suggestion: FileLoginModule.java

 112 private static final String PASSWORD_FILE_NAME = 
"jmxremote.password";


Maybe the constant could be public, then it could be
referred to by ConnectorBootstrap?
I see that com.sun.jmx.remote.internal is already
being exported to jdk.management.agent.

FileSystem.java:

I wonder if this functionality should have remained in
java.management. It doesn't have any type to any specific
protocol - and it seems to be something that anyone using
the FileLoginModule above might also need.

ConnectorBootstrap.java:

I would normally discourage creating helper methods
(such as config()) in a class that doesn't implement
System.Logger, as that will make the 'config' method
appear as the source of the log message (rather than
the method that calls config()).
In this specific case I don't think it matters much
though. It's probably better to keep it like this so
as not distract reviewers from the meaningful changes,
and we can clean that up any time later in a separate
changeset (and possibly not before 10).


best regards,

-- daniel

On 30/01/17 23:48, Mandy Chung wrote:

Webrev at:
 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173608/webrev.00/index.html

Mandy


On Jan 30, 2017, at 3:42 PM, Mandy Chung  wrote:

java.management is the module for JMX and JMX remote API and
java.lang.management. JDK management agent provides the JDK-specific
out-of-the-box monitoring and management support and it’s cleaner
for it to live in its own module.  It is proposed to move it
to a new module named `jdk.management.agent`.

This change involves:
1) renaming of sun.management.Agent along with 3 other classes
in sun.management package to jdk.internal.agent package

2) move sun.management.resources to jdk.internal.agent.resources

3) move sun.management.jmxremote and sun.management.jdp packages
to jdk.management.agent module

4) move the configuration files under conf/management

5) sun/management/jmxremote/ConnectorBootstrap.java is updated
to replace the use of the ClassLogger API with System.Logger.

6) change hotspot VM to add `jdk.management.agent` when
-Dcom.sun.management.* system property is set as well as
the Agent class rename

Daniel is working on JDK-8173607 [1] that conflicts with this
patch and require merges/coordination.

We propose to integrate these changes to jdk9/dev unless
there is any objection concerning that this trivial hotspot change
might cause any issue.  I have submitted a RBT on hotspot_runtime
and hotspot_serviceability in addition to PIT test runs.

thanks
Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8173607

Re: build failing on Mac

[Daniel Fuchs]

Hi Pete,

On 20/06/16 13:24, Pete Brunet wrote:

Thanks Daniel, I will give it a try.  I've not been having luck recently
finding old versions of xcode.  Any thoughts on the best place to find it?


Apparently this is the place where you can get it:
https://developer.apple.com/downloads/index.action

cheers,

-- daniel



Pete

On 6/20/16 4:30 AM, Daniel Fuchs wrote:

Hi Pete,

I had the same problem recently - and solved it by installing
Xcode 6.3 - which I was told is the officially supported version.

best regards,

-- daniel

On 18/06/16 01:20, Pete Brunet wrote:

I haven't done a full build in around a month, but just pulled (tpull
-u) / cleaned / reconfigured and got this on my Mac:

...
Creating gtestLauncher from 1 file(s)
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
make[4]: ***
[/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release/hotspot/variant-server/libjvm/gtest/objs/gtest-all.o]

Error 1
make[4]: *** Waiting for unfinished jobs
make[4]: ***
[/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release/hotspot/variant-server/libjvm/gtest/objs/gtestMain.o]

Error 1
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
make[4]: ***
[/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release/hotspot/variant-server/libjvm/gtest/objs/test_os.o]

Error 1
Compiling 390 files for BUILD_INTERIM_jdk.compiler
make[3]: *** [variant-server-libs] Error 2
make[2]: *** [hotspot] Error 2
...
=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_gtest_objs_gtest-all.o:
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
* For target hotspot_variant-server_libjvm_gtest_objs_gtestMain.o:
* For target hotspot_variant-server_libjvm_gtest_objs_test_os.o:
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
=== End of repeated output ===
...

Java -version: 1.8.0-b132
XCode is 6.2

configure output ends with this:

The existing configuration has been successfully updated in
/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release

using configure arguments '--with-sdk-name=macosx10.9'.

Configuration summary:
* Debug level:release
* HS debug level: product
* JDK variant:normal
* JVM variants:   server
* OpenJDK target: OS: macosx, CPU architecture: x86, address length: 64
* Version string: 9-internal+0-2016-06-17-185920.petebrunet.client
(9-internal)

Tools summary:
* Boot JDK:   java version "1.8.0" Java(TM) SE Runtime Environment
(build 1.8.0-b132) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b70,
mixed mode)  (at
/Library/Java/JavaVirtualMachines/jdk1.8.0.jdk/Contents/Home)
* Toolchain:  clang (clang/LLVM from Xcode 6.2)
* C Compiler: Version 6.0 (at /usr/bin/clang)
* C++ Compiler:   Version 6.0 (at /usr/bin/clang++)

Any ideas on how I should proceed or other information that is needed?

TiA, Pete

Re: build failing on Mac

[Daniel Fuchs]

Hi Pete,

I had the same problem recently - and solved it by installing
Xcode 6.3 - which I was told is the officially supported version.

best regards,

-- daniel

On 18/06/16 01:20, Pete Brunet wrote:

I haven't done a full build in around a month, but just pulled (tpull
-u) / cleaned / reconfigured and got this on my Mac:

...
Creating gtestLauncher from 1 file(s)
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
make[4]: ***
[/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release/hotspot/variant-server/libjvm/gtest/objs/gtest-all.o]
Error 1
make[4]: *** Waiting for unfinished jobs
make[4]: ***
[/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release/hotspot/variant-server/libjvm/gtest/objs/gtestMain.o]
Error 1
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
make[4]: ***
[/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release/hotspot/variant-server/libjvm/gtest/objs/test_os.o]
Error 1
Compiling 390 files for BUILD_INTERIM_jdk.compiler
make[3]: *** [variant-server-libs] Error 2
make[2]: *** [hotspot] Error 2
...
=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_gtest_objs_gtest-all.o:
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
* For target hotspot_variant-server_libjvm_gtest_objs_gtestMain.o:
* For target hotspot_variant-server_libjvm_gtest_objs_test_os.o:
error: unknown warning option '-Wno-tautological-undefined-compare'; did
you mean '-Wno-tautological-compare'? [-Werror,-Wunknown-warning-option]
=== End of repeated output ===
...

Java -version: 1.8.0-b132
XCode is 6.2

configure output ends with this:

The existing configuration has been successfully updated in
/Users/petebrunet/JDK9/JDK-8145207/client/build/macosx-x86_64-normal-server-release
using configure arguments '--with-sdk-name=macosx10.9'.

Configuration summary:
* Debug level:release
* HS debug level: product
* JDK variant:normal
* JVM variants:   server
* OpenJDK target: OS: macosx, CPU architecture: x86, address length: 64
* Version string: 9-internal+0-2016-06-17-185920.petebrunet.client
(9-internal)

Tools summary:
* Boot JDK:   java version "1.8.0" Java(TM) SE Runtime Environment
(build 1.8.0-b132) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b70,
mixed mode)  (at
/Library/Java/JavaVirtualMachines/jdk1.8.0.jdk/Contents/Home)
* Toolchain:  clang (clang/LLVM from Xcode 6.2)
* C Compiler: Version 6.0 (at /usr/bin/clang)
* C++ Compiler:   Version 6.0 (at /usr/bin/clang++)

Any ideas on how I should proceed or other information that is needed?

TiA, Pete

Re: RFR: JDK-8073328: Incremental build of gensrc broken

[Daniel Fuchs]

On 17/02/15 16:38, Erik Joelsson wrote:

Right, thanks for catching that. The OS specific file is not there for
every OS. This patch should fix it. I'm testing all platforms now.

Webrev: http://cr.openjdk.java.net/~erikj/8073328/webrev.jdk.03/


Thanks Erik.

This one appears to work like a charm on my machine :-)

-- daniel

Re: RFR: JDK-8073328: Incremental build of gensrc broken

[Daniel Fuchs]

Hi Erik,

I tried to import your patch, and now the build fails with the
following error:

make[3]: *** No rule to make target 
`/Users/danielfuchs/workspaces/jdk/jdk9-dev-java-time/jdk/make/data/charsetmapping/stdcs-macosx', 
needed by 
`/Users/danielfuchs/workspaces/jdk/jdk9-dev-java-time/build/macosx-x86_64-normal-server-release/support/gensrc/java.base/sun/nio/cs/_the.charsetmapping-stdcs'. 
 Stop.


So it looks that something is still wrong :-(

best regards,

-- daniel

On 17/02/15 15:56, Erik Joelsson wrote:

Hello,

Please review this fix for the incremental build behavior of the gensrc
build step. After JDK-8073152 the charsets would unconditionally get
regenerated each time make was invoked. The actual culprit was a stray
backslash which prevented the 'touch' on the next line from executing.

I took the liberty of cleaning up some more, adding to the prerequisites
lists of the rules in question and fixing whitespace according to our
guideline [1]. The whitespace changes aren't visible in the webrev.

I also quieted down the gensrc tool for charsets by adding the LOG_INFO
macro. This will hide the output for the default warn level. To see it,
run make with LOG=info. For this to work I had to modify the tool to
print its unconditional output to stdout instead of stderr. We want to
keep stderr open to the log for any actual failure messages.

Bug: https://bugs.openjdk.java.net/browse/JDK-8073328
Webrev: http://cr.openjdk.java.net/~erikj/8073328/webrev.jdk.01/

/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

Re: RFR: 8065138 - Encodings.isRecognizedEnconding sometimes fails to recognize 'UTF8'

[Daniel Fuchs]

On 24/11/14 11:44, Erik Joelsson wrote:

Hello Daniel,

Test seems like a great idea. Thanks!


OK - I have logged JDK-8065748
https://bugs.openjdk.java.net/browse/JDK-8065748

I'll send a patch for review when your fix is in :-)

Thanks!
-- daniel



/Erik

On 2014-11-20 18:25, Daniel Fuchs wrote:

On 20/11/14 14:36, Erik Joelsson wrote:

Here is my proposal for fixing the particular issue of generating the
correct properties files. I'm simply adding LC_ALL=C to the whole
command line instead of just the sort at the end. It seems to require
using export to get picked up.


Hi Erik,

Looks good to me.

I have applied your patch (manually, because the copy/paste
seems to have eaten the tab characters away, which caused patch to
reject the diff) - and I confirm that the issue has disappeared.

Thanks for solving this!

Do you think it would be worth to commit this test modification
later on, in a followup Bug/RFE?

http://cr.openjdk.java.net/~dfuchs/webrev_8065138/webrev.00/jdk/test/javax/xml/jaxp/Encodings/CheckEncodingPropertiesFile.java.frames.html


If so I will take care of it.

best regards,

-- daniel



Bug: https://bugs.openjdk.java.net/browse/JDK-8065138
Patch:
diff --git a/make/common/JavaCompilation.gmk
b/make/common/JavaCompilation.gmk
--- a/make/common/JavaCompilation.gmk
+++ b/make/common/JavaCompilation.gmk
@@ -400,13 +400,15 @@
# Now we can setup the depency that will trigger the copying.
$$($1_BIN)$$($2_TARGET) : $2
  $(MKDIR) -p $$(@D)
-$(CAT) $$ | $(SED) -e 's/\([^\\]\):/\1\\:/g' -e
's/\([^\\]\)=/\1\\=/g' \
+export LC_ALL=C ; $(CAT) $$ \
+| $(SED) -e 's/\([^\\]\):/\1\\:/g' -e 's/\([^\\]\)=/\1\\=/g' \
  -e 's/\([^\\]\)!/\1\\!/g' -e 's/#.*/#/g' \
  | $(SED) -f $(SRC_ROOT)/make/common/support/unicode2x.sed \
  | $(SED) -e '/^#/d' -e '/^/d' \
  -e :a -e '/\\/N; s/\\\n//; ta' \
  -e 's/^[ ]*//;s/[ ]*//' \
--e 's/\\=/=/' | LC_ALL=C $(SORT)  $$@
+-e 's/\\=/=/' \
+| $(SORT)  $$@
  $(CHMOD) -f ug+w $$@

# And do not forget this target


I filed a separate issue [1] for investigating the use of pipefail.

/Erik

[1] https://bugs.openjdk.java.net/browse/JDK-8065576

On 2014-11-20 10:34, Daniel Fuchs wrote:

On 11/20/14 10:26 AM, Erik Joelsson wrote:

Hello,

On 2014-11-20 02:20, Martin Buchholz wrote:

Amusingly, the $(SORT) has an LC_ALL=C carefully placed before it,
but
the $(SED)s need it too!

Yes, I think that's the correct fix in this case.

On Wed, Nov 19, 2014 at 5:18 PM, Martin Buchholz
marti...@google.com wrote:

[+ build-dev]

I think I see the problem.  By default, a Unix pipeline sadly fails
only when the last command fails.  In bash, you can change this to a
more sensible default via
set -o pipefail
but that's not portable enough for openjdk.

This is interesting and something I had missed. I will experiment
with enabling pipefail if configure finds support for it. This will
include setting the SHELL to bash. We really should fail instead of
silently generating broken builds.

Daniel, I can take over this bug if you want and work on a proper
build fix.


Thanks Erik! You are welcome!
So the whole issue seems to be that my default setting is
LC_ALL=en_US.UTF-8
whereas sed requires LC_ALL=C to work properly on these property
files...

When the test first failed I tried to rerun the test with LC_ALL=C -
with no success
of course. But I never thought of rebuilding with LC_ALL=C :-(

My apologies for the red herring :-(

best regards

-- daniel



/Erik

$(CAT) $$ | $(SED) -e 's/\([^\\]\):/\1\\:/g' -e
's/\([^\\]\)=/\1\\=/g' \
-e 's/\([^\\]\)!/\1\\!/g' -e 's/#.*/#/g' \
| $(SED) -f $(SRC_ROOT)/make/common/support/unicode2x.sed \
| $(SED) -e '/^#/d' -e '/^/d' \
-e :a -e '/\\/N; s/\\\n//; ta' \
-e 's/^[ ]*//;s/[ ]*//' \
-e 's/\\=/=/' | LC_ALL=C $(SORT)  $$@

On Wed, Nov 19, 2014 at 5:07 PM, huizhe wang
huizhe.w...@oracle.com wrote:

On 11/19/2014 4:09 PM, Mandy Chung wrote:


On 11/19/2014 3:49 PM, Mandy Chung wrote:


On 11/19/2014 12:50 PM, Daniel Fuchs wrote:

On 11/19/14 9:36 PM, Mandy Chung wrote:

resources.jar will be gone when we move to the modular runtime
image
(JEP 220 [1]).
JDK-8065138 and JDK-8065365 will become non-issue in JDK 9.

Do you mean that the property files will no longer be stripped
of their
comments?


(sorry for my delay in reply as I was trying to get the number
of the
resources in the modular image vs resources.jar but got
distracted.)

The current version copies all bytes when generating the modular
image.
This is a good question whether we should strip off the comments
when
writing to the modular runtime image.   I think we should look
at the
footprint and any performance saving and determine if we should
do the same
in JDK 9.


I looked at the exploded image under
BUILD_OUTPUTDIR/jdk/modules/java.xml
and found that the comments of Encodings.properties are stripped.
I

Re: RFR [JEP 220] Modular Run-Time Images

[Daniel Fuchs]

Hi Chris,

I had a look at the changes in java.logging and java.management modules
(+ corresponding tests) and they look reasonable to me.

I also had a cursory look at the jdeps changes. It's difficult for me
to review that without importing the changes and playing with the
tool as this is a part I'm less familiar with - but I didn't see
anything that would be obviously wrong.

I skipped most of the rest (especially the build changes ;-)).

best regards,

-- daniel

On 20/11/14 22:41, Chris Hegarty wrote:



From: Chris Hegarty chris.hega...@oracle.com
Subject: RFR [JEP 220] Modular Run-Time Images
Date: 20 November 2014 21:39:14 GMT
To: jigsaw-dev jigsaw-...@openjdk.java.net, jdk9-dev jdk9-...@openjdk.java.net, build-dev build-dev@openjdk.java.net, Alan Bateman alan.bate...@oracle.com, 
Alex Buckley alex.buck...@oracle.com, Chris Hegarty chris.hega...@oracle.com, Erik Joelsson erik.joels...@oracle.com, Jonathan Gibbons 
jonathan.gibb...@oracle.com, Karen Kinnear karen.kinn...@oracle.com, Jim Laskey (Oracle) james.las...@oracle.com, Magnus Ihse Bursie 
magnus.ihse.bur...@oracle.com, Mandy Chung mandy.ch...@oracle.com, Mark Reinhold mark.reinh...@oracle.com, Paul Sandoz paul.san...@oracle.com, A. 
Sundararajan sundararajan.athijegannat...@oracle.com

This is a review request for the changes for JEP 220: Modular Run-Time Images 
[1].

There are a number of individuals responsible for these changes. Some, possibly 
not all, are explicitly listed in the 'To' section of this mail, and they will 
help address any comments arising from this review request.

The new run-time image structure is defined in JEP 220 [1], and can be seen as 
a result of building the staging forest [2], or in the early access builds [3].

Webrevs:
   http://cr.openjdk.java.net/~chegar/8061971/00/

Due to the significant impact of these changes, a JDK 9 promotion has been 
tentatively reserved for their integration. All comments are welcome, although 
given the nature of the changes then we might have to create separate issues in 
JIRA to address some of them later in jdk9/dev.

-Chris.

[1] http://openjdk.java.net/jeps/220
[2] http://hg.openjdk.java.net/jigsaw/m2/
[3] http://openjdk.java.net/projects/jigsaw/ea

Re: RFR: 8065138 - Encodings.isRecognizedEnconding sometimes fails to recognize 'UTF8'

[Daniel Fuchs]

On 11/20/14 10:26 AM, Erik Joelsson wrote:

Hello,

On 2014-11-20 02:20, Martin Buchholz wrote:

Amusingly, the $(SORT) has an LC_ALL=C carefully placed before it, but
the $(SED)s need it too!

Yes, I think that's the correct fix in this case.
On Wed, Nov 19, 2014 at 5:18 PM, Martin Buchholz 
marti...@google.com wrote:

[+ build-dev]

I think I see the problem.  By default, a Unix pipeline sadly fails
only when the last command fails.  In bash, you can change this to a
more sensible default via
set -o pipefail
but that's not portable enough for openjdk.
This is interesting and something I had missed. I will experiment with 
enabling pipefail if configure finds support for it. This will include 
setting the SHELL to bash. We really should fail instead of silently 
generating broken builds.


Daniel, I can take over this bug if you want and work on a proper 
build fix.


Thanks Erik! You are welcome!
So the whole issue seems to be that my default setting is LC_ALL=en_US.UTF-8
whereas sed requires LC_ALL=C to work properly on these property files...

When the test first failed I tried to rerun the test with LC_ALL=C - 
with no success

of course. But I never thought of rebuilding with LC_ALL=C :-(

My apologies for the red herring :-(

best regards

-- daniel



/Erik
$(CAT) $$ | $(SED) -e 's/\([^\\]\):/\1\\:/g' -e 
's/\([^\\]\)=/\1\\=/g' \

-e 's/\([^\\]\)!/\1\\!/g' -e 's/#.*/#/g' \
| $(SED) -f $(SRC_ROOT)/make/common/support/unicode2x.sed \
| $(SED) -e '/^#/d' -e '/^/d' \
-e :a -e '/\\/N; s/\\\n//; ta' \
-e 's/^[ ]*//;s/[ ]*//' \
-e 's/\\=/=/' | LC_ALL=C $(SORT)  $$@

On Wed, Nov 19, 2014 at 5:07 PM, huizhe wang 
huizhe.w...@oracle.com wrote:

On 11/19/2014 4:09 PM, Mandy Chung wrote:


On 11/19/2014 3:49 PM, Mandy Chung wrote:


On 11/19/2014 12:50 PM, Daniel Fuchs wrote:

On 11/19/14 9:36 PM, Mandy Chung wrote:
resources.jar will be gone when we move to the modular runtime 
image

(JEP 220 [1]).
JDK-8065138 and JDK-8065365 will become non-issue in JDK 9.
Do you mean that the property files will no longer be stripped 
of their

comments?


(sorry for my delay in reply as I was trying to get the number of 
the

resources in the modular image vs resources.jar but got distracted.)

The current version copies all bytes when generating the modular 
image.
This is a good question whether we should strip off the comments 
when
writing to the modular runtime image.   I think we should look at 
the
footprint and any performance saving and determine if we should 
do the same

in JDK 9.

I looked at the exploded image under 
BUILD_OUTPUTDIR/jdk/modules/java.xml
and found that the comments of Encodings.properties are stripped. 
I was
confused with the mention of resources.jar that I assume it was a 
step

stripping the comments before writing to resources.jar. This is still
an issue in jigsaw m2 I believe.

Where does the build strip the comments?


A previous issue was that the build process strips off anything 
after '#'

when copying properties files. In JDK8:
jaxp/make/BuildJaxp.gmk:
$(JAXP_OUTPUTDIR)/classes/%.properties: 
$(JAXP_TOPDIR)/src/%.properties

 $(MKDIR) -p $(@D)
 $(RM) $@ $@.tmp
 $(CAT) $ | LANG=C $(NAWK) '{ sub(/#.*$$/,#); print }'  
$@.tmp

 $(MV) $@.tmp $@

This was fixed in JDK 9.  The per-repo process was removed. It 
looks like

the properties processing process is now consolidated into
make/common/JavaCompilation.gmk. So the issue Daniel found is new 
(in terms

of stripping). Search 'properties files' to see the macro.

Joe


Mandy

Re: RFR: 8065138 - Encodings.isRecognizedEnconding sometimes fails to recognize 'UTF8'

[Daniel Fuchs]

On 20/11/14 14:36, Erik Joelsson wrote:

Here is my proposal for fixing the particular issue of generating the
correct properties files. I'm simply adding LC_ALL=C to the whole
command line instead of just the sort at the end. It seems to require
using export to get picked up.


Hi Erik,

Looks good to me.

I have applied your patch (manually, because the copy/paste
seems to have eaten the tab characters away, which caused patch to
reject the diff) - and I confirm that the issue has disappeared.

Thanks for solving this!

Do you think it would be worth to commit this test modification
later on, in a followup Bug/RFE?

http://cr.openjdk.java.net/~dfuchs/webrev_8065138/webrev.00/jdk/test/javax/xml/jaxp/Encodings/CheckEncodingPropertiesFile.java.frames.html

If so I will take care of it.

best regards,

-- daniel



Bug: https://bugs.openjdk.java.net/browse/JDK-8065138
Patch:
diff --git a/make/common/JavaCompilation.gmk
b/make/common/JavaCompilation.gmk
--- a/make/common/JavaCompilation.gmk
+++ b/make/common/JavaCompilation.gmk
@@ -400,13 +400,15 @@
# Now we can setup the depency that will trigger the copying.
$$($1_BIN)$$($2_TARGET) : $2
  $(MKDIR) -p $$(@D)
-$(CAT) $$ | $(SED) -e 's/\([^\\]\):/\1\\:/g' -e
's/\([^\\]\)=/\1\\=/g' \
+export LC_ALL=C ; $(CAT) $$ \
+| $(SED) -e 's/\([^\\]\):/\1\\:/g' -e 's/\([^\\]\)=/\1\\=/g' \
  -e 's/\([^\\]\)!/\1\\!/g' -e 's/#.*/#/g' \
  | $(SED) -f $(SRC_ROOT)/make/common/support/unicode2x.sed \
  | $(SED) -e '/^#/d' -e '/^/d' \
  -e :a -e '/\\/N; s/\\\n//; ta' \
  -e 's/^[ ]*//;s/[ ]*//' \
--e 's/\\=/=/' | LC_ALL=C $(SORT)  $$@
+-e 's/\\=/=/' \
+| $(SORT)  $$@
  $(CHMOD) -f ug+w $$@

# And do not forget this target


I filed a separate issue [1] for investigating the use of pipefail.

/Erik

[1] https://bugs.openjdk.java.net/browse/JDK-8065576

On 2014-11-20 10:34, Daniel Fuchs wrote:

On 11/20/14 10:26 AM, Erik Joelsson wrote:

Hello,

On 2014-11-20 02:20, Martin Buchholz wrote:

Amusingly, the $(SORT) has an LC_ALL=C carefully placed before it, but
the $(SED)s need it too!

Yes, I think that's the correct fix in this case.

On Wed, Nov 19, 2014 at 5:18 PM, Martin Buchholz
marti...@google.com wrote:

[+ build-dev]

I think I see the problem.  By default, a Unix pipeline sadly fails
only when the last command fails.  In bash, you can change this to a
more sensible default via
set -o pipefail
but that's not portable enough for openjdk.

This is interesting and something I had missed. I will experiment
with enabling pipefail if configure finds support for it. This will
include setting the SHELL to bash. We really should fail instead of
silently generating broken builds.

Daniel, I can take over this bug if you want and work on a proper
build fix.


Thanks Erik! You are welcome!
So the whole issue seems to be that my default setting is
LC_ALL=en_US.UTF-8
whereas sed requires LC_ALL=C to work properly on these property files...

When the test first failed I tried to rerun the test with LC_ALL=C -
with no success
of course. But I never thought of rebuilding with LC_ALL=C :-(

My apologies for the red herring :-(

best regards

-- daniel



/Erik

$(CAT) $$ | $(SED) -e 's/\([^\\]\):/\1\\:/g' -e
's/\([^\\]\)=/\1\\=/g' \
-e 's/\([^\\]\)!/\1\\!/g' -e 's/#.*/#/g' \
| $(SED) -f $(SRC_ROOT)/make/common/support/unicode2x.sed \
| $(SED) -e '/^#/d' -e '/^/d' \
-e :a -e '/\\/N; s/\\\n//; ta' \
-e 's/^[ ]*//;s/[ ]*//' \
-e 's/\\=/=/' | LC_ALL=C $(SORT)  $$@

On Wed, Nov 19, 2014 at 5:07 PM, huizhe wang
huizhe.w...@oracle.com wrote:

On 11/19/2014 4:09 PM, Mandy Chung wrote:


On 11/19/2014 3:49 PM, Mandy Chung wrote:


On 11/19/2014 12:50 PM, Daniel Fuchs wrote:

On 11/19/14 9:36 PM, Mandy Chung wrote:

resources.jar will be gone when we move to the modular runtime
image
(JEP 220 [1]).
JDK-8065138 and JDK-8065365 will become non-issue in JDK 9.

Do you mean that the property files will no longer be stripped
of their
comments?


(sorry for my delay in reply as I was trying to get the number
of the
resources in the modular image vs resources.jar but got
distracted.)

The current version copies all bytes when generating the modular
image.
This is a good question whether we should strip off the comments
when
writing to the modular runtime image.   I think we should look
at the
footprint and any performance saving and determine if we should
do the same
in JDK 9.


I looked at the exploded image under
BUILD_OUTPUTDIR/jdk/modules/java.xml
and found that the comments of Encodings.properties are stripped.
I was
confused with the mention of resources.jar that I assume it was a
step
stripping the comments before writing to resources.jar. This is
still
an issue in jigsaw m2 I believe.

Where does the build strip the comments?


A previous issue was that the build process strips off anything
after '#'
when

Re: jmx-dev RFR 8060692 Delete com/sun/jmx/snmp and sun/management/snmp from OpenJDK

[Daniel Fuchs]

On 15/10/14 17:19, shanliang wrote:

Here is the new version:
http://cr.openjdk.java.net/~sjiang/JDK-8060692/01/


The new version looks fine :-)

-- daniel



I add:
./01/jdk9-make/
for updating ./make/CompileJavaModules.gmk

./01/jdk9-jdk-src/ is same to ./00

Thanks,
Shanliang


Erik Joelsson wrote:

Hello,

Removing the excludes would be appreciated. Here is a patch:

diff -r c173ba994245 make/CompileJavaModules.gmk
--- a/make/CompileJavaModules.gmk
+++ b/make/CompileJavaModules.gmk
@@ -247,14 +247,6 @@
 # Exclude building of IIOP transport for RMI Connector
 java.management_EXCLUDES := com/sun/jmx/remote/protocol/iiop

-# Why is this in the open source tree?
-ifdef OPENJDK
-  java.management_EXCLUDES := \
-  com/sun/jmx/snmp \
-  sun/management/snmp \
-  #
-endif
-
 ifeq ($(RMICONNECTOR_IIOP), false)
   java.management_EXCLUDES += com/sun/jmx/remote/protocol/iiop
 endif

/Erik

On 2014-10-15 15:46, Alan Bateman wrote:

On 15/10/2014 14:35, shanliang wrote:

Hi,

SNMP is not part of OpenJDK and SNMP packages are not compiled in
OpenJDK, so the SNMP sources should be deleted from the OpenJDK

Bug: https://bugs.openjdk.java.net/browse/JDK-8060692
Webrev: http://cr.openjdk.java.net/~sjiang/JDK-8060692/00/

The code removal looks good but don't you also need to update make
make/CompileJavaModules.gmk in the top-level remove to remove the
snmp directories from java.management_EXCLUDES ?

-Alan.

Re: RFR JDK-8044627: Update JNDI to work with modules

[Daniel Fuchs]

On 14/10/14 16:09, Chris Hegarty wrote:

On 14 Oct 2014, at 15:03, Pavel Rappo pavel.ra...@oracle.com wrote:


OK, so what I will do for now is I exclude these 4 files and push without them. 
I'll create a new issue to add them later.


That sounds like a fine plan. This issue has already gone on for long enough, 
and I don’t think that the crooks of the change should have to wait even longer.


Right. I see it now. If you have no providers - then the old
'helper.loadClass(className).newInstance();'
code will be executed - which should still work for now.

So not pushing the META-INF/services files sounds fine.

best regards,

-- daniel



Consider this issue Reviewed, provided that the changes in the webrev minus the 
4 services files compile and test ok. Then you can push the services files once 
build support is added.

-Chris.


-Pavel

On 14 Oct 2014, at 14:44, Alan Bateman alan.bate...@oracle.com wrote:


On 14/10/2014 14:34, Daniel Fuchs wrote:

Hi Pavel,

I saw your mail on build-dev.
I guess the issue will resolve itself once we have the
modular image.

I wonder whether the way to go for now would be
to add a single META-INF/services file - as you suggest -
in java.naming, with the 4 lines inside, and log a bug/RFE
to follow up on that once the modular image is there.

best regards,

-- daniel

Once we move to modules then these service configuration files will go away. 
The resolver will build a service-use graph based on the provides/uses in the 
module descriptors and that will link the JNDI module to the providers. Pavel 
just needs a short term solution and having all the providers in one file is 
okay for that. A better short term solution is to just concatenate them in the 
build, we are already doing this in the jigsaw/m2 forest for the JDI connectors.

-Alan

Re: Review request: 8055230: Rename attach provider implementation class

[Daniel Fuchs]

Hi Mandy,

I'm seeing some small differences in the various VirtualMachineImpl.java
files, but if I'm not mistaken, all the new AttachProviderImpl.java look
all the same. I wonder if the patch could be further simplified by
moving AttachProviderImpl.java to jdk.attach/share/classes (unless
there's a reason to keep all the different copies)...

best regards,

-- daniel

On 8/26/14 6:29 AM, Mandy Chung wrote:

Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
unix/classes/sun/tools/attach/${OS}VirtualMachine.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
unix/classes/sun/tools/attach/${OS}AttachProvider.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy

Re: Review request: 8055230: Rename attach provider implementation class

[Daniel Fuchs]

On 8/26/14 12:59 PM, Staffan Larsen wrote:

There are some differences in the AttachProvideImpl files. Most notably the 
windows version is very different. Linux, Bsd and Aix are the same. The Solaris 
version has a minor difference in the  “public String type()” implementation. 
The Linux, Bsd, Aix and Solaris versions could probably be unified.


Ah - right - I missed those. Forget my comment then!

Thanks Staffan!

-- daniel



/Staffan

On 26 aug 2014, at 12:40, Daniel Fuchs daniel.fu...@oracle.com wrote:


Hi Mandy,

I'm seeing some small differences in the various VirtualMachineImpl.java
files, but if I'm not mistaken, all the new AttachProviderImpl.java look
all the same. I wonder if the patch could be further simplified by
moving AttachProviderImpl.java to jdk.attach/share/classes (unless
there's a reason to keep all the different copies)...

best regards,

-- daniel

On 8/26/14 6:29 AM, Mandy Chung wrote:

Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
unix/classes/sun/tools/attach/${OS}VirtualMachine.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
unix/classes/sun/tools/attach/${OS}AttachProvider.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
  - ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy

Re: RFR: 8048184 : (s) handle mercurial dev build version string

[Daniel Fuchs]

Hi Mike,

sorry for not reacting sooner.
I applied your patch on my machine, and now get the following:

sh ./get_source.sh
./get_source.sh: line 70: [: too many arguments
./get_source.sh: line 76: [: `)' expected, found -1
./get_source.sh: line 82: [: `)' expected, found -1


I have added:

echo hgversion=$hgversion
echo hgmajor=$hgmajor
echo hgminor=$hgminor
echo hgrev=$hgrev

just before line 70 and I see this:

$ sh ./get_source.sh
hgversion=3.0.1
hgmajor=3
hgminor=0
-1
hgrev=
./get_source.sh: line 76: [: too many arguments
./get_source.sh: line 82: [: `)' expected, found -1
./get_source.sh: line 88: [: `)' expected, found -1


There is a strange '-1' which gets printed - and it
seems it comes from $hgminor, whose value is:

hgminor='0
-1'


best regards,

-- daniel


On 6/26/14 9:01 PM, Mike Duigou wrote:

Hello all;

After pushing JDK-8047925 it was discovered that unofficial development builds 
of Mercurial use a different version string format and get_source.sh needs an 
enhancement to correctly parse this version string.

I have also incorporated some cleanup suggested by Dave Pointon, made the 
script more defensive in it's handling of numeric values and replaced the use 
of 'which' with the more reliable 'command -v'

jbsbug: https://bugs.openjdk.java.net/browse/JDK-8048184
webrev: http://cr.openjdk.java.net/~mduigou/JDK-8048184/0/webrev/

Mike

Re: RFR: 8048184 : (s) handle mercurial dev build version string

[Daniel Fuchs]

Hi Mike,

The issue seems to be that 'expr 0 + 0' writes 0 on stdout
but has an exit status of 1

$ expr 0 + 0
0
$ echo $?
1

$ man expr

EXPR(1)   BSD General Commands Manual   EXPR(1)

[ ... ]

DIAGNOSTICS
 The expr utility exits with one of the following values:
 0   the expression is neither an empty string nor 0.
 1   the expression is an empty string or 0.
 2   the expression is invalid.

[ ... ]

-- daniel


On 7/8/14 12:30 PM, Daniel Fuchs wrote:

Hi Mike,

sorry for not reacting sooner.
I applied your patch on my machine, and now get the following:

sh ./get_source.sh
./get_source.sh: line 70: [: too many arguments
./get_source.sh: line 76: [: `)' expected, found -1
./get_source.sh: line 82: [: `)' expected, found -1


I have added:

echo hgversion=$hgversion
echo hgmajor=$hgmajor
echo hgminor=$hgminor
echo hgrev=$hgrev

just before line 70 and I see this:

$ sh ./get_source.sh
hgversion=3.0.1
hgmajor=3
hgminor=0
-1
hgrev=
./get_source.sh: line 76: [: too many arguments
./get_source.sh: line 82: [: `)' expected, found -1
./get_source.sh: line 88: [: `)' expected, found -1


There is a strange '-1' which gets printed - and it
seems it comes from $hgminor, whose value is:

hgminor='0
-1'


best regards,

-- daniel


On 6/26/14 9:01 PM, Mike Duigou wrote:

Hello all;

After pushing JDK-8047925 it was discovered that unofficial
development builds of Mercurial use a different version string format
and get_source.sh needs an enhancement to correctly parse this version
string.

I have also incorporated some cleanup suggested by Dave Pointon, made
the script more defensive in it's handling of numeric values and
replaced the use of 'which' with the more reliable 'command -v'

jbsbug: https://bugs.openjdk.java.net/browse/JDK-8048184
webrev: http://cr.openjdk.java.net/~mduigou/JDK-8048184/0/webrev/

Mike

Re: RFR: 8048184 : (s) handle mercurial dev build version string

[Daniel Fuchs]

Hi,

Thanks Dave,

yes - your patch works like a charm :-)

best regards,

-- daniel

Note: the following also works on my machine:

--

check() {
   read var
   if expr 1 + $var /dev/null ; then
  echo $var
   else
  return 2
   fi
}

hgmajor=`echo $hgversion | cut -f 1 -d . | check 2 /dev/null || echo -1`
hgminor=`echo $hgversion | cut -f 2 -d . | check 2 /dev/null || echo -1`
hgrev=`echo $hgversion.0 | cut -f 3 -d . | check 2 /dev/null || echo 
-1` # rev is omitted for minor and major releases


--


On 7/8/14 3:27 PM, pointo1d wrote:

#!/bin/sh -e

#
# Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights
reserved.
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
#
# This code is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License version 2 only, as
# published by the Free Software Foundation. Oracle designates this
# particular file as subject to the Classpath exception as provided
# by Oracle in the LICENSE file that accompanied this code.
#
# This code is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
# version 2 for more details (a copy is included in the LICENSE file that
# accompanied this code).
#
# You should have received a copy of the GNU General Public License version
# 2 along with this work; if not, write to the Free Software Foundation,
# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
#
# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
# or visit www.oracle.com if you need additional information or have any
# questions.
#

to_stderr() {
echo $@ 2
}

error() {
to_stderr ERROR: $1
exit ${2:-126}
}

warning() {
to_stderr WARNING: $1
}

get_ver_substr() {
_n=`echo $hgversion | awk -F. '$'$1' ~ /[0-9]+/{ print $'$1' }'`
echo ${_n:-'-1'}
}

# Version check

# required
reqdmajor=1
reqdminor=5
reqdrev=0

# requested
rqstmajor=2
rqstminor=6
rqstrev=3

# installed
hgwhere=`command -v hg`
if [ x$hgwhere = x ]; then
error Could not locate Mercurial command
fi

hgversion=`hg --version 2 /dev/null | sed -n -e 's@^Mercurial
Distributed SCM (version \([^+]*\).*)\$@\1@p'`

if [ x${hgversion} = x ] ; then
error Could not determine Mercurial version of $hgwhere
fi

hgmajor=`get_ver_substr 1`
hgminor=`get_ver_substr 2`
hgrev=`get_ver_substr 3` # rev is omitted for minor and major releases

if [ `expr $hgmajor$hgminor$hgrev : .*-1` != 0 ]; then
error Could not determine Mercurial version of $hgwhere from
\$hgversion\
fi

# Require
if [ $hgmajor -lt $reqdmajor -o \( $hgmajor -eq $reqdmajor -a $hgminor
-lt $reqdminor \) -o \( $hgmajor -eq $reqdmajor -a $hgminor -eq
$reqdminor -a $hgrev -lt $reqdrev \) ] ; then
error Mercurial version $reqdmajor.$reqdminor.$reqdrev or later is
required. $hgwhere is version $hgversion

fi

# Request
if [ $hgmajor -lt $rqstmajor -o \( $hgmajor -eq $rqstmajor -a $hgminor
-lt $rqstminor \) -o \( $hgmajor -eq $rqstmajor -a $hgminor -eq
$rqstminor -a $hgrev -lt $rqstrev \) ] ; then
warning Mercurial version $rqstmajor.$rqstminor.$rqstrev or later is
recommended. $hgwhere is version $hgversion
fi

# Get clones of all absent nested repositories (harmless if already exist)
sh ./common/bin/hgforest.sh clone $@ || exit $?

# Update all existing repositories to the latest sources
sh ./common/bin/hgforest.sh pull -u

Re: RFR: 8048184 : (s) handle mercurial dev build version string

[Daniel Fuchs]

Hi Mike,

This one works perfectly on my machine :-)

Thumbs up!

-- daniel

On 7/8/14 7:40 PM, Mike Duigou wrote:

I've updated the webrev with yet a different variant of the extract version 
field function.

http://cr.openjdk.java.net/~mduigou/JDK-8048184/1/webrev/

Sorry Dave, I resist all attempts to introduce awk.:-)

Mike

Re: 8029805/8029806: Remove XXX addPropertyChangeListener and removePropertyChangeListener methods

[Daniel Fuchs]

On 12/10/13 12:06 PM, Alan Bateman wrote:


(This one is for the jdk9-dev forest once it is created)

The addPropertyChangeListener and removePropertyChangeListener methods
defined by Pack200.{Packer,Unpacker} and LogManager methods are
deprecated and flagged with a warning that they will be removed in a
future release. This is highlighted in the EDR and Public Review of JSR
337 and also flagged in JEP 162.

I'd like to swing the axe early in JDK 9 so as to give every opportunity
for anything that might have a dependency. There are a few other
modularity related changes that need to go in early too but this is the
only one that involves the removal of methods.

The webrev with the changes is here:

http://cr.openjdk.java.net/~alanb/8029805%2b8029806/webrev/


Hi Alan

I had a look at the j.u.l part of the fix - test included, and that
looks fine. I am glad to see these go :-)

I applaud at doing this early in JDK 9 - hopefully we will get feedback
early if some kind of replacement appears to be needed.



I've cc'ed build-dev as there are make file changes to remove the
de-beaning rules (these methods do not exist in subset Profiles of Java
SE and were removed as part of the build).


Wahou. Debeaning - I didn't know about that :-) - don't count me as
reviewer for these :-)

-- daniel



-Alan.

Re: JDK7u40: Community Code Review: webrev for non Oracle people?

[Daniel Fuchs]

On 10/4/13 7:05 PM, Francis ANDRE wrote:

Hi

This page http://openjdk.java.net/guide/codeReview.html states that one
should use webrev for providing support of Community Code Review but it
also states that any user with push access to the OpenJDK Mercurial
http://hg.openjdk.java.net server can publish materials on the
cr.openjdk.java.net server.

Is this webrev tool only provided for Oracle people?


Hi Francis,

You will find webrev in the repository:

make/scripts/webrev.ksh

best regards,

-- daniel



Francis

Re: RFR: 8016780: (xs) README-builds.html misses crucial requirement on bootstrap JDK

[Daniel Fuchs]

On 6/18/13 8:28 AM, David Holmes wrote:

On 18/06/2013 4:02 PM, Jonathan Gibbons wrote:

The only problem with using N is that you don't know whether you have
broken building with N-1. Therefore the general recommendation for most
people should be to always use N-1.  I think Stuart is just searching
for ways to make people aware that using N-1 is the right thing to do.


There was certainly an issue here that caused a problem because the 
code used a JDK8 API that was not available when the source was 
compiled with JDK7. And sure compiling with 7u boot JDK would have 
caught that.


But we have lots of code that only compiles with JDK8 and that is the 
way we want it, else JDK8 could not take advantage of any new language 
features or APIs in JDK8. The real problem here was that the code in 
question is code that is not built in a way that allows it to use the 
latest language features or APIs. In which case perhaps the real fix 
is to use build commands that enforce this restriction ie by using 
-source 7 -target 7 ?


David

Hi David,

In the case of Jaxp - I'm not sure why exactly is Jaxp compiled with the 
boot JDK.
It comes early in the build cycle - at a time when the N JDK hasn't been 
compiled yet.
But is this a mere convenience - or is there a good technical reason for 
this?


Because personally - I would love to be able to use JDK N feature in the 
JAXP source

code.
One could argue that using N features impairs the ability to port the 
fixes to N-1, but
this is already the case today anyway: for many of the fixes I ported 
from 8 to 7 I had to
modify my patches because I was using N-1 features in N, and therefore 
had to convert

the code to only use N-2 features when porting from N to N-1.

So if that is possible (and it may not be - I'm not a build expert) - I 
would argue to

remove the restriction for Jaxp - rather than enforce it.

best regards,

-- daniel





-- Jon


On 06/17/2013 10:04 PM, David Holmes wrote:

I thought the only rule was must be buildable by N-1, not that you
must not try to use N!

Can the problem preventing a build using JDK8 as the boot JDK not be
corrected? I'm assuming it is one of the more unusual parts of the
build where we mess with bootclasspath etc?

David

On 18/06/2013 10:21 AM, Stuart Marks wrote:

On 6/17/13 4:02 PM, Kelly O'Hair wrote:

Rule #1 Nobody reads the README
Rule #2 When things go wrong, blame the README

I of course have no objection to the change, however, I'm not
convinced it will
help much the next time someone runs into this. :^(


Hi Kelly! You still read this stuff here? :-)

Yeah, I have no illusions that changing the README will prevent 
many, if

any, future occurrences of this problem. However, we had an internal
discussion on this incident where the N-1 rule was asserted. There was
no dispute about the rule, but I went off to find where it was
documented, and found only the fairly weak statement in the README. 
So,

at the very least, that ought to be fixed.

A stronger step would be to modify configure to check the version 
of the

boot JDK and to complain if it doesn't match N-1. Or perhaps even N-1
and update = 7. What do you think? I was considering filing an RFE.

A restriction in configure would probably be more effective at
preventing these kinds of errors.

s'marks

Re: Fwd: Need help to resolve build errors on Mac os x

[Daniel Fuchs]

Hi,

After cloning jdk8/tl you need to cd to the new clone and run:

$ sh ./get_sources.sh

to get the whole forest. Then you need to run configure
(sh ./configure) and then only can you call make images.

Hope this helps,

(and BTW you may need to install Xcode  its command line tools
 as well as XQuartz if you haven't done so already)

-- daniel


On 4/30/13 11:51 PM, kalyan ram wrote:

Hello Build Dev - I am a newbie to openjdk and want to get up to speed on
compiling and installing from source.

I have followed instructions to build from source for Mac OS X.

I however face the following error on attempting to build from source

## Starting jdk
make[2]: *** No rule to make target
`/Volumes/extDrive/Openjdk_source/myjdk8/build/macosx-x86_64-normal-server-release/corba/dist/lib/classes.jar',
needed by
`/Volumes/extDrive/Openjdk_source/myjdk8/build/macosx-x86_64-normal-server-release/jdk/classes/_the.CORBA.classes.imported'.
  Stop.
make[1]: *** [import-only] Error 2
make: *** [jdk-only] Error 2

I have cloned from

hg clone http://hg.openjdk.java.net/jdk8/tl jdk8_tl

and I am following instructions posted at(
https://java.net/projects/adoptopenjdk/pages/MakeImages)

Appreciate all the help in advance.

Regards,
Kalyan

Re: RFR: 8011347: JDK-8009824 has broken webrev with some ksh versions

[Daniel Fuchs]

On 4/16/13 5:02 PM, Jim Gish wrote:

I've updated the version to 24.0 and add Mike as a reviewer.  Could
someone please push this for me?


Hi Jim,

Done.

Thanks for the fix!

-- daniel



Thanks,
 Jim

On 04/15/2013 06:34 PM, Mike Duigou wrote:

I think the version number needs to be changed. My vote is to bump it
to 24.0

Mike

On Apr 12 2013, at 11:08 , Jim Gish wrote:


Please review http://cr.openjdk.java.net/~jgish/Bug8011347-webrev/
http://cr.openjdk.java.net/%7Ejgish/Bug8011347-webrev/, which fixes
the current webrev issues on solaris and mac.

Thanks,
   Jim

--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com

Re: RFR: 8011347: JDK-8009824 has broken webrev with some ksh versions

[Daniel Fuchs]

On 4/16/13 5:02 PM, Jim Gish wrote:

I've updated the version to 24.0 and add Mike as a reviewer.  Could
someone please push this for me?


Hi Jim,

I think you will need to update the bug id in your comment:

http://cr.openjdk.java.net/~jgish/Bug8011347-webrev/tl.changeset

8009824: JDK-8009824 has broken webrev with some ksh versions
^^^

shouldn't that be 8011347 ?

-- daniel




Thanks,
 Jim

On 04/15/2013 06:34 PM, Mike Duigou wrote:

I think the version number needs to be changed. My vote is to bump it
to 24.0

Mike

On Apr 12 2013, at 11:08 , Jim Gish wrote:


Please review http://cr.openjdk.java.net/~jgish/Bug8011347-webrev/
http://cr.openjdk.java.net/%7Ejgish/Bug8011347-webrev/, which fixes
the current webrev issues on solaris and mac.

Thanks,
   Jim

--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com

Re: JDK-8010495: Update JAXP NetBeans project - add support for generating javadoc

[Daniel Fuchs]

Hi,

Please find below a revised patch:

http://cr.openjdk.java.net/~dfuchs/JDK-8010495/jdk8/webrev.01/

I had oversimplified the changes in project.xml.
It seems you need to declare a source folder of type 'java'
as well as a compilation-unit in order for 'Find Usage' to work
properly in the editor.

I have also restrained the packageset for the javadoc
target as suggested by Joe.

-- daniel


On 3/25/13 12:45 PM, Daniel Fuchs wrote:

Hi guys,

I'd like to propose a small change to jaxp/build.xml and
jaxp/nbproject/project.xml - in order to allow editing
JAXP sources in NetBeans, as well as generating the JAXP
javadoc for previewing purposes.

I actually stole the javadoc target from
jdk/make/netbeans/common/shared.xml

The changes are small  targeted at developers.
There should be no impact on the build process.

Here is the webrev:

http://cr.openjdk.java.net/~dfuchs/JDK-8010495/jdk8/webrev.00/

best regards,

-- daniel

JDK-8010495: Update JAXP NetBeans project - add support for generating javadoc

[Daniel Fuchs]

Hi guys,

I'd like to propose a small change to jaxp/build.xml and
jaxp/nbproject/project.xml - in order to allow editing
JAXP sources in NetBeans, as well as generating the JAXP
javadoc for previewing purposes.

I actually stole the javadoc target from
jdk/make/netbeans/common/shared.xml

The changes are small  targeted at developers.
There should be no impact on the build process.

Here is the webrev:

http://cr.openjdk.java.net/~dfuchs/JDK-8010495/jdk8/webrev.00/

best regards,

-- daniel

Re: Building on Mac

[Daniel Fuchs]

Hi Peter,

Did you 'install the command line tools' in Xcode?

The fact that you have to refer to /Applications/Xcode.app/... let me 
think that

perhaps you didn't.
Start Xcode, go to preferences, and explore the different panes: there's 
one that

will let you install the command line tools.

If that was the issue - then after installing the command line tools you 
should be

able to run configure without the two options below.

Hope this helps,

-- daniel

On 3/18/13 7:56 AM, Peter Zhelezniakov wrote:

On Mar 18, 2013, at 3:41 AM, David Holmes david.hol...@oracle.com wrote:

Are you trying to build OpenJDK or Oracle JDK?

Oracle JDK -- I have jdk/make/closed and jdk/src/closed.

I've parsed the output of --debug-configure, and realised I need the 
--enable-macosx-runtime-support option. With this option, configure requires 
neither X11 nor freetype. However, the build still fails without X headers. So 
the complete command line that led to successful build for me was:

bash ./configure \
--x-includes=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.7.sdk/usr/X11/include
 \
--x-libraries=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.7.sdk/usr/X11/lib
  \
--enable-macosx-runtime-support

Thanks!

Re: Compiler build Question

[Daniel Fuchs]

I Tamer,

I believe you need to define this variable:

bootstrap.jdk=path to an existing jdk 6 or 7

in $HOME/.openjdk/build.properties.

see make/netbeans/README
http://hg.openjdk.java.net/jdk7/jdk7/jdk/file/d782143219d6/make/netbeans/README

Best regards,

-- daniel
http://blogs.sun.com/jmxetc



tamer abd el-lateef wrote:

Dear Members;

I downloaded the openJDK openjdk-7-ea-src-b40-20_nov_2008.zip then i 
added the compiler peoject Javac to the Netbeans IDE ver 6.5 .


- in the build.properties file i updated the target.java.home property 
to my JDK path.


then when i tried to copile the project i got the following error :

fail message=bootstrap java is not installed in Mypath }
where Mypath is my Installed JDK path.

Please find the attached snapshot.

and i am waiting for your advice.

thank you in advance



--
Tamer Mohamed Abd El-lateef
Senior Software Developer

Re: Can somebody sanity-check me here?

[Daniel Fuchs]

Hi Ted,

To complete Erick's answer I believe you will find some in-depth
explanation on Kelly's blog - and more particularly in these two
articles:

OpenJDK Mercurial Forest
http://blogs.sun.com/kto/entry/openjdk_mercurial_forest

and OpenJDK Mercurial Wheel (Integration Wheel of Open Life)
http://blogs.sun.com/kto/entry/openjdk_mercurial_wheel

To illustrate what Kelly is saying in these posts, as a
JMX developper, I am pulling from
  http://hg.openjdk.java.net/jdk7/tl
and pushing into
  http://hg.openjdk.java.net/jdk7/tl-gate
because traditionally the JMX APIs are integrated in 'tl'

The http://hg.openjdk.java.net/jdk7/tl (or 
http://hg.openjdk.java.net/jdk7/jdk7) are two roots for the

whole forest shown in:
http://blogs.sun.com/kto/entry/openjdk_mercurial_forest

The http://hg.openjdk.java.net/jdk7/tl is one of the integration
forests shown in http://blogs.sun.com/kto/entry/openjdk_mercurial_wheel
(one of the forests shown on the circle itself)
and http://hg.openjdk.java.net/jdk7/jdk7 is the master which sits
at the center of the wheel. Each integration forest is a clone of
the master sitting at the center of the wheel, and each of the
integration forest may contain fixes which have not yet been integrated
in the master (but will be).

So if you want to work from the sources already integrated in the
JDK 7 master, what you need to pull is:

  hg fclone http://hg.openjdk.java.net/jdk7/jdk7

By using fclone you will pull the whole forest shown
in  http://blogs.sun.com/kto/entry/openjdk_mercurial_forest

Note that if you only want to work on the JDK APIs - and if you
don't want to rebuild a full image of the JDK then pulling the
single tree
   http://hg.openjdk.java.net/jdk7/jdk7/jdk
might be enough.


Hope this helps,

-- daniel
http://blogs.sun.com/jmxetc

Erik Trimble wrote:

Ted Neward wrote:


I go to http://hg.openjdk.java.net, and there’s about 4 billion 
different repositories listed there, including several variations of 
repositories that I thought I was already pulling: hotspot, corba, 
jdk, and so on. (The variations are things like jdk7/corba-gate, 
jdk7/hotspot-comp, jdk7/hotspot-gc, and so on.) Do I need to 
explicitly pull these guys? Or are they somehow being brought in to 
the “core” name repositories?


 


Ted Neward

Java, .NET, XML Services

Consulting, Teaching, Speaking, Writing

http://www.tedneward.com

 

These are the promotion and sub-group repositories, and you don't need 
to explicitly pull any of them.


Developer work goes into many of these, which are then run up the repo 
trees after testing.   Work for a given repository is pushed to the 
corresponding *-gate repo, and then internally promoted.


The latest tested build of the full JDK can always be found in the 
http://hg.openjdk.java.net/jdk7/jdk7  forest of repositories.