I thought you would have done a hg commit --user tbell so it belongs to Tim?
I did a review too, but no big deal. -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 >>>>>>> >>>>>> >>>>>> >>>> >>>> >> >> >>
