On 06/29/12 09:35, Kelly O'Hair wrote:
Looks great to me.   It's fantastic to see this happening.

Just a few minor things:


1. For
  116 DOSPATH_CMD:=$(shell cd $(JDK_TOPDIR) 2>  $(DEV_NULL)&&  
pwd)/make/tools/mingw_build_scripts/dospath.sh
since dospath.sh cannot have execute permission in the repository, does this still work?

Good catch. This works because the definition of DOSPATH_CMD is only used in the 'define FullPath', where it is surrounded by another $(shell ...)

2. In Defs-utils:

  178     ifdef USING_MINGW
  179       ECHO   = $(UTILS_COMMAND_PATH)echo -e
  180       AR     = $(UTILS_DEVTOOL_PATH)ar
  181       ZIPEXE = $(UTILS_DEVTOOL_PATH)zip
  182       UNZIP  = $(UTILS_DEVTOOL_PATH)unzip
  183       NAWK   = $(UNIXCOMMAND_PATH)awk
  184     else
  185       ZIPEXE = $(UTILS_DEVTOOL_PATH)zip
  186       UNZIP  = $(UTILS_DEVTOOL_PATH)unzip
  187       NAWK   = $(UNIXCOMMAND_PATH)awk
  188     endif

seems like that can be simplified:

ZIPEXE = $(UTILS_DEVTOOL_PATH)zip
UNZIP = $(UTILS_DEVTOOL_PATH)unzip
NAWK = $(UNIXCOMMAND_PATH)awk
ifdef USING_MINGW
ECHO = $(UTILS_COMMAND_PATH)echo -e
AR = $(UTILS_DEVTOOL_PATH)ar
fi
  ??

Done.

3. I hate to see the for loop duplicated in Sanity.gmk, but this logic isn't 
long for this world, so I'd leave it.

OK.

4. The fixpath.pl and other make/scripts files might be missing a trailing newline, not sure.

I double-checked and they look OK. After I create a changeset, I will run jcheck to be sure.

But overall, I see no reason to block this work from being integrated.
As long as the current RE MKS builds are ok.

JPRT build jobs are going through OK.

Thanks for the review-
Tim

-kto

On Jun 27, 2012, at 9:45 AM, Tim Bell wrote:

I went over the open changes again and removed some debug code left in
by mistake, updated copyright dates and fixed a few typos.  Here is an
updated webrev:

http://cr.openjdk.java.net/~tbell/7152336/webrev.01/ <http://cr.openjdk.java.net/%7Etbell/7152336/webrev.01/>

These changes are building in both 32 and 64 bit mode on my Windows 7 system.

jtreg test runs of the '-automatic -noshell' tests under test/java/lang test/java/math test/java/util ran as expected (2 failures, both known bugs).

Thanks in advance for your review and feedback - I'd like to get these changes in soon.

Tim Bell


On 06/13/12 22:31, Tim Bell wrote:
Hello everyone-

Kelly asked me to pick up on bug #/7152336 "//Enable builds on Windows with MinGW/MSYS"/, and this email thread:

http://mail.openjdk.java.net/pipermail/build-dev/2012-April/thread.html#6083

As David pointed out, we will need at least one other bug # for the hotspot changes. That said, this is enough to get me started.

Hi Volker:

I have applied the patches originally from your posting. Many thanks for that: http://cr.openjdk.java.net/~simonis/MinGW_MSYS.v1/ <http://cr.openjdk.java.net/%7Esimonis/MinGW_MSYS.v1/>

With a few modifications (keep cpio for non MinGW/Msys builds, keep MKS as an option), the proposed changes are visible here for review:

http://cr.openjdk.java.net/~tbell/7152336/webrev.00/ <http://cr.openjdk.java.net/%7Etbell/7152336/webrev.00/>

For reference, my test build log is visible here:

http://cr.openjdk.java.net/~tbell/7152336/webrev.00/full_control_build_no_docs.log <http://cr.openjdk.java.net/%7Etbell/7152336/webrev.00/full_control_build_no_docs.log>

Additional test builds on JPRT (our internal build apparatus) verified that I didn't regress the existing build.

Abbreviated jtreg [1] testing on this build was successful:

$ /d/tools/jdk8/7152336/windows-i586/bin/java -jar /d/tools/jtreg-internal/jtreg/lib/jtreg.jar -automatic -noshell test/java/lang test/java/math test/java/util
Directory "JTreport" not found: creating
Directory "JTwork" not found: creating
Directory "JTwork\scratch" not found: creating
Test results: passed: 698; failed: 1; error: 5
Report written to D:\tools\jdk8\7152336\jdk\JTreport\html\report.html
Results written to D:\tools\jdk8\7152336\jdk\JTwork
Error: Some tests failed or other problems occurred.

The failing test (java/lang/Math/WorstCaseTests.java) is due to a known regression: 7174532 <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7174532> "jdk/test/java/lang/Math/WorstCaseTests.java failing on x86"

The 5 error tests are all ignored until bug xxxxxxx (for some value of x) is resolved.

Thanks in advance for your review and feedback -

Tim Bell

[1] http://openjdk.java.net/projects/code-tools/




Reply via email to