Skip to content
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

Support VS "15" specific features #10812

Merged
merged 15 commits into from
May 13, 2016
Merged

Support VS "15" specific features #10812

merged 15 commits into from
May 13, 2016

Conversation

Pilchie
Copy link
Member

@Pilchie Pilchie commented Apr 23, 2016

Changing to NuGet packages meant that we were referencing some older
versions of things causing us problems:

  1. I needed to package up Microsoft.VisualStudio.Text.Internal so that we
    can use the v14 assembly since it in turn references other things we
    normally get from NuGet.
  2. Now that we are using the packages, we're referencing
    Microsoft.VisualStudio.Shell.14.dll instead of .15 again. This means
    that types that moved to Shell.Immutable.15 are not type forwarders.
    Luckily we didn't need anything from Shell.Immutable.15 in the project
    where it was a problem.

@@ -17,6 +17,7 @@
</config>
<packageSources>
<clear />
<add key="local" value="C:\Code\Packages" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously not correct - we'll need to push a real package.

@Pilchie
Copy link
Member Author

Pilchie commented Apr 24, 2016

Adding the rest of @dotnet/roslyn-ide to take a look.

@Pilchie Pilchie changed the title Work in progress: Get build working with VS "15" again Work in progress: Support VS "15" specific features Apr 24, 2016
<StartAction>Program</StartAction>
<StartProgram>$(DevEnvDir)devenv.exe</StartProgram>
<StartArguments>/rootsuffix RoslynDev /log</StartArguments>
</PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you F5 the Dev14 project from Dev15? What's supposed to happen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will build and deploy the Dev14 stuff to Dev15 and run it without the lightup.

@Pilchie
Copy link
Member Author

Pilchie commented May 12, 2016

test vsi please

@Pilchie
Copy link
Member Author

Pilchie commented May 12, 2016

retest vsi please

@Pilchie Pilchie changed the title Work in progress: Support VS "15" specific features Support VS "15" specific features May 12, 2016
@Pilchie
Copy link
Member Author

Pilchie commented May 12, 2016

Ping @CyrusNajmabadi @davkean @jasonmalinowski @heejaechang @dotnet/roslyn-ide Can people start reviewing this "for real" now. I think it's ready to go.

It doesn't make 100% of tests pass on Dev15 yet, but it unblocks development, and keeps Dev14 green.

@@ -15,7 +15,8 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<SolutionDir Condition="'$(SolutionDir)' == '' OR '$(SolutionDir)' == '*Undefined*'">..\..\..\..\</SolutionDir>
<RestorePackages>true</RestorePackages>
<TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
<TargetFrameworkVersion>v4.6</TargetFrameworkVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, are we moving to 4.6?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes for future (in fact, I think @agocke already did it there). We can't for master for shipping projects, but since this is non-shipping, I felt comfortable updating it.

@CyrusNajmabadi
Copy link
Member

Looks mostly good to me. I think there's a cleaner dev approach that avoids needing ifdefs in our code. It does mean we'll have more forwarding methods to approximate hte mixin pattern. But i still think that's nicer FWIW.

@CyrusNajmabadi
Copy link
Member

Don't gate yourself on this feedback though
we can always make such an improvement later.

@Pilchie
Copy link
Member Author

Pilchie commented May 12, 2016

retest vsi please

@Pilchie
Copy link
Member Author

Pilchie commented May 12, 2016

retest vsi p2 please

Condition="'$(ForceGenerationOfBindingRedirects)' == 'true'">

<PropertyGroup>
<!-- Needs to be set in a target because it has to be set after the initial evaluation in the common targets -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a bug tracking this on the MSBuild side?

@@ -44,7 +45,8 @@ class Q
var typeQ = assembly.GetType("Q");

string defaultDebuggeeSideVisualizerTypeName = "Microsoft.VisualStudio.DebuggerVisualizers.VisualizerObjectSource";
string defaultDebuggeeSideVisualizerAssemblyName = "Microsoft.VisualStudio.DebuggerVisualizers, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a";
var vsVersion = Environment.GetEnvironmentVariable("VisualStudioVersion") ?? "14.0";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share in a test helper somewhere?

@jasonmalinowski
Copy link
Member

👍, modulo various nitpicks. Only clear functional problem I saw was the use of types from Roslyn.VisualStudio.Setup.dll which we don't ship, and thus might break various things at runtime.

Pilchie and others added 14 commits May 12, 2016 21:00
1. Reference a bunch of stuff from Nuget packages instead of VS.
2. Move to Update 2 versions of packages.
3. Now that we are using the packages, we're referencing
   Microsoft.VisualStudio.Shell.14.dll instead of .15 again.  This means
   that types that moved to Shell.Immutable.15 are not type forwarders.
   Luckily we didn't need anything from Shell.Immutable.15 in the project
   where it was a problem.
More cleanup to do between RoslynCompletionSet and
FilteredRoslynCompletionSet.
@Pilchie
Copy link
Member Author

Pilchie commented May 13, 2016

retest vsi please

@Pilchie Pilchie merged commit c90e10d into dotnet:master May 13, 2016
@Pilchie Pilchie deleted the Dev15Build branch May 13, 2016 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants