Skip to content

Make IISIntegration use Reference instead of PackageReference (and reactionary work) #4311

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Dec 12, 2018
Merged

Make IISIntegration use Reference instead of PackageReference (and reactionary work) #4311

merged 13 commits into from
Dec 12, 2018

Conversation

jkotalik
Copy link
Contributor

@jkotalik jkotalik commented Nov 29, 2018 •
edited
Loading

  • Make IISIntegration use Reference
  • Use full msbuild to be able to build vcxproj files
  • Remove obsolete ANCMV2 stuff from 2.1.

@jkotalik jkotalik force-pushed the jkotalik/Update21Ref branch from 3a2ea5f to 37e357c Compare November 30, 2018 01:02
@jkotalik
Copy link
Contributor Author

For #4246.

@jkotalik jkotalik changed the title [WIP] Make IISIntegration use Reference instead of PackageReference (and reactionary work) Make IISIntegration use Reference instead of PackageReference (and reactionary work) Nov 30, 2018
@natemcmaster natemcmaster mentioned this pull request Nov 30, 2018
Closed
24 tasks
@jkotalik
Copy link
Contributor Author

Still can't figure out the Nuget.Packaging issue. Besides that, I need to resolve some long path issues and a bit of folder structure stuff.

@natemcmaster
Copy link
Contributor

We're adding required PR checks to ensure the repo compiles before you merge. Can you update this PR by rebasing on master?

git fetch origin master:master
git rebase master

Alternative: if you plan to 'squash merge' this PR, you can also use a merge to update this PR. This can be easier to manage if you have lots of changes in this PR.

git fetch origin master:master
git merge master

You can ignore the error that says "AspNetCore-pr-validation — Unable to find '.azure/pipelines/fast-pr-validation.yml'". This is the result of changing PR checks after a PR was created.

@jkotalik jkotalik force-pushed the jkotalik/Update21Ref branch from ab86f46 to a0ac480 Compare December 7, 2018 00:22
@natemcmaster
Copy link
Contributor

Windows build passes, but fails on macos/linux.

System.IO.DirectoryNotFoundException: Could not find a part of the path '/Users/vsts/agent/2.142.1/work/1/s/src/Servers/IIS/src/AspNetCoreModuleV1/AspNetCore/bin/Release/Win32'

Probably just need to exclude the c++ stuff on non-Windows.

@natemcmaster
Copy link
Contributor

I think we should take a closer look at getting rid of src/Servers/IIS/version.props. If possible, it would be nice to have the root-level version.props file defining the needful settings.

@natemcmaster natemcmaster requested a review from pakrym December 7, 2018 22:18
@natemcmaster
Copy link
Contributor

E.g. in the master branch: https://github.com/aspnet/AspNetCore/blob/85e2147ff037cc9950249fbb9d095ad47ec4184a/version.props#L10

This might be related to #4289 (cc @pakrym )

natemcmaster
natemcmaster reviewed Dec 7, 2018
View reviewed changes
Copy link
Contributor

@natemcmaster natemcmaster left a comment

Choose a reason for hiding this comment

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

A few questions. Looks like this is close.

@jkotalik jkotalik force-pushed the jkotalik/Update21Ref branch from 42d1179 to 60be6a5 Compare December 7, 2018 22:53
@natemcmaster natemcmaster mentioned this pull request Dec 11, 2018
Merged
@jkotalik jkotalik force-pushed the jkotalik/Update21Ref branch from 54895a9 to 8686a58 Compare December 12, 2018 16:49
@jkotalik jkotalik mentioned this pull request Dec 12, 2018
Merged
@natemcmaster
Copy link
Contributor

It looks like this will save 3-5 minutes on build time. Yay! 👍

@natemcmaster
Copy link
Contributor

Are you looking into the test failures? They appear to be the result of reorganizing source code.

IISIntegration.FunctionalTests.Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests.NtlmAuthenticationTests.NtlmAuthentication_CoreClr_X64_Portable
System.IO.DirectoryNotFoundException : Application path D:\a\1\s\src\Servers\src\servers\iis\test\testassets\OutOfProcessWebSite does not exist

@natemcmaster
Copy link
Contributor

It looks like remaining test failures are known issues tracked in aspnet/AspNetCore-Internal. Good work! :shipit:

Head's up @ajaybhargavb - we will probably need to handle the Razor merge from 2.1 => 2.2 separately from this one as both are large changes.

@ajaybhargavb
Copy link
Contributor

Head's up @ajaybhargavb - we will probably need to handle the Razor merge from 2.1 => 2.2 separately from this one as both are large changes.

I don't follow. They are anyways separate merges right?

@jkotalik
Copy link
Contributor Author

Don't merge Razor into 2.1 before IISIntegration has been merged into 2.2

@ajaybhargavb
Copy link
Contributor

Don't merge Razor into 2.1 before IISIntegration has been merged into 2.2

Okay. I assume #4606 needs to go in before you can merge this to 2.2

@jkotalik
Copy link
Contributor Author

Whoops, didn't realize there were other razor changes currently in 2.1. Yes finalize that before I merge this 😄

@ajaybhargavb
Copy link
Contributor

#4612 is in. This is now good to go.

@jkotalik jkotalik merged commit 429719b into release/2.1 Dec 12, 2018
@jkotalik jkotalik deleted the jkotalik/Update21Ref branch December 12, 2018 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natemcmaster natemcmaster natemcmaster approved these changes

@pakrym pakrym Awaiting requested review from pakrym

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jkotalik @natemcmaster @ajaybhargavb