Skip to content

Replace custom targets for generating assembly attributes and source control info by using SDK features #7504

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

Merged
merged 7 commits into from
Feb 13, 2019

Conversation

natemcmaster
Copy link
Contributor

@natemcmaster natemcmaster commented Feb 12, 2019

Part of aspnet/BuildTools#618 and preparation for #7280

Changes:

  • This disables the sourcelink and attribute generation targets from Internal.AspNetCore.Sdk
  • Replace source linke with Microsoft.SourceLink.{GitHub, Vsts.Git} packages
  • Replace Internal.AspNetCore.Sdk task which generates file version by using the values already computed in version.props
  • Move some properties from Directory.Build.props to CSharp.Common.props if they only apply to C# projects
  • Port some settings from Internal.AspNetCore.Sdk into Directory.Build.props. Except for some build tasks still used, we're almost ready to completely remove this package.
  • add workaround for Microsoft.SourceLink does not work with Razor components #7503

@natemcmaster natemcmaster requested a review from dougbu February 12, 2019 19:32
@Eilon Eilon added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Feb 12, 2019
Nate McMaster added 4 commits February 12, 2019 15:08
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Components changes look good.

…ported by project types which support source link
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Questions, questions…

</PropertyGroup>

<PropertyGroup Condition=" '$(OfficialBuildId)' != '' AND '$(BuildNumberSuffix)' == '' ">
<PropertyGroup Condition=" '$(OfficialBuildId)' != '' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean we can only build -t000 packages locally now? Ugh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can still override whatever you want, e.g. build.cmd /p:IsOfficialBuild=true, build.cmd /p:BuildNumberSuffix=1234, etc. This change is mean to remove the part of this code that uses TeamCity environment variables.

<!-- Additional assembly attributes are already configured to include the source revision ID. -->
<IncludeSourceRevisionInInformationalVersion>false</IncludeSourceRevisionInInformationalVersion>

<DisableSourceLink Condition="'$(MSBuildProjectExtension)' == '.zipproj' OR '$(MSBuildProjectExtension)' == '.debproj' OR '$(MSBuildProjectExtension)' == '.rpmproj'">true</DisableSourceLink>
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems odd to me that *.proj, *.javaproj, *.npmproj, *.pkgproj, *.shfxproj , and *.vcxproj aren't included in this condition. Why not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The project types listed were excluded because adding the PackageReference causes NuGet to fail because they don't import Microsoft.Common.targets. We probably could disable source link on javaproj/npmproj, but I left them on enabled because they weren't causing issues.

…only imported by project types which support source link
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants