Integrated: 8284890: Support for Do not fragment IP socket options
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 This pull request has now been integrated. Changeset: 67755edd Author:Michael McMahon URL: https://git.openjdk.java.net/jdk/commit/67755edd6ff2e2eeafafec207d0459bca53f882b Stats: 498 lines in 11 files changed: 495 ins; 0 del; 3 mod 8284890: Support for Do not fragment IP socket options Reviewed-by: erikj, ihse, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v8]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8245/files - new: https://git.openjdk.java.net/jdk/pull/8245/files/428a9801..e6b12eb8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8245=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8245=06-07 Stats: 20 lines in 1 file changed: 2 ins; 17 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v7]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8245/files - new: https://git.openjdk.java.net/jdk/pull/8245/files/1e08ee9a..428a9801 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8245=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8245=05-06 Stats: 18 lines in 1 file changed: 18 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v5]
On Tue, 19 Apr 2022 16:07:29 GMT, Daniel Fuchs wrote: >> 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. I don't mind adding it. Though, the no-arg constructor is the same as cases 2. or 3. depending on the value of the preferIPv4Stack property. - PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v6]
> 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: typo in windows native code - Changes: - all: https://git.openjdk.java.net/jdk/pull/8245/files - new: https://git.openjdk.java.net/jdk/pull/8245/files/509c3f81..1e08ee9a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8245=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8245=04-05 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v5]
On Tue, 19 Apr 2022 14:50:57 GMT, Daniel Fuchs wrote: >> 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? Yes, there is. > 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. - PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v5]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8245/files - new: https://git.openjdk.java.net/jdk/pull/8245/files/5f1d87eb..509c3f81 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8245=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8245=03-04 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v4]
> 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: updates - Changes: - all: https://git.openjdk.java.net/jdk/pull/8245/files - new: https://git.openjdk.java.net/jdk/pull/8245/files/e90aa7c3..5f1d87eb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8245=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8245=02-03 Stats: 42 lines in 2 files changed: 13 ins; 10 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v3]
> 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 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 25 additional commits since the last revision: - windows update - test update - Merge branch 'master' into mtu - builds in github action now - fix whitespace errors - minor spec update - windows 2016 issue - windows issue - windows update - test update - ... and 15 more: https://git.openjdk.java.net/jdk/compare/3b3bec01...e90aa7c3 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8245/files - new: https://git.openjdk.java.net/jdk/pull/8245/files/14c776b1..e90aa7c3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8245=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8245=01-02 Stats: 148852 lines in 1314 files changed: 108219 ins; 7158 del; 33475 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v2]
On Fri, 15 Apr 2022 10:04:48 GMT, Daniel Jeliński wrote: >> Michael McMahon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> builds in github action now > > src/jdk.net/windows/native/libextnet/WindowsSocketOptions.c line 73: > >> 71: if (family == AF_INET) { >> 72: opt = optval; >> 73: rv = setsockopt(fd, IPPROTO_IP, IP_DONTFRAGMENT, (char *), >> sizeof(int)); > > Why do we only use `IPV6_MTU_DISCOVER` but not `IP_MTU_DISCOVER`? As far as I > can tell, `IP_DONTFRAGMENT` alone doesn't guarantee that the DF bit will be > set. I did (manually) check that the DF bit is set, though unfortunately, there's no straightforward way to test that in the regression test. We could have the same construction for AF_INET as AF_INET6 and try IP_MTU_DISCOVER first which won't work pre Windows 10/2019. So, I'll make that change. - PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options [v2]
> 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: builds in github action now - Changes: - all: https://git.openjdk.java.net/jdk/pull/8245/files - new: https://git.openjdk.java.net/jdk/pull/8245/files/446dd6cf..14c776b1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8245=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8245=00-01 Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284890: Support for Do not fragment IP socket options
On Fri, 15 Apr 2022 09:19:48 GMT, Daniel Fuchs 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() ? Good idea - PR: https://git.openjdk.java.net/jdk/pull/8245
RFR: 8284890: Support for Do not fragment IP socket options
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 - Commit messages: - fix whitespace errors - minor spec update - windows 2016 issue - windows issue - windows update - test update - updates - simplified test. Loosened spec - Merge branch 'master' into mtu - fixed test - ... and 11 more: https://git.openjdk.java.net/jdk/compare/40ddb755...446dd6cf Changes: https://git.openjdk.java.net/jdk/pull/8245/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8245=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284890 Stats: 437 lines in 11 files changed: 434 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8245/head:pull/8245 PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8277459: Add jwebserver tool [v3]
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 LGTM - Marked as reviewed by michaelm (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6497
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v9]
On Tue, 12 Oct 2021 10:40:15 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 24 commits: > > - Minor rewording of bind address output > - Merge branch 'master' into simpleserver > - Merge branch 'master' into simpleserver > - update output for all interfaces > - 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 > - ... and 14 more: > https://git.openjdk.java.net/jdk/compare/b1b66965...e86609d0 Marked as reviewed by michaelm (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v9]
On Tue, 12 Oct 2021 10:40:15 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 24 commits: > > - Minor rewording of bind address output > - Merge branch 'master' into simpleserver > - Merge branch 'master' into simpleserver > - update output for all interfaces > - 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 > - ... and 14 more: > https://git.openjdk.java.net/jdk/compare/b1b66965...e86609d0 test/jdk/com/sun/net/httpserver/simpleserver/ServerMimeTypesResolutionTest.java line 200: > 198: */ > 199: //@Test(dataProvider = "commonExtensions") > 200: public static void testCommonExtensions(String extension) { Is this test and the one below supposed to be commented out? - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]
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/SimpleFileServerImpl.java line 135: > 133: var socketAddr = new InetSocketAddress(addr, port); > 134: var server = SimpleFileServer.createFileServer(socketAddr, > root, outputLevel); > 135: server.setExecutor(Executors.newSingleThreadExecutor()); I think this code has the effect of creating one thread for the selector and a second one for the execution of the handlers. If we want to keep thread usage to an absolute minimum then it might be better to not set an executor. Then, the selector thread executes the handlers as well. - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]
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 Changes requested by michaelm (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]
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 test/jdk/com/sun/net/httpserver/FilterTest.java line 330: > 328: var response = client.send(request, > HttpResponse.BodyHandlers.ofString()); > 329: assertEquals(response.statusCode(), 200); > 330: assertEquals(inspectedURI.get(), URI.create("/")); Could you make the request URI something like /foo/bar instead of just / here? - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v2]
On Wed, 15 Sep 2021 08:42:40 GMT, Julia Boes wrote: >> src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpsServer.java >> line 152: >> >>> 150: return server; >>> 151: } >>> 152: >> >> Too bad we couldn't simplify the setting up a basic certificate for https. > > That would be nice indeed, but the goal of this JEP is a minimal HTTP-only > server, intentionally leaving anything HTTPS aside. `HttpsServer::create` > being the exception, added to provide the same convenience as for > `HttpServer`. Any HTTPS configuration can be done using the existing API. I agree the JEP should focus on a minimal HTTP server and the new API does allow an HTTPS based file server to be setup in one or two lines of code. I don't think there would be much value in providing something like self signed certificates, but it has become a lot easier to obtain real https certificates through services like LetsEncrypt so it might be interesting to write an article for inside.java showing how to set up a HTTPS server from start to finish. - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server
On Tue, 14 Sep 2021 16:51:40 GMT, Daniel Fuchs wrote: >> 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). I agree with your second suggestion. It's better not to refer to the 'Content-Length' header at all. - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245194: Unix domain socket channel implementation [v16]
> Continuing this review as a PR on github with the comments below > incorporated. I expect there will be a few more > iterations before integrating. > On 06/09/2020 19:47, Alan Bateman wrote: >> On 26/08/2020 15:24, Michael McMahon wrote: >>> >>> As I mentioned the other day, I wasn't able to use the suggested method on >>> Windows which returns an absolute path. So, >>> I added a method to WindowsPath which returns the path in the expected >>> UTF-8 encoding as a byte[]. Let me know what you >>> think of that. >>> >>> There is a fair bit of other refactoring and simplification done also. Next >>> version is at: >>> >>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/ >>> >> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I >> don't think we should be caching it as the >> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about >> encoding a relative path when the resolved >> path (before encoding) is > 247 chars. The documentation on the MS site >> isn't very completely and I think there are a >> number points that require clarification to fully understand how this will >> work with relative, directly relative and >> drive relative paths. >> > > Maybe it would be better to just use the path returned from toString() and do > the conversion to UTF-8 externally. That > would leave WindowsPath unchanged. >> In the same area, the new PathUtil is a bit inconsistent with the existing >> provider code. One suggestion is to add a >> method to AbstractFileSystemProvider instead. That is the base class the >> platform providers and would be a better place >> to get the file path in bytes. >> > > Okay, I gave the method a name that is specific to Unix domain sockets > because this UTF-8 strangeness is not likely to > be useful by other components. >> One other comment on the changes to the file system provider it should be >> okay to change UnixUserPrinipals ad its >> fromUid and fromGid method to be public. This would mean that the patch >> shouldn't need to add UnixUserGroup (the main >> issue is that class is that it means we end up with two classes with static >> factory methods doing the same thing). > > Okay, that does simplify it a bit. > > Thanks, > Michael. > >> -Alan. >> >> >> >> >> >> Michael McMahon 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 18 additional commits since the last revision: - Merge branch 'master' into unixdomainchannels - - update after Alan's review on Oct 4 - includes API change required by JDK-8251952 - original CSR for the overall change will be resubmitted with all api changes consolidated based on this update - - simplified Copy.gmk to CAT source files directly - renamed net.properties source files to all be net.properties - unixdomainchannels: error in the last commit in make/modules/java.base/Copy.gmk - unixdomainchannels: (1) rename UnixDomainHelper to UnixDomainSocketsUtil (2) remove hardcoded /tmp and /var/tmp paths from UnixDomainSocketsUtil (3) replace (2) with documented system/networking properties (4) Small update to UnixDomainSocketAddress API (5) CSR for (3) and (4) submitted at JDK-8253930 (6) Update build to generate net.properties from shared net.properties.common plus platform specific additions. - Merge branch 'master' into unixdomainchannels - unixdomainchannels: remove more cruft from Windows Net.c - unixdomainchannels: change to Net.c was missed after all - unixdomainchannels: typo in WindowsFileSystemProvider.java - unixdomainchannels: Update after Alan's review of Sept 29 - ... and 8 more: https://git.openjdk.java.net/jdk/compare/96b742dd...36efb46c - Changes: - all: https://git.openjdk.java.net/jdk/pull/52/files - new: https://git.openjdk.java.net/jdk/pull/52/files/17acf10a..36efb46c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=52=15 - incr: https://webrevs.openjdk.java.net/?repo=jdk=52=14-15 Stats: 15436 lines in 1715 files changed: 5751 ins; 2807 del; 6878 mod Patch: https://git.openjdk.java.net/jdk/pull/52.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52 PR: https://git.openjdk.java.net/jdk/pull/52
Re: RFR: 8245194: Unix domain socket channel implementation [v15]
> Continuing this review as a PR on github with the comments below > incorporated. I expect there will be a few more > iterations before integrating. > On 06/09/2020 19:47, Alan Bateman wrote: >> On 26/08/2020 15:24, Michael McMahon wrote: >>> >>> As I mentioned the other day, I wasn't able to use the suggested method on >>> Windows which returns an absolute path. So, >>> I added a method to WindowsPath which returns the path in the expected >>> UTF-8 encoding as a byte[]. Let me know what you >>> think of that. >>> >>> There is a fair bit of other refactoring and simplification done also. Next >>> version is at: >>> >>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/ >>> >> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I >> don't think we should be caching it as the >> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about >> encoding a relative path when the resolved >> path (before encoding) is > 247 chars. The documentation on the MS site >> isn't very completely and I think there are a >> number points that require clarification to fully understand how this will >> work with relative, directly relative and >> drive relative paths. >> > > Maybe it would be better to just use the path returned from toString() and do > the conversion to UTF-8 externally. That > would leave WindowsPath unchanged. >> In the same area, the new PathUtil is a bit inconsistent with the existing >> provider code. One suggestion is to add a >> method to AbstractFileSystemProvider instead. That is the base class the >> platform providers and would be a better place >> to get the file path in bytes. >> > > Okay, I gave the method a name that is specific to Unix domain sockets > because this UTF-8 strangeness is not likely to > be useful by other components. >> One other comment on the changes to the file system provider it should be >> okay to change UnixUserPrinipals ad its >> fromUid and fromGid method to be public. This would mean that the patch >> shouldn't need to add UnixUserGroup (the main >> issue is that class is that it means we end up with two classes with static >> factory methods doing the same thing). > > Okay, that does simplify it a bit. > > Thanks, > Michael. > >> -Alan. >> >> >> >> >> >> Michael McMahon has updated the pull request incrementally with one additional commit since the last revision: - update after Alan's review on Oct 4 - includes API change required by JDK-8251952 - original CSR for the overall change will be resubmitted with all api changes consolidated based on this update - Changes: - all: https://git.openjdk.java.net/jdk/pull/52/files - new: https://git.openjdk.java.net/jdk/pull/52/files/0096645e..17acf10a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=52=14 - incr: https://webrevs.openjdk.java.net/?repo=jdk=52=13-14 Stats: 327 lines in 35 files changed: 108 ins; 125 del; 94 mod Patch: https://git.openjdk.java.net/jdk/pull/52.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52 PR: https://git.openjdk.java.net/jdk/pull/52
Re: RFR: 8245194: Unix domain socket channel implementation [v14]
On Mon, 5 Oct 2020 12:58:52 GMT, Erik Joelsson wrote: >> Michael McMahon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> - simplified Copy.gmk to CAT source files directly >> - renamed net.properties source files to all be net.properties > > Build changes look good. Thanks again Alan. Assume where there is no comment from me below that suggestions are accepted: I will push an update based on these changes soon. Michael. > _Mailing list message from [Alan Bateman](mailto:alan.bate...@oracle.com) on > [nio-dev](mailto:nio-...@openjdk.java.net):_ > On 04/10/2020 14:14, Michael McMahon wrote: > Another round of comments, this time on v14. > > src/java.base/share/classes/java/net/StandardProtocolFamily.java > - the enum constant is "UNIX" and might be better to put "Local" in > parentheses rather than "Unix domain". > > src/java.base/share/classes/java/nio/channels/package-info.java > - L260 ends with "family only", I think "only" can be dropped. > > src/java.base/share/classes/java/net/UnixDomainSocketAddress.java > - left over "fix message" comments in constructor > - a few formatting nits in the equals method due to a spurious space in > the expression at L192, and missing a space at L194. > I don't see a missing space at L194 ? > src/java.base/share/classes/sun/net/util/SocketExceptions.java > - Can of(IOException e, SocketAddress addr) check enhancedExceptionText > to avoid the ugly check in the ofXXX methods? > - Can you explain the inconsistency in the null handling for the address > types, maybe a left over from an early prototype? > The null check was always there for Inet sockets. It is not required for Unix but would be benign. So, the check can be promoted to the calling method. > src/java.base/share/classes/sun/nio/ch/ServerSocketChannelImpl.java > src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java > - I think we need to change the class descriptions of both to start with > "Base implementations ..." > - We need to find a clean way to push the socket() method down to the > InetXXXImpl classes as they are the only classes that should know about > the socket adaptors. Only way to do this is to define yet another implXXX method (implSocket) but I think it is worthwhile as neither the field nor the implementation belong in the super class > > src/java.base/share/classes/sun/nio/ch/UnixDomainServerSocketChannelImpl.java > - implBind should not loop/retry when a local address is provided It actually catches the BindException and rethrows it when 'local != null, but maybe the comment above the loop is confusing. I will reword it. > - what behavior is expected when getTempDir returns null? I was assuming that ${java.io.tmpdir} would always be defined, but I see getTempDir() could be more robust in dealing with errors in the preceding search steps. So, in the unlikely event that ${java.io.tmpdir} is not defined null would be returned. I will change to throw a BindException in that case. > - Why doesn't this code use SecureRandom, as is done in the Windows > Selector implementation. It does use SecureRandom but only uses the Random api. > - Can supportedOptions be changed to return > Set.of(StandardSocketOptions.SO_RCVBUF)? I can simplify that code definitely, but I think for consistency it should still return a static unmodifiable instance > - toString has a left over "TODO". > - The message for the IOException in implBind should start with a > capital to be consistent with the other exceptions thrown in this area > > src/java.base/share/classes/sun/nio/ch/UnixDomainSockets.java > - The initializer sets the system property > "jdk.nio.channels.unixdomain.maxnamelength". Looks like it's to help the > tests so need to find another solution for the tests. Okay, the tests didn't really need this anyway > - checkCapability should be renamed to checkPermission as it does a > security manager check. Would it more be consistent with existing code > if changed to "if (sm != null) sm.checkPermission(...)" > - Can inTempDirectory be removed as it doesn't seem to be used and > raises several questions if it is needed. > - getTempName should use the path separator rather than "/".? Also I > don't think "nio" should be in the name. > - Can the init method be removed, may be a left over from a previous > iteration. > - The left breaks in the initialization of UNNAMED and support seem > unnecessary, maybe they were very long in previous iterations? > - Several methods are public, is that intentional, or maybe a left over? > > src/java.base/unix/native/libnio/ch/UnixDomainSockets.c > =>
Re: RFR: 8245194: Unix domain socket channel implementation [v14]
> Continuing this review as a PR on github with the comments below > incorporated. I expect there will be a few more > iterations before integrating. > On 06/09/2020 19:47, Alan Bateman wrote: >> On 26/08/2020 15:24, Michael McMahon wrote: >>> >>> As I mentioned the other day, I wasn't able to use the suggested method on >>> Windows which returns an absolute path. So, >>> I added a method to WindowsPath which returns the path in the expected >>> UTF-8 encoding as a byte[]. Let me know what you >>> think of that. >>> >>> There is a fair bit of other refactoring and simplification done also. Next >>> version is at: >>> >>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/ >>> >> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I >> don't think we should be caching it as the >> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about >> encoding a relative path when the resolved >> path (before encoding) is > 247 chars. The documentation on the MS site >> isn't very completely and I think there are a >> number points that require clarification to fully understand how this will >> work with relative, directly relative and >> drive relative paths. >> > > Maybe it would be better to just use the path returned from toString() and do > the conversion to UTF-8 externally. That > would leave WindowsPath unchanged. >> In the same area, the new PathUtil is a bit inconsistent with the existing >> provider code. One suggestion is to add a >> method to AbstractFileSystemProvider instead. That is the base class the >> platform providers and would be a better place >> to get the file path in bytes. >> > > Okay, I gave the method a name that is specific to Unix domain sockets > because this UTF-8 strangeness is not likely to > be useful by other components. >> One other comment on the changes to the file system provider it should be >> okay to change UnixUserPrinipals ad its >> fromUid and fromGid method to be public. This would mean that the patch >> shouldn't need to add UnixUserGroup (the main >> issue is that class is that it means we end up with two classes with static >> factory methods doing the same thing). > > Okay, that does simplify it a bit. > > Thanks, > Michael. > >> -Alan. >> >> >> >> >> >> Michael McMahon has updated the pull request incrementally with one additional commit since the last revision: - simplified Copy.gmk to CAT source files directly - renamed net.properties source files to all be net.properties - Changes: - all: https://git.openjdk.java.net/jdk/pull/52/files - new: https://git.openjdk.java.net/jdk/pull/52/files/dc70cd6e..0096645e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=52=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=52=12-13 Stats: 9 lines in 4 files changed: 0 ins; 4 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/52.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52 PR: https://git.openjdk.java.net/jdk/pull/52
Re: RFR: 8245194: Unix domain socket channel implementation [v13]
On Sun, 4 Oct 2020 08:27:39 GMT, Alan Bateman wrote: >> Good points. I will update as suggested. Thanks. > > I would prefer if we didn't rename net.properties. Can we use the same > approach as lib/security/default.policy where > the share and platform specific are concatenated? Okay, I have just pushed these suggested updates. - PR: https://git.openjdk.java.net/jdk/pull/52
Re: RFR: 8245194: Unix domain socket channel implementation [v13]
On Fri, 2 Oct 2020 12:58:02 GMT, Erik Joelsson wrote: >> Michael McMahon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> unixdomainchannels: error in the last commit in >> make/modules/java.base/Copy.gmk > > make/modules/java.base/Copy.gmk line 195: > >> 193: $(call MakeTargetDir) >> 194: $(RM) $@ $@.tmp >> 195: $(foreach f,$(NET_PROPERTIES_SRC_LIST),$(CAT) $(f) >> $@.tmp;) > > This can be simplified. Cat takes multiple files as input, so no need for > 'foreach'. Also no need to go via a temp > file. We have make configured to delete targets if a recipe fails, so the tmp > dance isn't needed. (I know we still have > this pattern all over the place, but we are trying to quit the practice) Good points. I will update as suggested. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/52
Re: RFR: 8245194: Unix domain socket channel implementation [v13]
> Continuing this review as a PR on github with the comments below > incorporated. I expect there will be a few more > iterations before integrating. > On 06/09/2020 19:47, Alan Bateman wrote: >> On 26/08/2020 15:24, Michael McMahon wrote: >>> >>> As I mentioned the other day, I wasn't able to use the suggested method on >>> Windows which returns an absolute path. So, >>> I added a method to WindowsPath which returns the path in the expected >>> UTF-8 encoding as a byte[]. Let me know what you >>> think of that. >>> >>> There is a fair bit of other refactoring and simplification done also. Next >>> version is at: >>> >>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/ >>> >> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I >> don't think we should be caching it as the >> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about >> encoding a relative path when the resolved >> path (before encoding) is > 247 chars. The documentation on the MS site >> isn't very completely and I think there are a >> number points that require clarification to fully understand how this will >> work with relative, directly relative and >> drive relative paths. >> > > Maybe it would be better to just use the path returned from toString() and do > the conversion to UTF-8 externally. That > would leave WindowsPath unchanged. >> In the same area, the new PathUtil is a bit inconsistent with the existing >> provider code. One suggestion is to add a >> method to AbstractFileSystemProvider instead. That is the base class the >> platform providers and would be a better place >> to get the file path in bytes. >> > > Okay, I gave the method a name that is specific to Unix domain sockets > because this UTF-8 strangeness is not likely to > be useful by other components. >> One other comment on the changes to the file system provider it should be >> okay to change UnixUserPrinipals ad its >> fromUid and fromGid method to be public. This would mean that the patch >> shouldn't need to add UnixUserGroup (the main >> issue is that class is that it means we end up with two classes with static >> factory methods doing the same thing). > > Okay, that does simplify it a bit. > > Thanks, > Michael. > >> -Alan. >> >> >> >> >> >> Michael McMahon has updated the pull request incrementally with one additional commit since the last revision: unixdomainchannels: error in the last commit in make/modules/java.base/Copy.gmk - Changes: - all: https://git.openjdk.java.net/jdk/pull/52/files - new: https://git.openjdk.java.net/jdk/pull/52/files/275e0928..dc70cd6e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=52=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=52=11-12 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/52.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52 PR: https://git.openjdk.java.net/jdk/pull/52
Re: RFR: 8245194: Unix domain socket channel implementation [v12]
> Continuing this review as a PR on github with the comments below > incorporated. I expect there will be a few more > iterations before integrating. > On 06/09/2020 19:47, Alan Bateman wrote: >> On 26/08/2020 15:24, Michael McMahon wrote: >>> >>> As I mentioned the other day, I wasn't able to use the suggested method on >>> Windows which returns an absolute path. So, >>> I added a method to WindowsPath which returns the path in the expected >>> UTF-8 encoding as a byte[]. Let me know what you >>> think of that. >>> >>> There is a fair bit of other refactoring and simplification done also. Next >>> version is at: >>> >>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/ >>> >> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I >> don't think we should be caching it as the >> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about >> encoding a relative path when the resolved >> path (before encoding) is > 247 chars. The documentation on the MS site >> isn't very completely and I think there are a >> number points that require clarification to fully understand how this will >> work with relative, directly relative and >> drive relative paths. >> > > Maybe it would be better to just use the path returned from toString() and do > the conversion to UTF-8 externally. That > would leave WindowsPath unchanged. >> In the same area, the new PathUtil is a bit inconsistent with the existing >> provider code. One suggestion is to add a >> method to AbstractFileSystemProvider instead. That is the base class the >> platform providers and would be a better place >> to get the file path in bytes. >> > > Okay, I gave the method a name that is specific to Unix domain sockets > because this UTF-8 strangeness is not likely to > be useful by other components. >> One other comment on the changes to the file system provider it should be >> okay to change UnixUserPrinipals ad its >> fromUid and fromGid method to be public. This would mean that the patch >> shouldn't need to add UnixUserGroup (the main >> issue is that class is that it means we end up with two classes with static >> factory methods doing the same thing). > > Okay, that does simplify it a bit. > > Thanks, > Michael. > >> -Alan. >> >> >> >> >> >> Michael McMahon has updated the pull request incrementally with one additional commit since the last revision: unixdomainchannels: (1) rename UnixDomainHelper to UnixDomainSocketsUtil (2) remove hardcoded /tmp and /var/tmp paths from UnixDomainSocketsUtil (3) replace (2) with documented system/networking properties (4) Small update to UnixDomainSocketAddress API (5) CSR for (3) and (4) submitted at JDK-8253930 (6) Update build to generate net.properties from shared net.properties.common plus platform specific additions. - Changes: - all: https://git.openjdk.java.net/jdk/pull/52/files - new: https://git.openjdk.java.net/jdk/pull/52/files/c40d2059..275e0928 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=52=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=52=10-11 Stats: 357 lines in 10 files changed: 194 ins; 154 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/52.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52 PR: https://git.openjdk.java.net/jdk/pull/52
Re: RFR 8210318: idea.sh script doesn't work on Mac
The change looks fine to me Maurizio. Maybe you could append a .$$ to the temporary file name to make it less likely to overwrite something that already exists in /tmp - Michael. On 03/09/2018, 13:39, Maurizio Cimadamore wrote: Hi, following the latest updates to the idea.sh script, Mac users reported issues - mostly having to do with usage of 'sed' - more specifically: * sed -i option is not portable - it has different formats in Mac vs. Linux. This patch does without -i, by moving the replaced file onto a temporary file, then moving such file on top of the template file in a subsequent step. This should be more robust. * sed doesn't like newlines in replaced text in Mac. I've thus omitted the newline from the SOURCE template - as that was mostly cosmetic. Thanks for Michael McMahon to report (and figure out how to deal with) these issues, and to Alan Bateman for testing the patch. I also fixed another minor glitch, this time in the langtools-only template - which was still referring to the old ant file location in the various run configuration. Webrev: http://cr.openjdk.java.net/~mcimadamore/8210318/ Cheers Maurizio
Re: RFR [9] Warning notice for types in Incubator Modules
Hi Chris As regards the networking changes, they look fine to me. As discussed off list, I think the link generated by the @Incubating tag might possibly point to a new page in the docs bundle rather than directly to the JEP 11 page. But, that can be the subject of another change. Thanks, Michael. On 24/01/17 14:08, Chris Hegarty wrote: As per the guidance in JEP 11 [1], the javadoc for types in incubator modules should have a clear and explicit warning notice. To that end, I’ve added a simple inline taglet that can be used to effectively inject a standard notice, and applied it to all public types in the HTTP module ( I’ll cleanup the line width formatting before pushing ). http://cr.openjdk.java.net/~chegar/incubator_taglet/ -Chris. P.S. this work is, somewhat, in preparation for when we move to a single documentation bundle [2]. [1] http://openjdk.java.net/jeps/11 [2] http://openjdk.java.net/jeps/299
Re: RFR [9] : get_source.sh should be more friendly to MQ
That's very useful Chris. I wonder is it okay to assume that all patches must be pushed back again after the update? Would it be feasible to remember which (if any) patches had been popped first, and only push the same ones again? Michael On 11/04/14 15:58, Chris Hegarty wrote: Anyone using MQ for their daily development will know about this, forgetting to qpop before sync'ing up. It would be nice it get_source would pop and push patches ( only if you are using MQ ) automatically. If you do not have patch repos, then there is no change. diff --git a/get_source.sh b/get_source.sh --- a/get_source.sh +++ b/get_source.sh @@ -28,6 +28,21 @@ # Get clones of all nested repositories sh ./common/bin/hgforest.sh clone $@ || exit 1 +patchdirs=`ls -d ./.hg/patches ./*/.hg/patches ./*/*/.hg/patches \ + ./*/*/*/.hg/patches ./*/*/*/*/.hg/patches 2/dev/null` + +# Pop all patches, if any, before updating +if [ ${patchdirs} != ] ; then + echo Found queue repository, qpop. + sh ./common/bin/hgforest.sh qpop -a || exit 1 +fi + # Update all existing repositories to the latest sources -sh ./common/bin/hgforest.sh pull -u +sh ./common/bin/hgforest.sh pull -u || exit 1 +# Push all patches, if any, after updating +if [ ${patchdirs} != ] ; then + echo Found queue repository, qpush. + sh ./common/bin/hgforest.sh qpush -a +fi + -Chris.
Re: RFR [9] : get_source.sh should be more friendly to MQ
On 11/04/14 16:19, Chris Hegarty wrote: On 11/04/14 15:59, Michael McMahon wrote: That's very useful Chris. I wonder is it okay to assume that all patches must be pushed back again after the update? Would it be feasible to remember which (if any) patches had been popped first, and only push the same ones again? That would require a much more involved set of changes in hgforest, but could be doable. All you need to know is queue tip, 'hg qtop', of each repo, qpush back to it. or something like: list=`hg qapplied` hg qpop -a before doing the update and then afterwards hg qpush `echo $list` -Chris. Michael On 11/04/14 15:58, Chris Hegarty wrote: Anyone using MQ for their daily development will know about this, forgetting to qpop before sync'ing up. It would be nice it get_source would pop and push patches ( only if you are using MQ ) automatically. If you do not have patch repos, then there is no change. diff --git a/get_source.sh b/get_source.sh --- a/get_source.sh +++ b/get_source.sh @@ -28,6 +28,21 @@ # Get clones of all nested repositories sh ./common/bin/hgforest.sh clone $@ || exit 1 +patchdirs=`ls -d ./.hg/patches ./*/.hg/patches ./*/*/.hg/patches \ + ./*/*/*/.hg/patches ./*/*/*/*/.hg/patches 2/dev/null` + +# Pop all patches, if any, before updating +if [ ${patchdirs} != ] ; then + echo Found queue repository, qpop. + sh ./common/bin/hgforest.sh qpop -a || exit 1 +fi + # Update all existing repositories to the latest sources -sh ./common/bin/hgforest.sh pull -u +sh ./common/bin/hgforest.sh pull -u || exit 1 +# Push all patches, if any, after updating +if [ ${patchdirs} != ] ; then + echo Found queue repository, qpush. + sh ./common/bin/hgforest.sh qpush -a +fi + -Chris.
Re: build warnings
Great. I'll close the bug I filed as a dup of this one then. Thanks Michael On 10/02/14 10:39, Chris Hegarty wrote: On 08/02/14 00:41, Mike Duigou wrote: Part of the issue seems to be that the meaning of -Wno-unused seems to have changed and/or become ineffective. It's reported that it previously hid all unused parameter warnings though it doesn't seem to on any compiler I'm currently using. I've included -Wno-unused-parameter with the changes going in JDK-8030350 so you should soon see the number of these warnings greatly diminish. Thanks Mike, -Chris. Mike On Feb 7 2014, at 02:02 , Michael McMahon michael.x.mcma...@oracle.com wrote: Just wondering is there a plan to deal with build warnings on Linux? I was experimenting with macros that could be used to deal with the -Wunused-parameter warning, but there are over 3700 occurrences and frankly it's not a very useful warning in the context of JNI formal function parameters at least. Some more useful warnings are getting lost in the noise. - Michael.
build warnings
Just wondering is there a plan to deal with build warnings on Linux? I was experimenting with macros that could be used to deal with the -Wunused-parameter warning, but there are over 3700 occurrences and frankly it's not a very useful warning in the context of JNI formal function parameters at least. Some more useful warnings are getting lost in the noise. - Michael.
RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac
Can I get the following change for jdk 8 reviewed please? It's a simple build change to enable compilation of the dummy SCTP API layer on macosx, plus the dummy implementation used on windows. The existing jdk_sctp tests cover this. http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/ Thanks, Michael.
Re: RFR: 7076487 (sctp) SCTP API classes does not exist in JDK for Mac
On 10/10/13 12:01, Alan Bateman wrote: On 10/10/2013 11:10, Michael McMahon wrote: Can I get the following change for jdk 8 reviewed please? It's a simple build change to enable compilation of the dummy SCTP API layer on macosx, plus the dummy implementation used on windows. The existing jdk_sctp tests cover this. http://cr.openjdk.java.net/~michaelm/7076487/webrev.1/ This looks okay to me (I didn't look at the Sctp* sources closely as I assume they are just a copy of the dummy implementation from src/windows). Yes. That's what I meant to say. The sources are copied from the windows tree. They just throw UnsupportedOperationException, but it means that Sctp tests can be compiled on all the reference platforms now. A minor comment (a question) is whether there is a convention to sort the values of EXFILES or not. If there is then you might have to re-order them. Right. I'll defer to someone from build on that question. Thanks Michael
Re: autoconf problem
On 16/07/13 14:42, Volker Simonis wrote: On Tue, Jul 16, 2013 at 3:02 PM, Tim Bell tim.b...@oracle.com wrote: Hi Michael: Since my latest sync of jdk8/cpu (today) I can't run configure. I get the following error: Looks like your merge touched some of the .m4 files (common/autoconf/basics.m4 and common/autoconf/build-performance.m4 to name two): Our standard guidance when this happens is to do your merge, and then run bash common/autoconf/autogen.sh to recreate the generated-configure.sh file(s). That's right but you definitely need autoconf in order to do that (as indicated in the error). Okay. Thanks! Notice that you don't necessarily need to run autoconf an the same system on which you want to run the generated configure script. If you have a shared workspace, you may for example install autoconf on a Linux machine and run autogeg.sh there. Afterwards you can run configure on any other system (i.e. Windows, Mac, Solaris..) Regards, Volker Lastly, commit them as part of your merge. If you have the closed repos, there will be two of them. Tim $ bash ./configure Warning: The generated configure file contains changes not present in the custom generated file. Cannot locate autoconf, unable to correct situation. Please install autoconf and run 'bash autogen.sh' to update the generated files. Error: Cannot continue mm72272@mm72272-ThinkPad-T420:/export/repos/jdk8-cpu$ bash ./configure --help Warning: The generated configure file contains changes not present in the custom generated file. Cannot locate autoconf, unable to correct situation. Please install autoconf and run 'bash autogen.sh' to update the generated files. Error: Cannot continue Now, I don't have autoconf installed but before I try and continue by installing it, I'd like to understand more why this is happening. Any ideas? Thanks! Michael
building jdk 8 with Java FX
I ran configure with --with-javafx-zip-dir=/java/re/javafx/jdk-cobundle/8, but I don't see any Java FX artifacts in the resulting build. What am I doing wrong? Thanks! Michael
Re: building jdk 8 with Java FX
Erik I made images, but I don't see anything with all either. Is the directory below correct? Or should I leave out the /8 ? Thanks Michael On 03/07/13 14:43, Erik Joelsson wrote: Which make target did you build? Javafx is imported in install, so you would need to run either make all or make installer /Erik On 2013-07-03 15:19, Michael McMahon wrote: I ran configure with --with-javafx-zip-dir=/java/re/javafx/jdk-cobundle/8, but I don't see any Java FX artifacts in the resulting build. What am I doing wrong? Thanks! Michael
Re: Mac build fun
Are you building on a remote filesystem like NFS? Those .DS_Store files get created by the finder and can cause problems on remote filesystems where they become visible to other software. There is a way to disable creation of .DS_Store on remote filesystems documented at http://support.apple.com/kb/HT1629 - Michael On 09/01/13 03:30, David Chase wrote: I had a failure building on a Mac (Mountain Lion, 10.8.2), both new build and old build, at this step: /Library/Java/JavaVirtualMachines/1.7.0.jdk/Contents/Home/bin/java -XX:-PrintVMOptions -XX:+UnlockDiagnosticVMOptions -XX:-LogVMOutput -Djava.awt.headless=true -Xmx512m -Xms512m -XX:PermSize=32m -XX:MaxPermSize=160m -jar /Users/dr2chase/work/jdk8tl/build/macosx-x86_64/../macosx-x86_64-debug/btjars/buildmetaindex.jar \ -o meta-index *.jar Exception in thread main java.lang.StringIndexOutOfBoundsException: String index out of range: -7 at java.lang.String.substring(String.java:1958) at build.tools.buildmetaindex.JarMetaIndex.isPrefixKnown(BuildMetaIndex.java:317) at build.tools.buildmetaindex.JarMetaIndex.getMetaIndex(BuildMetaIndex.java:233) at build.tools.buildmetaindex.BuildMetaIndex.main(BuildMetaIndex.java:97) I did some snooping, and this was the problem: firstSlashIndex = 5, name = javax/.DS_Store I.E. I must have visited a directory in the Finder or some such, and it created a .DS_Store file for me. Joy. And it turns out, using the usual command for getting rid of them, that they are getting recreated again, very quickly. find . -name .DS_Store -exec rm '{}' \; find . -name .DS_Store -print the second command would find things to print. I closed all my finder and spotlight windows, deleted again, and it stayed deleted, and my build succeeded. I think this also explains mystery failures (directory not empty) in various attempts at make clean. I plan to look into a solution that redirects .DS_Store files into a separate directory: http://asepsis.binaryage.com/#about Just a heads up. I'm not sure whether it makes sense to harden the build against this or not; it seems like a royal pain. David
Re: Review Request: Build-infra M1
On 27/03/12 16:04, Fredrik Öhrström wrote: 2012-03-27 16:39, Michael McMahon skrev: A few more things I'd like to understand are: 1) In what circumstances exactly would the configure script have to be re-run? You run configure once to setup a particular build configuration. After it is run, you only need to run make for the build configuration. Rerunning configure on an already setup build configuration is not recommended. If you want to tweak something (like different flags to compilers and such, just edit the generated spec.gmk file. 2) Can you give examples of the kind of build changes that would be needed :- - when a small'ish change is made (eg adding a couple of new source files, or renaming or removing them)? Adding/removing source files does not require a rerun of configure. - when a new component is added, with both Java and native sources in new source directories Adding an entire subdirectory of sources, more native source etc etc does not require a rerun of configure. Configure does not care about source code layout except where the repositories are. It knows how to find/download compilers/linkers/tools and create a spec.gmk file with all the necessary variables that encode this information. (For example: CC,JDK_TOPDIR,OUTPUT_ROOT,ARCH etc etc) Right. Configure doesn't need to be re-run, but presumably there is some build meta-information linking sources to target libraries etc. Say a new native library libfoo were to be added with associated C (and Java) sources. Where would this get added to the build system? - Michael
Re: Review Request: Build-infra M1
On 23/03/12 19:11, Fredrik Öhrström wrote: in increasing order of verbosity make VERBOSE= make VERBOSE=-d make VERBOSE=-d -p Thanks. It looks like the first of the three options above is most similar to the verbosity level of the old build. Is all of this documented anywhere? I think at the very least, a cheat-sheet will be needed to help people transition between the two systems, and options like the first one above will be very commonly used as part of that. Maybe an actual flag value for that setting (as opposed to the empty string) might be more meaningful. Another minor (newbie) comment, is that some of the configure flags are documented (in --help) as --with-XXX=PATH, but there are some that should be documented the same way, but aren't (--with-boot-jdk is the one many will see first) - Michael Den fredagen den 23:e mars 2012 skrev Michael McMahonmichael.x.mcma...@oracle.com mailto:michael.x.mcma...@oracle.com: Is it possible to get a more verbose style of echoing the complete compile/build commands like the old build ? - Michael On 21/03/12 14:07, Erik Joelsson wrote: As outlined in [1], the build-infra project would like to push the current work into jdk8 in order to expose it to a wider audience. The webrevs are made against the jdk8/build forest. In each repository, there are two kinds of changes: 1. Changes to old makefiles and source code to be compatible with the new build. 2. The new makefiles For corba, jaxp and jaxws, all changes of category 1 have already gone in. For langtools, we are awaiting one more change for introducing the GenerateNativeHeader annotation. For hotspot, all necessary changes have been pushed into hotspot-rt. For jdk, there are two webrevs, one with everything and one with just the category 1 changes, to make it easier to see them. Finally for the root repository there are only new files in the common subdir. root, configure script and makefiles: http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-root-new/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-root-new/ langtools, 1 new makefile: http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-langtools-new/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-langtools-new/ langtools, GenerateNativeHeader annotation (this is already going in through tools, but adding it here for reference as the jdk changes depend on it) http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-langtools-nativeheader/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-langtools-nativeheader/ corba, 1 new makefile: http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-corba-new/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-corba-new/ jaxp, 1 new makefile http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jaxp-new/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jaxp-new/ jaxws, 1 new makefile http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jaxws-new/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jaxws-new/ jdk, just the changes to old files http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jdk-other/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jdk-other/ jdk, all changes including a partial copy of the old makefiles. http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jdk-new/ http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jdk-new/ Of course, if you prefer you can look at the new makefiles directly in the build-infra/jdk8 repository forest too. These changes should not affect the old build at all. To build using the new build system, change directory to common/makefiles and: ../autoconf/configure make (make images) State of the new build (the old build should of course be unaffected): Linux 32bit: Works Linux 64bit: Works Windows 32bit: Works Windows 64bit: Works Solaris i586: Builds but launchers currently unusable Some notes: The old and new build (on linux x64) produce very close to equal results. There is a comparison script in common/bin/compareimage.sh with which this can be checked. Not all makefiles in jdk have been converted yet, for those that haven't been, a copy of the old files are used. Not all promised features in the java compilation are active and ready in this milestone. Most notably, it's still not using more than one cpu and the nifty new dependency tracking is disabled. A clean build is still pretty fast, but incremental builds aren't as good as they will be yet. On windows, only cygwin is currently supported. Now please share your feedback! /Erik [1] http://mail.openjdk.java.net/pipermail/build-infra-dev/2012-March/000571.html
RFR: 7151898: Check for LANG in Mac OS X jdk build sanity check [macosx]
Can I get the following jdk8 change reviewed please? It is a simple sanity check on Mac OS X to ensure that LANG is set in the environment. Currently, the build fails if it's not set, but the failure is quite obscure. http://cr.openjdk.java.net/~michaelm/7151898/webrev.1/ Thanks Michael.
Re: RFR: 7151898: Check for LANG in Mac OS X jdk build sanity check [macosx]
It definitely works with LANG=en_US.UTF-8 (and other en country variants). It definitely doesn't**work in the zh_CN locale, as was reported recently. The reason evidently is a dependency in the build on the output format of the date command. Which other locales it works in, I don't know. So, I guess it makes sense to standardize on a particular setting. The idea of setting it internally within the build to C sounds attractive because forcing people to use such an outdated compatibility mode locale seems wrong. I'm not sure how easy it is to set environment variables in make is (as opposed to reading/using them ). I wonder why this wasn't done before... In any case, this is a general build issue rather than being Mac specific. By the way, I did file a bug on the LANG breakage (7151897) Thanks Michael. On 15/03/12 18:57, Stuart Marks wrote: I agree, there's a larger question about LANG that needs to be asked here. From a pragmatic point of view, I ran into a build issue that boiled down to the absence of LANG in my environment in certain cases (ssh vs VNC). README-builds.html recommends setting LANG=C for OpenSolaris and all the Linux-flavored build environments. So, it makes sense for LANG=C to go into the Mac build instructions as well. (README-builds.html should get updated too.) Unlike on Linux, the consequences of LANG being unset on Mac are that the build breaks, so Michael has added this check in the Makefile to prevent this obvious error. I believe he was also going to file a bug on the Mac build breaking when LANG is unset. Now, the larger questions are, what are the valid values of LANG, and if all the Unix-flavored build instructions recommend setting LANG=C, why not just have the makefiles or build scripts set this value and be done with it? I don't know, and I don't have the expertise in the build system to know how other LANG settings would affect the build. Perhaps somebody else on build-dev knows. Meanwhile, we're patching things up this way, even though it makes things a bit messier. s'marks On 3/15/12 10:59 AM, Mike Swingler wrote: What other values are valid? UTF8? Why would a builder ever want to change the lang? I think the build script should define it and use it for it's own private purposes (allowing it to be overridden) if there is no compelling reason for an ordinary user to know/care what lang is. I'd prefer not to clutter up the build instructions unless you _really_ have to pass some value that is machine-specific (like the location of the bootstrap JDK). Even then, on the Mac, I think the build scripts should call /usr/libexec/java_home -v 1.7+ on their own, and only balk if there is not sufficient OpenJDK installed. Regards, Mike Swingler Apple Inc. On Mar 15, 2012, at 9:43 AM, Stuart Marks wrote: Looks good to me too. I've updated the Mac build instructions on the wiki to state that LANG should be set. s'marks On 3/15/12 9:30 AM, Kelly O'Hair wrote: Looks fine to me. -kto On Mar 15, 2012, at 9:18 AM, Michael McMahon wrote: Can I get the following jdk8 change reviewed please? It is a simple sanity check on Mac OS X to ensure that LANG is set in the environment. Currently, the build fails if it's not set, but the failure is quite obscure. http://cr.openjdk.java.net/~michaelm/7151898/webrev.1/ Thanks Michael.
Re: Question about BUILD_HEADLESS and HEADLESS
The stub library is headless/libmawt.so*. Right? It's being built on Mac OS as well, but currently being put in a different location to the normal one. It also seems like it is not being used because if you rename or delete it, there isn't any change in behavior, which sounds like a bug. So, what exactly is the function of the stub library? Is it to allow some functions of awt to work that don't require a real display? In any case, I don't see why Mac should be different from Solaris or Linux in this respect. - Michael. * In 8 it has a different name, but it'll be built on Mac as well in 8. On 14/02/12 17:27, Phil Race wrote: On windows headless is simply a state. But Solaris/Linux have true headless builds where there are headless (stub) versions of UI libraries. And I think that headless is a valid JCK mode .. you can pass JCK in headless on platforms that don't support UI. But I'd check the JCK guys on that one, don't take my word for it. -phk. On 2/14/2012 6:49 AM, Michael McMahon wrote: On 14/02/12 13:47, Fredrik Öhrström wrote: Is the use of HEADLESS in gcc.make (linux and bsd) an archaeological remnant and should be removed? (No source in the hotspot repo looks a the HEADLESS define.) Is there any reason to not build a headless version of awt? (ie modify BUILD_HEADLESS to not be defined.) It seeems like headless is not currently built on bsd/macosx/windows, but it is built on solaris and linux? //Fredrik Fredrik, It's being built on macosx as well. - Michael.
Re: Question about BUILD_HEADLESS and HEADLESS
On 14/02/12 13:47, Fredrik Öhrström wrote: Is the use of HEADLESS in gcc.make (linux and bsd) an archaeological remnant and should be removed? (No source in the hotspot repo looks a the HEADLESS define.) Is there any reason to not build a headless version of awt? (ie modify BUILD_HEADLESS to not be defined.) It seeems like headless is not currently built on bsd/macosx/windows, but it is built on solaris and linux? //Fredrik Fredrik, It's being built on macosx as well. - Michael.
Re: RFR 7142950: jdk7u cannot bootstrap Mac OS build [macosx]
Hi, http://cr.openjdk.java.net/~michaelm/7142950/jdk/webrev.2/ This is an updated webrev based on the contribution/suggestion from Scott Kovatch. It changes the build image directories on Mac, to have the same format/directory structure as the other platforms (ie. it removes the Contents/Home stuff). That directory structure required by Mac bundles is now generated in specific bundle directories and these are used by the install. A consequence of this change is that anyone who has adjusted scripts that used the built j2sdk-image, or j2re-image to know about the mac specific paths will have to change that back again. This version of the change is almost agnostic on the os.arch setting. The only dependency is on 'src/macosx/bin/amd64/jvm.cfg'. That will have to be renamed to x86_64/jvm.cfg when 'os.arch' is changed. No other change is required. With this change, jdk only, and incremental builds should work again. If you are using the previous output from a control build as bootstrap or import jdk for a subsequent jdk only build, then it is possible you could run into the build problem described in 6967648. The workaround is to rename the top-level build directory to some other name before doing the jdk build. Thanks, Michael On 06/02/12 23:21, Scott Kovatch wrote: On Feb 6, 2012, at 2:31 PM, Michael McMahon wrote: There are a few problems with the Mac build at the moment. 1. If SKIP_BOOT_CYCLE=false then the build fails, due to two problems:- 1) top level Makefile doesn't know about Mac OS image directory structure 2) it also fails due to problem 2. below 2. General bootstrapping problem. The build currently cannot use itself as the bootstrap JDK due to an assumption in the framework classes that os.arch is x86_64, whereas we currently set it to amd64. The change is to fix that code to expect amd64. There is a related question then about what the correct value for os.arch should be. I'd like to raise this in a separate discussion and hopefully fix these build problems independently of that. I was going to propose that we fix the image directory issues by building into the image directories the same way on all platforms and have the mac build copy the image directories into j2sdk-bundle. (7133768) I'm pretty sure that even with this change you can't build rt.jar because the jar tool gets moved out from underneath the running build when the bundle is constructed. I don't think your changes would interfere with my proposed fix for 7133768, though. And, the other changes for generating the folder name dynamically are very much welcome. -- Scott K. Scott Kovatch scott.kova...@oracle.com Santa Clara/Pleasanton, CA
RFR 7142950: jdk7u cannot bootstrap Mac OS build [macosx]
There are a few problems with the Mac build at the moment. 1. If SKIP_BOOT_CYCLE=false then the build fails, due to two problems:- 1) top level Makefile doesn't know about Mac OS image directory structure 2) it also fails due to problem 2. below 2. General bootstrapping problem. The build currently cannot use itself as the bootstrap JDK due to an assumption in the framework classes that os.arch is x86_64, whereas we currently set it to amd64. The change is to fix that code to expect amd64. There is a related question then about what the correct value for os.arch should be. I'd like to raise this in a separate discussion and hopefully fix these build problems independently of that. 3. jvm.cfg is currently being taken from the solaris src tree even though a macos version of the file is there. Fix that problem too. The macos source jvm.cfg is changed to be the same as the solaris/amd64 version (ie. the one that was being used, and which makes -client UNKNOWN) The following webrevs address these issues. Top level repo -- http://cr.openjdk.java.net/~michaelm/7142950/top/webrev.1/ JDK repo http://cr.openjdk.java.net/~michaelm/7142950/jdk/webrev.1/