Review request (M): Updates to Visual Studio project creation and development launcher
kelly.ohair at oracle.com
Mon Dec 13 09:14:36 PST 2010
Makefiles are kind of hard to review, so this is the best I can do...
Just curious, have you tried building this with CYGWIN?
Not a big deal, and not related to your change, but as we move
forward, I'd like to see MKS_HOME
changed to something more generic, like TOOLS_BINDIR or something
It's not necessarily MKS and it is not a product home directory
either. But I know this is historic stuff.
The NUL used here is a Windows/MKS thing, and is considered a
filename in CYGWIN.
So this needs to be put into a variable or something, using NUL
when MKS is used and /dev/null when
CYGWIN is used. Otherwise builds with CYGWIN will leave NUL files
scattered all over the place.
(Inside the bat files, I think NUL is the right thing to use, but
I'm not a bat file expert).
The name is hotspot.exe? Is this the temp launcher that does not
ship in the jdk image?
Have we always had an exe name with "hotspot" in it? Not sure I
know that it is wrong, just seems strange.
The secure coding class (which I very recently and painfully
completed) seemed to imply that a
printf like this is insecure. Not sure I agree, but I suspect we
should change to use a format string
before some tool complains about it. I'll also bet that this is
done all over the place already by everybody. :^(
Of course the getenv() is also considered a problem too. But again,
not your issue here, but it might
be a good idea in the future for the hotspot team to encapsulate
use of getenv so there is one place to say
'@IgnoreThisGetenv call". ;^)
Use of pwd with CYGWIN could get you a path like /cygdrive/c/
instead of C:/ (from MKS).
In this case I think it's ok, but pwd can be a problem in Makefiles
when we allow the use of
either MKS or CYGWIN for builds.
More printf's without format strings.
I'm confused by this code. So the env variable "JAVA_HOME" is not
used anymore and it reads it
from a file now? And then you set the JAVA_HOME environment
variable with the value?
Why is this being done?
Maybe I don't care because the gamma launcher isn't shipped as part
of the jdk (right?).
But this seems really unusual to me.
Has ALT_JAVA_HOME always been around? Is it just for this gamma
I've never seen any documentation on a ALT_JAVA_HOME variable
spelling, just curious if it
creeps into the jdk product at all.
On the java code, I didn't review it in detail but appreciate the
change from wildcard imports to explicit
imports, always seems so much more accurate that way. ;^)
On Dec 13, 2010, at 5:06 AM, Staffan Larsen wrote:
> Webrev: http://cr.openjdk.java.net/~sla/7006354/webrev.00/
> This update makes it easier to work with Visual Studio projects.
> After this update you can create a VS project by:
> 1) Run “vsvars32.cmd”
> 2) Run “make\windows\create.bat <path to JDK to use for running
> From inside Visual Studio you can open the generated vsbuild
> \jvm.vcproj. Edit the properties to set the debugging command to “$
> (TargetDir)\hotspot.exe”. Compile and run/debug from inside VS.
> · Changes have only been tested on VS2008 and VS2010.
> · The script do not generate VS2010 project, but the
> conversion wizard in VS2010 can read the generated VS2008 project
> · Only 32-bit targets work.
> This change includes the proposed patch in CR7002870.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-dev