Integrated: 8284890: Support for Do not fragment IP socket options

2022-04-26 Thread Michael McMahon
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]

2022-04-20 Thread Michael McMahon
> 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]

2022-04-20 Thread Michael McMahon
> 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]

2022-04-20 Thread Michael McMahon
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]

2022-04-19 Thread Michael McMahon
> 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]

2022-04-19 Thread Michael McMahon
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]

2022-04-19 Thread Michael McMahon
> 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]

2022-04-19 Thread Michael McMahon
> 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]

2022-04-19 Thread Michael McMahon
> 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]

2022-04-15 Thread Michael McMahon
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]

2022-04-15 Thread Michael McMahon
> 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

2022-04-15 Thread Michael McMahon
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

2022-04-15 Thread Michael McMahon
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]

2021-11-26 Thread Michael McMahon
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]

2021-10-18 Thread Michael McMahon
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]

2021-10-18 Thread Michael McMahon
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]

2021-09-21 Thread Michael McMahon
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]

2021-09-21 Thread Michael McMahon
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]

2021-09-21 Thread Michael McMahon
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]

2021-09-15 Thread Michael McMahon
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

2021-09-15 Thread Michael McMahon
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]

2020-10-06 Thread Michael McMahon
> 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]

2020-10-06 Thread Michael McMahon
> 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]

2020-10-05 Thread Michael McMahon
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]

2020-10-04 Thread Michael McMahon
> 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]

2020-10-04 Thread Michael McMahon
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]

2020-10-02 Thread Michael McMahon
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]

2020-10-02 Thread Michael McMahon
> 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]

2020-10-02 Thread Michael McMahon
> 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

2018-09-04 Thread Michael McMahon
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

2017-01-24 Thread Michael McMahon

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

2014-04-11 Thread Michael McMahon

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

2014-04-11 Thread Michael McMahon

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

2014-02-10 Thread Michael McMahon

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

2014-02-07 Thread Michael McMahon

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

2013-10-10 Thread Michael McMahon

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

2013-10-10 Thread Michael McMahon

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

2013-07-16 Thread Michael McMahon

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

2013-07-03 Thread Michael McMahon
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

2013-07-03 Thread Michael McMahon

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

2013-01-09 Thread Michael McMahon
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

2012-03-27 Thread Michael McMahon

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

2012-03-26 Thread Michael McMahon

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]

2012-03-15 Thread Michael McMahon

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]

2012-03-15 Thread Michael McMahon

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

2012-02-15 Thread Michael McMahon
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

2012-02-14 Thread Michael McMahon

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]

2012-02-09 Thread Michael McMahon

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]

2012-02-08 Thread Michael McMahon

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/