vs integration: fix default path to clang-cl

[zturner]

I’m pretty sure I tested this code path, was it wrong? Also I don’t think
we should be explicitly adding the \ after the $(LLVMInstallDir), if you
look through the way every other path in a VS project works, they assume
the \ is in the registry path, so if it’s possible I think we should follow
the same pattern.

[hans]

In D50335#1189413, @zturner wrote:

I’m pretty sure I tested this code path, was it wrong?

It didn't work when I tried it.

Also I don’t think
we should be explicitly adding the \ after the $(LLVMInstallDir), if you
look through the way every other path in a VS project works, they assume
the \ is in the registry path, so if it’s possible I think we should follow
the same pattern.

I don't think we should change the path in the registry (for example that would make the vsix not work with old installs), but we can make LLVMInstallDir add a backslash on top of what it gets from the registry.

[hans]

Make LLVMInstallDir include a trailing backslash.

[zturner]

I’m OOO this week so I can’t easily check this stuff. Sorry for all the
questions. The registry key that we create, is it the (Default) value or is
it a string value named LLVM?

[hans]

In D50335#1189424, @zturner wrote:

I’m OOO this week so I can’t easily check this stuff. Sorry for all the
questions. The registry key that we create, is it the (Default) value or is
it a string value named LLVM?

If I understand it correctly, it's a string value named LLVM.

[zturner]

In that case I think the first version was correct. LLVM@LLVM means “the
value LLVM under the key LLVM”. LLVM\LLVM means “the default value under
the key LLVM which itself is under the key LLVM”

[hans]

In D50335#1189457, @zturner wrote:

In that case I think the first version was correct. LLVM@LLVM means “the
value LLVM under the key LLVM”. LLVM\LLVM means “the default value under
the key LLVM which itself is under the key LLVM”

Arrr, no I got it backwards. It looks like this:

[zturner]

If you go to vs settings you can go to project -> build & run -> output
level and set it to Diagnostic. Then Ctrl+F for LLVMInstallDir and you
should see the value. Try this with @ and with \ and see if they’re
different.

Btw, unfortunately i had a few more changes to the csproj file that I may
have forgotten to upstream before i left. IIRC, they were changing the
author name to LLVMExtensions , deleting the part in the csproj about
Brutal Strong name signer, setting the key file in project settings, and
fixing the path to sn.exe.

Edit: just saw your update. That is such weird installer behavior. What
goes in the LLVM key above that? Maybe the installer behavior changed and
we need to support both?

[hans]

In D50335#1189459, @zturner wrote:

Btw, unfortunately i had a few more changes to the csproj file that I may
have forgotten to upstream before i left. IIRC, they were changing the
author name to LLVMExtensions , deleting the part in the csproj about
Brutal Strong name signer, setting the key file in project settings, and
fixing the path to sn.exe.

Ouch, it's not so great that the published extension was built from source that's not checked in. But I guess publishing an updated version can wait until you're back.

In D50335#1189459, @zturner wrote:

Edit: just saw your update. That is such weird installer behavior. What
goes in the LLVM key above that? Maybe the installer behavior changed and
we need to support both?

I tried an old installer (3.8.0) and it sets the same registry keys.

I don't know if it's that weird. I think cmake/cpack just figures the package vendor is LLVM and the product is also called LLVM, and that's how it ended up there.

tools/msbuild/LLVM.Cpp.Common.props

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <PropertyGroup>
     <!-- Short names for platform toolsets (added to project name in Solution Explorer) -->
     <_PlatformToolsetShortNameFor_llvm>LLVM</_PlatformToolsetShortNameFor_llvm>
     <_PlatformToolsetFriendlyNameFor_llvm>LLVM</_PlatformToolsetFriendlyNameFor_llvm>
   </PropertyGroup>
 
   <!-- Find an installed LLVM and set up our paths. -->
   <PropertyGroup>
-    <LLVMInstallDir>$(Registry:HKEY_LOCAL_MACHINE\SOFTWARE\LLVM@LLVM)</LLVMInstallDir>
-    <LLVMInstallDir Condition="'$(LLVMInstallDir)' == ''">$(Registry:HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\LLVM@LLVM)</LLVMInstallDir>
-    <ClangClExecutable>$(LLVMInstallDir)bin\clang-cl.exe</ClangClExecutable>
+    <LLVMInstallDir>$(Registry:HKEY_LOCAL_MACHINE\SOFTWARE\LLVM\LLVM)</LLVMInstallDir>
+    <LLVMInstallDir Condition="'$(LLVMInstallDir)' == ''">$(Registry:HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\LLVM\LLVM)</LLVMInstallDir>
+    <ClangClExecutable>$(LLVMInstallDir)\bin\clang-cl.exe</ClangClExecutable>
   </PropertyGroup>
 
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.WindowsSDK.props" Condition="Exists('$(VCTargetsPath)\Microsoft.Cpp.WindowsSDK.props')"/>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.Common.props" />
 
   <PropertyGroup>
     <!-- Set some paths (such as include paths) that are common to all platforms.  This is the same as what
          the default paths for cl will use.

tools/msbuild/llvm-general.xml

<Rule
   <Rule.DataSource>
     <DataSource Persistence="ProjectFile" Label="Configuration" />
   </Rule.DataSource>
 
   <StringProperty Name="ClangClExecutable"
                   DisplayName="Clang-CL Executable"
                   Description="Specifies the path to clang-cl.exe."
                   Category="General"
-                  Default="$(LLVMInstallDir)bin\clang-cl.exe"
+                  Default="$(LLVMInstallDir)\bin\clang-cl.exe"
                   Subtype="file">
     <StringProperty.DataSource>
       <DataSource Persistence="ProjectFile" Label="" />
     </StringProperty.DataSource>
   </StringProperty>
 </Rule>