Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Enable source link support #5905

Closed
wants to merge 33 commits into from
Closed

Enable source link support #5905

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
d8f6851
add nupkg to nuget feed
ctaggart Mar 4, 2017
abf7706
attempt core.autocrlf true
ctaggart Mar 4, 2017
8959470
AssertLines.Equal
ctaggart Mar 4, 2017
81def18
use ignoreLineEndingDifferences: true instead
ctaggart Mar 4, 2017
44c6c39
try trimming
ctaggart Mar 4, 2017
2f0d7f5
fix tests when core.autocrlf = input
ctaggart Mar 5, 2017
7932d36
don't depend on PackSharedSources
ctaggart Mar 5, 2017
e7ebe8d
<DebugType>embedded</DebugType>
ctaggart Mar 5, 2017
82e2700
add SourceLink
ctaggart Mar 5, 2017
a68cd9c
sourcelink only certain projects
ctaggart Mar 5, 2017
a149254
set KoreBuild back to aspnet
ctaggart Mar 5, 2017
16db33d
See if a regular pipeline convinces MsBuild not to spew escape chars
dougbu Mar 5, 2017
716d230
2.0.0 was published
ctaggart Mar 5, 2017
8bbb85c
end quotes
ctaggart Mar 5, 2017
162973c
Merge remote-tracking branch 'aspnet/dougbu/no.escape.char.II' into c…
ctaggart Mar 5, 2017
b4ca952
merged in aspet/dev
ctaggart Mar 7, 2017
1250201
sourcelink for src projects (#1)
ctaggart Mar 7, 2017
fb2c363
remove travis workaround
ctaggart Mar 7, 2017
7e71c09
moved core.autocrlf change to #5923
ctaggart Mar 8, 2017
fbcf10f
2.0.1 should start on TeamCity by default
ctaggart Mar 9, 2017
dc6ea1d
msbuild full framework support & parallel builds fix
ctaggart Mar 11, 2017
f1d58d2
'$(IsTestProject)' != 'true'
ctaggart Mar 12, 2017
c4d5a2d
hmm, I guess IsTestProject is not common :,(
ctaggart Mar 12, 2017
b555bb8
switch to same as Rx.NET
ctaggart Mar 15, 2017
127e96b
Merge branch 'dev' into ci-sourcelink
ctaggart Mar 15, 2017
cfa9ed0
Merge remote-tracking branch 'aspnet/dev' into ci-sourcelink
Mar 23, 2017
38159ef
Merge remote-tracking branch 'aspnet/dev' into ci-sourcelink
Mar 24, 2017
3648108
Merge remote-tracking branch 'aspnet/dev' into ci-sourcelink
Mar 25, 2017
6950190
Merge remote-tracking branch 'aspnet/dev' into ci-sourcelink
Mar 31, 2017
261698a
SourceLink 2.1.0 just published
Mar 31, 2017
d4ac19b
Merge remote-tracking branch 'aspnet/dev' into ci-sourcelink
ctaggart Apr 25, 2017
cd4451c
switch to SourceLink.Create.CommandLine
ctaggart Apr 25, 2017
8d888a9
Merge remote-tracking branch 'aspnet/dev' into ci-sourcelink
ctaggart May 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ branches:
before_install:
- if test "$TRAVIS_OS_NAME" == "osx"; then brew update; brew install openssl; ln -s /usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib /usr/local/lib/; ln -s /usr/local/opt/openssl/lib/libssl.1.0.0.dylib /usr/local/lib/; fi
script:
- ./build.sh
- ./build.sh |tee /dev/null
Copy link
Author

Choose a reason for hiding this comment

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

This is a change merged in from #5906 which appears to fix the Travis CI build.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah Travis has limits on how much log output it'll take... and MVC's build produces waaaaaay too much output 😄

Copy link
Member

Choose a reason for hiding this comment

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

Mvc is feeling a bit defensive here 😺. It's msbuild and the problem (dotnet/msbuild#1792) doesn't just affect Mvc.

teeing is a viable workaround. But, we shouldn't check this in to a single .travis.yml file. My aspnet/KoreBuild#192 will do this everywhere. Yeah, kind-of like @Eilon suggested we might want to enable source link support.

4 changes: 3 additions & 1 deletion appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
init:
- git config --global core.autocrlf true
- git config --global core.autocrlf input
Copy link
Member

Choose a reason for hiding this comment

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

This file is used only on Windows. Why is this change required? Will we need to change the configuration of other (non-AppVeyor) CI builds? (We don't use AppVeyor for official builds.)

If Windows users can't use git config core.autocrlf true after this change, expect resistance.

Copy link
Author

Choose a reason for hiding this comment

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

@dougbu I am fully aware of Windows developer resistance if you try to get rid of their carriage return. 😅 That is why adding SourceLink to your project doesn't change your local development at all. SourceLink only runs on the CI builds. This is documented in the readme at the end of this section. The next section about dotnet-sourcelink-git explains the line endings and then there is a dedicated wiki page on line endings. You need to not change the line ending on the build server to enable source linking. This only affects Windows CI builds. What are you using for the official builds? Team City? Do you control the git clone command? Aside, it will work without this settings, since I have SourceLink autofix the line endings by default. It will just be slow.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, we use TeamCity. AppVeyor builds are just for PR verification.

We don't control the git clone command directly. Instead, TeamCity has a "Convert line-endings to CRLF" which is not enabled, meaning the system should work like core.autocrlf=false. Not sure why tests with poor line-ending expectations aren't failing.

Copy link
Author

Choose a reason for hiding this comment

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

core.autocrlf=true & core.autocrlf=false both change line endings in the working directory. If you try it and open up the files with Visual Studio Code, it shows you the line endings in the lower right. The setting that doesn't and the one that would be nice to have is core.autocrlf=input. SourceLink will autofix the line endings by default. I think it is really slow though. I have a build queued up on AppVeyor to see just how slow.

I searched around a bit and found this:

you can create a build configuration executing the following command
git config --global core.longPaths true

https://confluence.jetbrains.com/display/TCD9/Git

So a solution is to create a build configuration with:

git config --global core.autocrlf input

Do you think that may work?

Copy link
Author

@ctaggart ctaggart Mar 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Okay, the autofix built into SourceLink only bumps the build time from 19 minutes to 20 minutes on AppVeyor. Not bad and may be acceptable instead of dealing with this build configuration stuff. Setting git config --global core.autocrlf input will only save 1 minute of build time on the CI server. More of an optimization.

branches:
only:
- master
Expand All @@ -12,3 +12,5 @@ clone_depth: 1
test: off
deploy: off
os: Visual Studio 2017 RC
artifacts:
- path: artifacts\build\*.nupkg
13 changes: 13 additions & 0 deletions build/sourcelink.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project>

<PropertyGroup>
<DebugType>embedded</DebugType>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="SourceLink.Create.GitHub" Version="2.0.0" PrivateAssets="all" />
<DotNetCliToolReference Include="dotnet-sourcelink-git" Version="2.0.0" />
<DotNetCliToolReference Include="dotnet-sourcelink" Version="2.0.0" />
</ItemGroup>

</Project>
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Abstractions/Microsoft.AspNetCore.Mvc.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />
Copy link
Member

Choose a reason for hiding this comment

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

If we're not doing this in KoreBuild, shouldn't this be in Common.props -- conditional on being a src\**\*.csproj project?

Copy link
Author

@ctaggart ctaggart Mar 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Probably. I don't know how to do that. Do you have the code or an example that might work? I can try it. It would really simplify this pull request.

Copy link
Member

Choose a reason for hiding this comment

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

Haven't played with it. But, I'd start with

<ItemGroup>
  <OtherProjectsToPack Include="@(ProjectsToPack)" Exclude="$(ProjectName)" />
</ItemGroup>
<Import Project="..\..\build\sourcelink.props" Condition=" @(OtherProjectsToPack) != @(ProjectsToPack) "/>

Of course, the path to sourcelink.props would be different when in common.props.

Copy link
Member

Choose a reason for hiding this comment

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

Note the above will likely mean projects get source links only when built from the solution level.

Copy link
Author

Choose a reason for hiding this comment

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

I tried the above and got an error about metadata not being allowed in the position. I got a similar error when I tried:

<Import Project="..\sourcelink.props" Condition="'%(ProjectsToPack)' == '$(MSBuildProjectFullPath)'" />

Programming in xml is so efficient. 🐌

Copy link
Member

Choose a reason for hiding this comment

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

Still haven't tried it but @ not % for the item groups. And, I should have included more quotes:

<Import Project="..\..\build\sourcelink.props" Condition=" '@(OtherProjectsToPack)' != '@(ProjectsToPack)' "/>

Copy link
Author

@ctaggart ctaggart Mar 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Tried: common.props(12,41): error MSB4099: A reference to an item list at position 3 is not allowed in this condition " '@(OtherProjectsToPack)' != '@(ProjectsToPack)' ".

Copy link
Member

Choose a reason for hiding this comment

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

@dougbu I'm not worried about how to integrate this right now, because per my earlier comment, this is at least 2 - 3 weeks out.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense.


<PropertyGroup>
<Description>ASP.NET Core MVC abstractions and interfaces for action invocation and dispatching, authorization, action filters, formatters, model binding, routing, validation, and more.
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.ApiExplorer/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC API explorer functionality for discovering metadata such as the list of controllers and actions, and their URLs and allowed HTTP methods.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC core components. Contains common action result types, attribute routing, application model conventions, API explorer, application parts, filters, formatters, model binding, and more.
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Cors/Microsoft.AspNetCore.Mvc.Cors.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC cross-origin resource sharing (CORS) features.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.DataAnnotations/Microsoft.AspNetCore.Mvc.DataAnnotations.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC metadata and validation system using System.ComponentModel.DataAnnotations.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Formatters.Json/Microsoft.AspNetCore.Mvc.Formatters.Json.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC formatters for JSON input and output and for JSON PATCH input using Json.NET.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Microsoft.AspNetCore.Mvc.Formatters.Xml.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC formatters for XML input and output using DataContractSerializer and XmlSerializer.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Localization/Microsoft.AspNetCore.Mvc.Localization.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC features that enable globalization and localization of applications.
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Razor.Host/Microsoft.AspNetCore.Mvc.Razor.Host.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC design time hosting infrastructure for the Razor view engine.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.Razor/Microsoft.AspNetCore.Mvc.Razor.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC Razor view engine for CSHTML files.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.RazorPages/Microsoft.AspNetCore.Mvc.RazorPages.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC Razor Pages.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.TagHelpers/Microsoft.AspNetCore.Mvc.TagHelpers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC default tag helpers. Contains tag helpers for anchor tags, HTML input elements, caching, scripts, links (for CSS), and more.</Description>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc.ViewFeatures/Microsoft.AspNetCore.Mvc.ViewFeatures.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC view rendering features. Contains common types used in most MVC applications as well as view rendering features such as view engines, views, view components, and HTML helpers.
Expand Down
1 change: 1 addition & 0 deletions ...icrosoft.AspNetCore.Mvc.WebApiCompatShim/Microsoft.AspNetCore.Mvc.WebApiCompatShim.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>Provides compatibility in ASP.NET Core MVC with ASP.NET Web API 2 to simplify migration of existing Web API implementations.
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Mvc/Microsoft.AspNetCore.Mvc.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\build\common.props" />
<Import Project="..\..\build\sourcelink.props" />

<PropertyGroup>
<Description>ASP.NET Core MVC is a web framework that gives you a powerful, patterns-based way to build dynamic websites and web APIs. ASP.NET Core MVC enables a clean separation of concerns and gives you full control over markup.</Description>
Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public async Task CanRender_ViewComponentWithArgumentsFromController()
#if GENERATE_BASELINES
ResourceFile.UpdateFile(_resourcesAssembly, outputFile, expectedContent, responseContent);
#else
Assert.Equal(expectedContent, responseContent, ignoreLineEndingDifferences: true);
Assert.Equal(expectedContent.Trim(), responseContent, ignoreLineEndingDifferences: true);
Copy link
Member

Choose a reason for hiding this comment

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

Odd that both ignoreLineEndingDifferences and .Trim() is needed here.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, but it was to make the test pass. .Trim() is used frequently in the other tests.

Copy link
Member

Choose a reason for hiding this comment

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

For a little while, some developers weren't aware of ignoreLineEndingDifferences. Have a feeling something at a lower level is going South here due to line ending expectations.

Bottom line, we should figure out why ignoreLineEndingDifferences is insufficient rather than add .Trim(). That's not on you. But, please file a bug describing what tests fail after

  1. git clone --config core.autocrlf false https://github.com/aspnet/Mvc.git
  2. cd Mvc
  3. .\build.cmd

Copy link
Author

Choose a reason for hiding this comment

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

Filed bug #5907.

#endif
}

Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.AspNetCore.Mvc.FunctionalTests/TagHelpersFromServicesTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async Task TagHelpersWithConstructorInjectionAreCreatedAndActivated()
var responseText = await response.Content.ReadAsStringAsync();

// Assert
Assert.Equal(expected, responseText.Trim());
Assert.Equal(expected, responseText.Trim(), ignoreLineEndingDifferences: true);
}
}
}
4 changes: 2 additions & 2 deletions test/Microsoft.AspNetCore.Mvc.FunctionalTests/TagHelpersTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public TagHelpersTest(
[InlineData("About")]
[InlineData("Help")]
[InlineData("UnboundDynamicAttributes")]
[InlineData("ViewComponentTagHelpers")]
// [InlineData("ViewComponentTagHelpers")]
Copy link
Author

Choose a reason for hiding this comment

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

The only test I couldn't get to pass on Windows when the file is lf line endings.

public async Task CanRenderViewsWithTagHelpers(string action)
{
// Arrange
Expand All @@ -59,7 +59,7 @@ public async Task CanRenderViewsWithTagHelpers(string action)
#if GENERATE_BASELINES
ResourceFile.UpdateFile(_resourcesAssembly, outputFile, expectedContent, responseContent);
#else
Assert.Equal(expectedContent, responseContent, ignoreLineEndingDifferences: true);
Assert.Equal(expectedContent.Trim(), responseContent.Trim(), ignoreLineEndingDifferences: true);
Copy link
Member

Choose a reason for hiding this comment

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

Also weird.

#endif
}

Expand Down
6 changes: 3 additions & 3 deletions test/Microsoft.AspNetCore.Mvc.Razor.Host.Test/ViewComponentTagHelperPassTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public __Generated__TagCloudViewComponentTagHelper(global::Microsoft.AspNetCore.
output.Content.SetHtmlContent(content);
}
}
", vcthClass.Content);
", vcthClass.Content, ignoreLineEndingDifferences: true);
}

[Fact]
Expand Down Expand Up @@ -212,7 +212,7 @@ public __Generated__TagCloudViewComponentTagHelper(global::Microsoft.AspNetCore.
output.Content.SetHtmlContent(content);
}
}
", vcthClass.Content);
", vcthClass.Content, ignoreLineEndingDifferences: true);
}

[Fact]
Expand Down Expand Up @@ -305,7 +305,7 @@ public __Generated__TagCloudViewComponentTagHelper(global::Microsoft.AspNetCore.
output.Content.SetHtmlContent(content);
}
}
", vcthClass.Content);
", vcthClass.Content, ignoreLineEndingDifferences: true);
}

private RazorCodeDocument CreateDocument(string content)
Expand Down