[PATCH] Enhance jaotc to automatically find VS2017+ linker

[Vladimir Kozlov]

+1

Vladimir

On 2/7/19 10:05 AM, Igor Veresov wrote:
> Ok, this look good.
> 
> igor
> 
> 
> 
>> On Feb 4, 2019, at 4:02 PM, Andrew Luo <andrewluotechnologies at outlook.com <mailto:andrewluotechnologies at outlook.com>> 
>> wrote:
>>
>> Sounds good.  Attached.
>> Changes:
>> 1.Continue with finding other versions of link.exe if finding a VS2017+ version fails (exception is printed to stderr 
>> however)
>> 2.Check if vswhere.exe exists, returns null if not (instead of throwing an exception previously – now it will silently 
>> continue with searching for other linkers)
>> 3.Pass -latest to vswhere.
>> Thanks,
>> -Andrew
>> *From:*Igor Veresov <igor.veresov at oracle.com <mailto:igor.veresov at oracle.com>>
>> *Sent:*Monday, February 4, 2019 2:57 PM
>> *To:*Andrew Luo <andrewluotechnologies at outlook.com <mailto:andrewluotechnologies at outlook.com>>
>> *Cc:*Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>; 
>> hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>
>> *Subject:*Re: [PATCH] Enhance jaotc to automatically find VS2017+ linker
>> Just send another diff. I’ve already created an internal Graal PR for this.
>> igor
>>
>>
>>
>>
>>
>>     On Feb 4, 2019, at 2:22 PM, Andrew Luo <andrewluotechnologies at outlook.com
>>     <mailto:andrewluotechnologies at outlook.com>> wrote:
>>     Makes sense.
>>     Ah, I forgot to add a Files.exists(vswhere) check and just return null if it doesn’t exist… But besides that, I
>>     suppose we can just try/catch the exceptions anyways so we can try other versions as well in case finding the
>>     VS2017+ linker fails for whatever reason.  I didn’t see any other similar error handling logic in the code, so I
>>     wasn’t sure (and didn’t want to eat the exception in case someone wanted to figure out why VS2017 isn’t being
>>     chosen), but it appears in other classes we’re just printing the stack trace to stderr…
>>     Right, -latest probably should be passed… It doesn’t make a difference now, but when VS2019 comes out it will matter.J
>>     I’ll make these changes and send out another diff (or perhaps just a pull request on Github).
>>     Thanks,
>>     -Andrew
>>     *From:*Igor Veresov <igor.veresov at oracle.com <mailto:igor.veresov at oracle.com>>
>>     *Sent:*Monday, February 4, 2019 12:54 PM
>>     *To:*Andrew Luo <andrewluotechnologies at outlook.com <mailto:andrewluotechnologies at outlook.com>>
>>     *Cc:*Vladimir Kozlov <vladimir.kozlov at oracle.com
>>     <mailto:vladimir.kozlov at oracle.com>>;hotspot-compiler-dev at openjdk.java.net
>>     <mailto:hotspot-compiler-dev at openjdk.java.net>
>>     *Subject:*Re: [PATCH] Enhance jaotc to automatically find VS2017+ linker
>>     Perhaps, we should catch all the exceptions that are thrown by getVC141AndNewerLinker() and try other methods of
>>     getting to a linker if it fails?
>>     Another issue raised by Bob is that perhaps we should be passing “-latest” to vswhere for the case when multiple
>>     VS version are installed. What do you think?
>>     igor
>>
>>
>>
>>
>>
>>
>>
>>         On Feb 4, 2019, at 12:00 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
>>         wrote:
>>         Hi Andrew,
>>
>>         I looked on changes nire and I have question about throwing next and following errors in getVC141AndNewerLinker():
>>
>>         +    private static Path getVC141AndNewerLinker() throws Exception {
>>         +        String programFilesX86 = System.getenv("ProgramFiles(x86)");
>>         +        if (programFilesX86 == null) {
>>         +            throw new InternalError("Could not read the ProgramFiles(x86) environment variable");
>>
>>         Method getVC141AndNewerLinker() is called before any other paths are checked. If this variable is not set or
>>         other checks in getVC141AndNewerLinker() failed, caller getWindowsLinkPath() exits before following code is
>>         executed and other paths are checked.
>>
>>         I don't think it is correct if old VS is installed instead of VS2017+. What dio you think. Can you verify that
>>         it works in all cases?
>>
>>         Thanks,
>>         Vladimir
>>
>>         On 1/29/19 2:33 PM, Vladimir Kozlov wrote:
>>
>>
>>             Looks good.
>>             Thanks,
>>             Vladimir
>>             On 1/25/19 3:56 PM, Andrew Luo wrote:
>>
>>
>>                 Minor public -> private visibility fix.  Just noticed right after I sent it out…
>>
>>                 Thanks,
>>
>>                 -Andrew
>>
>>                 *From:* hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net
>>                 <mailto:hotspot-compiler-dev-bounces at openjdk.java.net>> *On Behalf Of *Andrew Luo
>>                 *Sent:* Friday, January 25, 2019 3:55 PM
>>                 *To:*hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>
>>                 *Subject:* [PATCH] Enhance jaotc to automatically find VS2017+ linker
>>
>>                 See attached patch.  Any feedback is welcome.
>>
>>                 Tested on a system with only VS2017 installed, just ran jaotc with a simple class file, and got the
>>                 expected .dll output with no errors…
>>
>>                 Thanks,
>>
>>                 -Andrew
>>
>> <jaotcdiff3.txt>
>