Kelly, that's correct, you did. It was my oversight that you were not listed. Dan - thanks again.

Brace yourselves for the rest of the MinGW/MSYS changes in bug #7152336 - hotspot got off relatively easily.

Tim

On 08/03/12 18:34, Daniel D. Daugherty wrote:
On 8/3/12 6:08 PM, Kelly O'Hair wrote:
I thought you would have done a hg commit --user tbell so it belongs to Tim?

Yes, the changeset belongs to Tim, but I also listed
him as a reviewer since he reviewed the code as well.


I did a review too, but no big deal.

Sorry, I wasn't aware of that. I'll fix that in a minute.

Dan


-kto

On Aug 3, 2012, at 4:21 PM, Daniel D. Daugherty wrote:

Added you to the reviewer list. You're also the "user" for the
changeset. The job is in the JPRT-hotspotwest queue heading to
RT_Baseline.

Dan


On 8/3/12 5:08 PM, Tim Bell wrote:
On 08/03/12 15:09, Daniel D. Daugherty wrote:
Gotta put 'tbell' in there some where...

Can I add you as a reviewer?
Oh - that's right.  Sure, add me as a reviewer.

Tim

Dan


On 8/3/12 3:58 PM, Tim Bell wrote:
Thanks, Dan

How about this for a commit message:

7181175: Enable builds on Windows with MinGW/MSYS
Summary: This fix is the minimum number of Makefile changes to enable building HotSpot with MinGW/MSYS
Contributed-by: [email protected]
Reviewed-by: jcoomes, dcubed,

Tim

On 08/03/12 13:45, Daniel D. Daugherty wrote:
Thumbs up on this version.

Do you have a commit message ready for this patch?

Dan


On 8/3/12 1:26 PM, Tim Bell wrote:
On 08/02/12 14:20, Daniel D. Daugherty wrote:
http://cr.openjdk.java.net/~tbell/7181175/webrev.01/
Thanks for the review, Dan.

make/windows/makefiles/defs.make
    No comments.

make/windows/makefiles/rules.make
    lines 28-33: Might want to comment on why these paths still
        use backslash instead of forward slash.
OK - done.

make/windows/makefiles/sa.make
Oops, my error. Thanks for catching these, Dan. This goes to show how long these MinGW/MSYS changes have been in play...

lines 88, 90:
These lines drop the "/YX" option instead of changing it to "-YX".
Fixed.

line 97:
This line adds back the "-Z" option and drops the "/YX" option
        instead of changing it to "-YX".
I added back the -YX and removed -Z.

        The "-Z" option is conditionally added on line 99.
Ah - I see it now.  Good.

line 106:
        This line adds back the "-map -debug" options.

The "-map" and "-debug" options are conditionally added on line 108.
Fixed.

make/windows/makefiles/shared.make
    line 43: Might want to comment on why these paths still
        use backslash instead of forward slash.
OK - done.


I submittted a new JPRT test job with these changes (2012-08-03-171845.tbell.hotspot) that completed successfully.

New webrev here:

http://cr.openjdk.java.net/~tbell/7181175/webrev.02/

Or if you prefer, here are the diffs relative to my previous webrev:

+SA_CFLAGS = -nologo $(MS_RUNTIME_OPTION) -W3 $(GX_OPTION) -Od -D "WIN32" -D "WIN64" -D "_WINDOWS" -D "_DEBUG" -D "_CONSOLE" -D "_MBCS" -YX -FD -c
!elseif "$(BUILDARCH)" == "amd64"
-SA_CFLAGS = -nologo $(MS_RUNTIME_OPTION) -W3 $(GX_OPTION) -Od -D "WIN32" -D "WIN64" -D "_WINDOWS" -D "_DEBUG" -D "_CONSOLE" -D "_MBCS" -FD -c +SA_CFLAGS = -nologo $(MS_RUNTIME_OPTION) -W3 $(GX_OPTION) -Od -D "WIN32" -D "WIN64" -D "_WINDOWS" -D "_DEBUG" -D "_CONSOLE" -D "_MBCS" -YX -FD -c
!if "$(COMPILER_NAME)" == "VS2005"
# On amd64, VS2005 compiler requires bufferoverflowU.lib on the link command line, # otherwise we get missing __security_check_cookie externals at link time.
SA_LD_FLAGS = bufferoverflowU.lib
!endif
!else
-SA_CFLAGS = -nologo $(MS_RUNTIME_OPTION) -W3 -Gm $(GX_OPTION) -ZI -Od -D "WIN32" -D "_WINDOWS" -D "_DEBUG" -D "_CONSOLE" -D "_MBCS" -FD -GZ -c +SA_CFLAGS = -nologo $(MS_RUNTIME_OPTION) -W3 -Gm $(GX_OPTION) -Od -D "WIN32" -D "_WINDOWS" -D "_DEBUG" -D "_CONSOLE" -D "_MBCS" -YX -FD -GZ -c
!if "$(ENABLE_FULL_DEBUG_SYMBOLS)" == "1"
SA_CFLAGS = $(SA_CFLAGS) -ZI
!endif
@@ -103,7 +103,7 @@
SA_LD_FLAGS = -manifest $(SA_LD_FLAGS)
!endif
SASRCFILE = $(AGENT_DIR)/src/os/win32/windbg/sawindbg.cpp
-SA_LFLAGS = $(SA_LD_FLAGS) -nologo -subsystem:console -map -debug -machine:$(MACHINE) +SA_LFLAGS = $(SA_LD_FLAGS) -nologo -subsystem:console -machine:$(MACHINE)
!if "$(ENABLE_FULL_DEBUG_SYMBOLS)" == "1"
SA_LFLAGS = $(SA_LFLAGS) -map -debug
!endif
--- /x/jdk8/7181175/hotspot.00/make/windows/makefiles/shared.make 2012-07-18 12:11:25.954735638 -0700 +++ /x/jdk8/7181175/hotspot/make/windows/makefiles/shared.make 2012-08-03 10:15:53.249176409 -0700
@@ -36,6 +36,7 @@


!ifdef SUBDIRS
+# \ is used below because $(MAKE) is nmake here, which expects Windows paths
$(SUBDIRS): FORCE
        @if not exist $@ mkdir $@
        @if not exist $@/local.make echo # Empty>  $@/local.make







Reply via email to