Skip to content

made service hub works with dev15 preview 4 setup#12885

Merged
heejaechang merged 1 commit into
dotnet:dev15-preview-4from
heejaechang:dev15preview4setup
Aug 3, 2016
Merged

made service hub works with dev15 preview 4 setup#12885
heejaechang merged 1 commit into
dotnet:dev15-preview-4from
heejaechang:dev15preview4setup

Conversation

@heejaechang

Copy link
Copy Markdown
Contributor

some dependencies we got automatically in preview 3 now needed to be explicitly added in preview4

@heejaechang

Copy link
Copy Markdown
Contributor Author

@mavasani @dotnet/roslyn-ide @srivatsn can you take a look?

@heejaechang

Copy link
Copy Markdown
Contributor Author

I need to check this in to have signed bit ready for willow PR drop.

@heejaechang

Copy link
Copy Markdown
Contributor Author

retest vsi please

"Newtonsoft.Json": "6.0.6",
"Microsoft.VisualStudio.Threading": "14.1.131",
"Microsoft.VisualStudio.Validation": "14.1.111",
"RoslynDependencies.Microsoft.Build.Desktop": "14.3.25407"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also replace the current Roslyn.Microsoft.Build dependency in all our project.json files with this new nuget package.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

how about creating new issue for that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, that is fine too. Can you create one for infrastructure team?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mavasani

mavasani commented Aug 3, 2016

Copy link
Copy Markdown
Contributor

👍

@heejaechang heejaechang merged commit 90f4815 into dotnet:dev15-preview-4 Aug 3, 2016
@Pilchie

Pilchie commented Aug 3, 2016

Copy link
Copy Markdown
Member

This was not approved for merging into the dev15-preview-4 branch yet.

@heejaechang

Copy link
Copy Markdown
Contributor Author

@Pilchie ah, sorry I was thinking this is part of the previous OOP work which got approved. should I revert it back?

"Microsoft.Build.Tasks.Core.dll",
"Microsoft.Build.Utilities.Core.dll",
"Microsoft.VisualStudio.Threading.dll",
"Microsoft.VisualStudio.Validation.dll",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So that I understand - does this mean it's part of our VSIX that we build for dogfooding, but is NOT inserted as part of our binaries for VS? Do we pick up the ones installed by VS in that case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this basically means, we don't put these in our wxs setup authoring file. our service hub authoring (service hub json file) uses vsix mechanism and only that mechanism. which means OOP installed through wxs won't work anyway since servicehub won't discover us.

since we use vsix mechanism, it doesn't matter how it got installed. as long as vsix is installed, OOP will work.

in preview 4, since willow uses vsix as a way to install our component, it will work just fine.

except, there was some change which cause us to include all our dependency explicitly which we would have gotten implicitly in preview 3.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. We still need to keep "classic" installs working (when built with the .wxs files).
  2. I'm not thrilled about us having to redistribute all of these binaries, and them potentially being out of sync with the main versions used by VS. It may be okay for Preview 4, but we should try to unify after that.

@heejaechang heejaechang Aug 3, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FYI. I talked with @srivatsn about this before going this route.

anyway, for

  1. legacy set up VS will continue to work. just OOP won't be supported. as far as I know and told, since Dev15 is willow only and OOP is for dev15, OOP not working with legacy setup is okay. (by the way, our service hub service won't work with dev14 and dev15 preview 3 - for preview 3, if one override preview 3 service hub with preview 4 service hub bits, our service, vsix based service hub service, will work)

  2. MS.VS.* dlls are required for JsonRpc we use as a mechanism to communicate with VS. if we change it to some other mechanism such as MessagePack or others, we will need dlls for those. basically, it is just part of lib we use like NewstonJson dll. we just happen to not needed to distribute it in preview 3 since service hub.exe itself used that and they were already loaded. which is no longer case for preview 4 since service hub now isolate us from them so we no longer share those dlls.

Microsoft.Build.* dependency is not specifically for OOP, but MS.CA.Workspace.Desktop's runtime dependencies. any one who consume MS.CA.Workspace.Desktop.dll will need them (which our service hub services does).

we can get rid of those if we split MSBuildWorkspace out from MS.CA.Workspace.Desktop.dll by the way. (like MS.CA.MSBuildWorkspace.dll which has dependency to MS.CA.Workspace.Desktop.dll)

anyway, this change is not something temporary for preview 4. this is the setup we will use in dev15 RTM.

if we want to get rid of Microsoft.Build.* dlls, only way I can think of is splitting MSBuildWorkspace from MS.CA.Workspace.Desktop.dll

heejaechang pushed a commit to heejaechang/roslyn that referenced this pull request Aug 4, 2016
made service hub works with dev15 preview 4 setup
heejaechang added a commit to heejaechang/roslyn that referenced this pull request Aug 4, 2016
…4setup"

This reverts commit 90f4815, reversing
changes made to 720f5c5.
davidwengier added a commit that referenced this pull request Apr 28, 2026
Sorry, I promised Todd I wouldn't remove any more big swathes of code,
but when [converting the SDK](dotnet/sdk#53345)
to use System.Text.Json I discovered that none of the serialization code
in this repo is actually used there, and so it was a case of either
leave this code to be unused, and not matching the SDK, or convert
unused code, or just get rid of it. So I got rid of it.

Arguably some of this is bits I missed in the last PR anyway (in that PR
I removed ProjectSnapshotHandle from being used, but left the code that
knew how to serialize it 🤷‍♂️)
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.

4 participants