Hi Erik, Thanks for sponsoring this. Looking at the patch, I noticed I left in an extra "echo "$VS_INSTALL_DIR/$f"". This probably could be removed.
Thanks, -Andrew -----Original Message----- From: Erik Joelsson <[email protected]> Sent: Thursday, January 3, 2019 1:22 AM To: Andrew Luo <[email protected]>; [email protected] Subject: RFR: JDK-8216048: Fix devkit creation in WSL Hello Andrew, Thanks for fixing this. The patch looks good. I have verified that it produces the same output in WSL and Cygwin. Just like in Cygwin, I did not need to be in a Tools Command Prompt to run it (because VS was installed in the default location). Regarding the hard coded dir version strings, I'm in the process of updating our kits to the latest VS2017 update, and in that patch, I chose to dynamically resolve those versions instead of putting them in the script. I would expect that to be ready for push sometime this spring. The older VS2013 and VS2015 scripts are indeed best left alone. We should probably delete them from the repo at this point. I've created https://bugs.openjdk.java.net/browse/JDK-8216048 for this and uploaded the patch as a webrev here: http://cr.openjdk.java.net/~erikj/8216048/webrev.01/ In addition to your changes, I bumped the copyright year and fixed indentation on one line. /Erik On 2019-01-03 05:06, Andrew Luo wrote: > Hi, > > I noticed that we have a few scripts checked in for creating devkits on > Windows. However they only work in Cygwin. Since we recently added support > for WSL, it seems like it would be nice to be able to use those scripts on > WSL as well... > > Anyways, patch is attached. This one you do have to open a VS2017 Native > Tools Command Prompt but I think that's the same as before (in Cygwin). I > tested the script in WSL and everything seems to work (I did have to change > some of the versions at the top since the version of VS2017 I have installed > is much newer, but I reverted those changes before generating this diff), but > will need someone to check Cygwin as well. > > I didn't update the VS2013 and VS2015 scripts. As far as I'm aware we > don't (officially) support those compilers anymore: > https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms > > Thanks, > > -Andrew
