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

Upgrade the project system to depend on the latest version of Roslyn #4312

Merged
merged 4 commits into from
Dec 7, 2018

Conversation

jmarolf
Copy link
Contributor

@jmarolf jmarolf commented Nov 22, 2018

This updated to this commit of roslyn. This forces us to bring along several other VS dependencis.

NOTE: Async package made a breaking change forcing 3 tests to be disabled

This updated to [this](dotnet/roslyn@170d0bb) commit of roslyn.  This forces us to bring along several other VS dependencis.

NOTE: Async package made a breaking change forcing 3 tests to be disabled
@jmarolf jmarolf requested a review from a team as a code owner November 22, 2018 07:13
@jmarolf
Copy link
Contributor Author

jmarolf commented Nov 22, 2018

@genlu

@davkean
Copy link
Member

davkean commented Nov 22, 2018

Has this been inserted into Preview 2, if so, which build? If not, we can't merge this.

@jmarolf
Copy link
Contributor Author

jmarolf commented Nov 22, 2018

Has this been inserted into Preview 2

No Roslyn is currently working on that. Unlikely to happen before Monday.

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

Thanks!

@genlu
Copy link
Member

genlu commented Nov 28, 2018

@davkean I upgraded Roslyn to a build contains the new SetProperty API. But we haven't inserted anything to preview 2 yet, so this PR still can't be merged. Also, IWorkspaceProjectContext can be accessed from background thread now, could you please check if ForegroundWorkspaceProjectContext is still needed?

@davkean
Copy link
Member

davkean commented Nov 28, 2018

ForegroundWorkspaceProjectContext is known a issue in master, and you can ignore it. It's already been fixed.

@jmarolf
Copy link
Contributor Author

jmarolf commented Nov 29, 2018

NOTE: this PR #4318 also depends on this getting merged.

@davidwengier
Copy link
Member

Actually this Roslyn package isn't new enough to contain my changes, and I also need a new CPS package for that PR, so was going to tackle the integration as the last step.

@jmarolf
Copy link
Contributor Author

jmarolf commented Nov 30, 2018

Actually this Roslyn package isn't new enough to contain my changes

Fair enough. Once Roslyn insertion into Preview 2 we can take literally the newest package they have in master so that's not a problem. Is the CPS package that we would need to consume published?

@davidwengier
Copy link
Member

Yep, it is (>= 16.0.429-pre-gf4f06355f6) but I think Drew still needs a new one at some point, which would be after my changes there, so whoever gets there first wins with that one I guess.

@drewnoakes
Copy link
Member

The CPS change we need has been inserted to d16.0stg and a new package is on MyGet.

https://devdiv.visualstudio.com/DevDiv/_releaseProgress?_a=release-environment-logs&releaseId=201112&environmentId=805874

@genlu
Copy link
Member

genlu commented Dec 7, 2018

We finally completed the first roslyn insertion for preview 2.
https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/152224?_a=files

Could you update roslyn to 2.11.0-beta2-63529-05 to match the version and merge this?

@jmarolf
Copy link
Contributor Author

jmarolf commented Dec 7, 2018

@dotnet/project-system this is safe to merge now as the equivalent roslyn bits are in d16.0stg. Once things are green I am going to merge this.

@jmarolf jmarolf merged commit a19c1fc into dotnet:master Dec 7, 2018
@jmarolf jmarolf deleted the infrastructure/update-dependencies branch January 30, 2019 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants