[WIP] add source link support to portable pdbs #5860

ctaggart · 2017-02-27T00:00:16Z

This builds off the csproj branch #5852. It just demos adding source link support. It has the same changes I made to dotnet/efcore#7726. Please see the comments there. Let's see if this build is better maintained.

Here are the steps for trying it out here:

git clone git@github.com:ctaggart/Mvc.git -c core.autocrlf=input
cd Mvc\src\Microsoft.AspNetCore.Mvc
dotnet build /p:ci=true
dotnet sourcelink print-json .\bin\Debug\netstandard1.6\Microsoft.AspNetCore.Mvc.Abstractions.pdb
dotnet sourcelink print-urls .\bin\Debug\netstandard1.6\Microsoft.AspNetCore.Mvc.Abstractions.pdb
dotnet sourcelink test .\bin\Debug\netstandard1.6\Microsoft.AspNetCore.Mvc.Abstractions.pdb

The dotnet sourcelink commands are just showing you that everything is working.

- Allow >= RC3 CLI's to build and run MVC. - Added extra sln's so they could be opened in VS. Currently VS' project system can't currently handle Mvc.sln. - Worked around several dotnet migration issues. They are listed in the re-attempted migration section here: aspnet#5482 - One large feature bit that couldn't be worked around was the functional tests running on desktop; it represented several known vstest issues. Removed desktop running of functional tests. - Skipped an ActionContextAccessor test due to a vstest appdomain problem.

- see 63507c8 for previous, project.json work - xUnit no longer supports .NET 4.5.1 - build tests for desktop .NET only on Windows - always place xunit.runner.visualstudio dependency after xunit; helps build scripts run correctly

Question: Should Microsoft.Extensions.DependencyModel dependency really be 1.1.0?

…s from dev - manually recreate *.sln files to correct VS versions and use old project GUIDs - add missing lines to the .sln files; build configuration likely not used before - manually migrate RazorPagesWebSite project - remove `ToolsVersion` attributes - add repo.props file; never test TestCommon project and only build Mvc.sln - add dependencies.props file to match other repos Test-related: - add `$(PreserveCompilationContext)` to test projects that examine their own dependencies - remove comments from xunit.runner.json files; xUnit does not support them - remove .notest files - work around inability to deserialize a odd `ref` type - xUnit and vstest now serialize / deserialze test data more often - work around microsoft/vstest#419 instead of skipping test - enable testes that hit microsoft/vstest#427; bug has been fixed - add `<Service>`s to test projects

- build unconditionally creates a package if this is in the shared folder - basically, remove the .xproj that's in old world and compile the files in

- re-enable .NET Framework run of the functional tests - disable shadow copying - add support for `/p:GenerateBaselines=true` for functional and Razor.Host tests - remove aspnet/KoreBuild#182 workaround; bug fixed - remove `Microsoft.DotNet.InternalAbstractions` and `System.Xml.XmlDocument` dependencies - stop floating `$(CoreFxVersion)` and `Microsoft.Extensions.DependencyModel` dependencies - separate `GetCSharpTypeName_ReturnsCorrectTypeNames_ForOutParameter()` test - remove `$(RuntimeIdentifier)` settings nits: - remove all web.config files - remove conditional compilation from class libraries - remove unnecessary project properties from `UserClassLibrary` - move a few properties to the top of a .csproj file - remove some trailing whitespace in .csproj files

…cListener a regular class library

dnfclas · 2017-02-27T00:00:20Z

@ctaggart,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

ctaggart · 2017-02-27T00:01:54Z

build/SourceLink.props

@@ -0,0 +1,11 @@
+<Project>
+  <PropertyGroup>
+    <SourceLinkCreate>true</SourceLinkCreate>


Forcing it on with <SourceLinkCreate>true</SourceLinkCreate> is just to show that it works. The line should be removed before a merge.

Why would we remove it? Isn't the plan to enable the feature by default?

You want to run source link on whatever you are using as your build server. It doesn't need to run every time someone is doing development locally. It is setup by default to be enabled when the CI environment variable is set to true. This works for AppVeyor, Travis CI, and others. You can also turn on SourceLinkCreate by passing in the property dotnet build /p:SourceLinkCreate=true or /p:CI=true. Hope that helps.

FYI @natemcmaster - something for us to track in KoreBuild.

davidfowl · 2017-02-27T00:18:07Z

build/SourceLink.props

+    <SourceLinkNoAutoLF>true</SourceLinkNoAutoLF>
+  </PropertyGroup>
+  <ItemGroup>
+    <PackageReference Include="SourceLink.Create.GitHub" Version="2.0.0-*" />


This should be PrivateAssets="all'

Thanks. I'll update it and my docs.

@davidfowl Just to confirm, is only has stuff in the build folder. PrivateAssets="all" is needed for build only packages?

Yes. It'll make sure it doesn't end up in the dependencies of the packages produced. Hopefully we get this fixed NuGet/Home#4125. Which means this will automatically happen for packages marked as dev dependencies.

ctaggart · 2017-03-04T20:04:47Z

Closing this pull request. It was just an example of how easy it is to add.

NTaylorMullen and others added 12 commits February 25, 2017 19:30

Addressed code review comments.

de7b4d0

React to recent project.json changes.

c808c90

Addressed code review comments.

e43b537

Change xunit.runner.json inclusion scheme.

c5e090b

Apply test migration to .NET 4.5.2 in *.csproj world

daac4be

- see 63507c8 for previous, project.json work - xUnit no longer supports .NET 4.5.1 - build tests for desktop .NET only on Windows - always place xunit.runner.visualstudio dependency after xunit; helps build scripts run correctly

Apply dependency version downgrade from 0097e40 in .csproj world

a51f2ee

Question: Should Microsoft.Extensions.DependencyModel dependency really be 1.1.0?

Move TestDiagnosticListener.Sources back into test folder

36ba0e5

- build unconditionally creates a package if this is in the shared folder - basically, remove the .xproj that's in old world and compile the files in

Per offline conversation, make Microsoft.AspNetCore.Mvc.TestDiagnosti…

d350769

…cListener a regular class library

add source link support to portable pdbs

f3ba66e

dnfclas added the cla-already-signed label Feb 27, 2017

ctaggart commented Feb 27, 2017

View reviewed changes

ctaggart mentioned this pull request Feb 27, 2017

Source Index and pack .pdb files into regular .nupkg aspnet/Universe#131

Closed

4 tasks

davidfowl reviewed Feb 27, 2017

View reviewed changes

ctaggart added 2 commits February 26, 2017 18:29

PrivateAssets="all"

d9c45c8

turn on with dotnet build /p:ci=true instead

189587d

dougbu force-pushed the dougbu/migration branch 2 times, most recently from a136c86 to acfad83 Compare March 1, 2017 05:50

ctaggart closed this Mar 4, 2017

ctaggart deleted the sourcelink branch March 4, 2017 20:51

ctaggart restored the sourcelink branch March 4, 2017 20:51

ctaggart deleted the sourcelink branch March 4, 2017 22:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] add source link support to portable pdbs #5860

[WIP] add source link support to portable pdbs #5860

ctaggart commented Feb 27, 2017 •

edited

Loading

dnfclas commented Feb 27, 2017

ctaggart Feb 27, 2017

pranavkm Feb 27, 2017 •

edited

Loading

ctaggart Feb 27, 2017 •

edited

Loading

pranavkm Feb 27, 2017

davidfowl Feb 27, 2017

ctaggart Feb 27, 2017

ctaggart Feb 27, 2017

davidfowl Feb 27, 2017

ctaggart commented Mar 4, 2017

[WIP] add source link support to portable pdbs #5860

[WIP] add source link support to portable pdbs #5860

Conversation

ctaggart commented Feb 27, 2017 • edited Loading

dnfclas commented Feb 27, 2017

ctaggart Feb 27, 2017

Choose a reason for hiding this comment

pranavkm Feb 27, 2017 • edited Loading

Choose a reason for hiding this comment

ctaggart Feb 27, 2017 • edited Loading

Choose a reason for hiding this comment

pranavkm Feb 27, 2017

Choose a reason for hiding this comment

davidfowl Feb 27, 2017

Choose a reason for hiding this comment

ctaggart Feb 27, 2017

Choose a reason for hiding this comment

ctaggart Feb 27, 2017

Choose a reason for hiding this comment

davidfowl Feb 27, 2017

Choose a reason for hiding this comment

ctaggart commented Mar 4, 2017

ctaggart commented Feb 27, 2017 •

edited

Loading

pranavkm Feb 27, 2017 •

edited

Loading

ctaggart Feb 27, 2017 •

edited

Loading