-
Notifications
You must be signed in to change notification settings - Fork 804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update vsix generation to use latest tools. #1903
Conversation
@brettfo is it your issue or infrastructure issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
@heejaechang Mine to update the tooling in this repo and ideally infrastructure's issue to do the insertion, but since there are other moving parts I'll handle this one end-to-end. |
@@ -389,7 +389,8 @@ if exist "%ProgramFiles(x86)%\Microsoft Visual Studio 12.0\common7\ide\devenv.ex | |||
if exist "%ProgramFiles%\Microsoft Visual Studio 12.0\common7\ide\devenv.exe" set VisualStudioVersion=12.0 | |||
|
|||
:vsversionset | |||
if '%VisualStudioVersion%' == '' echo Error: Could not find an installation of Visual Studio && goto :failure | |||
if "%VisualStudioVersion%" == "" echo Error: Could not find an installation of Visual Studio && goto :failure | |||
if "%VSSDK150INSTALL%" == "" (if "%BUILD_VS%" == "1" echo Error: Could not find an installation of Visual Studio 2015 VSSDK which is required for building VS components && goto :failure) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought with the move to VSSDK nuget, we shouldnet need this whole version check? we would never use the local VSSDK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll remove this check.
<package id="Microsoft.VisualStudio.Language.StandardClassification" version="14.3.25407" targetFramework="net46" /> | ||
<package id="Microsoft.VisualStudio.Language.Intellisense" version="14.3.25407" targetFramework="net46" /> | ||
<package id="Microsoft.VisualStudio.Language.Intellisense" version="14.3.25407" targetFramework="net46" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update these references in any other packages.config
in the rep?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VS binding redirects will continue to take care of those until we have RTM packages that we can reference.
Do we need to update the |
The generated |
<ShortcutPath>..\CommonExtensions\Microsoft\FSharp</ShortcutPath> | ||
<PackageId>Microsoft.FSharp.VSIX.Full.Core</PackageId> | ||
<ExtensionInstallationRoot>CommonExtensions</ExtensionInstallationRoot> | ||
<ExtensionInstallationFolder>Microsoft\FSharp</ExtensionInstallationFolder> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These VSIXes both go to Microsoft\FSharp, which is not allowed. They need to go to different installation folders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These extensions will never be installed at the same time.
<ShortcutPath>..\CommonExtensions\Microsoft\FSharp</ShortcutPath> | ||
<PackageId>Microsoft.FSharp.VSIX.OpenSource.Core</PackageId> | ||
<ExtensionInstallationRoot>CommonExtensions</ExtensionInstallationRoot> | ||
<ExtensionInstallationFolder>Microsoft\FSharp</ExtensionInstallationFolder> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above.
@@ -5,7 +5,9 @@ | |||
<Identity Id="VisualFSharp" Version="15.4.1.0" Language="en-US" Publisher="Microsoft.VisualFSharpTools" /> | |||
<DisplayName>Visual F# Tools</DisplayName> | |||
<Description xml:space="preserve">Deploy Visual F# Tools templates to Visual Studio</Description> | |||
<ShortcutPath>..\CommonExtensions\Microsoft\FSharp</ShortcutPath> | |||
<PackageId>Microsoft.FSharp.VSIX.Full.Core</PackageId> | |||
<ExtensionInstallationRoot>CommonExtensions</ExtensionInstallationRoot> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These (ExtensionInstallationFolder and ExtensionInstallationRoot) should be added to the csproj as build properties, not the VSIX manifest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done.
<ShortcutPath>..\CommonExtensions\Microsoft\FSharp</ShortcutPath> | ||
<PackageId>Microsoft.FSharp.VSIX.Web.Core</PackageId> | ||
<ExtensionInstallationRoot>CommonExtensions</ExtensionInstallationRoot> | ||
<ExtensionInstallationFolder>Microsoft\FSharp</ExtensionInstallationFolder> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments above.
No description provided.