Do a two-phase publish to mimic VS behavior

[baronfel]

I wanted to take a run at this because it just makes sense to me.

Publish profiles work in VS from a technical level because they operate in two phases - one to locate and parse build properties from a specified publish profile pubxml, and the second to run the build's Publish target with those properties applied. This allows users to put properties that are specific to that publish configuration in a single file, not cluttering their project file.

This PR emulates this behavior. It

• does an evaluation, adhering to any user-specified CLI options that might be relevant to MSBuild evaluation,
  • LastUsedBuildConfiguration
  • Configuration
  • Platform
  • TargetFramework
  • TargetFrameworks
  • RuntimeIdentifier
• checks the results of that evaluation to determine a) if a profile was imported at all and b) if so, the path to the profile that was chosen
• loads the properties from the profile that was chosen
• appends those properties to the end of the MSBuild publish invocation, so that those properties take precedence (since MSBuild CLI properties are a latest-takes-precedence ordering)

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[baronfel]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[baronfel]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[nagilson]

While I haven't done the side-by-side comparison, I imagine that it would be faster to loop through the properties in the propertiesToForwardFromProfile, instead of loading in every property on the project into memory

[nagilson]

Note that this will be another +~18-25% performance regression in time to publish which I don't think we want. If we were to do this, I think it would be better to create something like a singleton wrapper for pre-eval project access which only ever instantiates or pre-evaluates the project once.

[nagilson]

Technically this is a breaking change because you could have a publish profile in your directory that currently isn't being used by the .NET SDK, and so the Config, Rid, etc, could suddenly change with this change. IMO it would be better to take in .NET 8, but maybe this qualifies for a 300 band break. Either way, we would need a breaking change doc.

[nagilson]

My main concern here is the GetTargetedProject call. In NET 8 we choose an arbitrary project in the solution as opposed to the executable project. This means the publish profile import could be non-deterministic unless further changes are made for the 8.0 code, and those changes, while not an extreme amount of work, also will need to be very well thought-through to prevent bugs

[baronfel]

interesting - so the API changed from "if I'm called with a solution I return null" to "if I'm called with a solution I pick a project" in 8? That's good to know - for publish specifically we'd want to not run at all if the argument is a solution, so I can change this call to explicitly check that condition before calling GetTargetedProject.

[nagilson]

Yeah. Docstring is updated here:

sdk/src/Cli/dotnet/ReleasePropertyProjectLocator.cs

Line 120 in 0e9ba20

{

[nagilson]

I would prefer to use TryGetProjectInstance as race conditions could occur and cause failure if the project file is changed on disk in between this and the above lines of code

[nagilson]

It would be preferable to add a test showing that publish profile proeprties are imported correctly, and for properties like configuration, they actually are changed (so DebugSymbols and Optimize also change, not just configuration)

[nagilson]

It would be a big win to support publish profiles in the SDK. Thanks for this investigation. I think more work would be required to insert this into production, but great work so far.

[baronfel]

After some consideration I'm going to retarget this to 8.0 (but it will still need to be documented as a breaking change!).

[Balkoth]

Is anywhere concrete and complete documentation available on what properties are working from the pubxml file? I only found this which only gives away that there might be other properties which are not supported.

[baronfel]

@Balkoth those docs were added in part because of the investigation required to get this PR going - VS uses PubXml properties for both building the assets to publish, as well as potentially orchestrating pushing those assets to another location. The .NET CLI will only do the first of these (building the assets to publish). Because of this, the properties listed are the only ones we care about at the current time.

In general, all properties from the pubxml will be included in the build - but due to timing/ordering of the Import statement that includes the pubxml file into the build those properties may not be available when you expect. That's why I made this PR in the first place. If you're not seeing a property 'take' in the way you expect from a pubxml, I'd create a binlog and inspect it with the binlog viewer to verify the ordering/usage.

[Balkoth]

Because of this, the properties listed are the only ones we care about at the current time.

In the linked document i can't see any properties which you care about, only ones that "aren't supported"

In general, all properties from the pubxml will be included in the build - but due to timing/ordering of the Import statement that includes the pubxml file into the build those properties may not be available when you expect. That's why I made this PR in the first place. If you're not seeing a property 'take' in the way you expect from a pubxml, I'd create a binlog and inspect it with the binlog viewer to verify the ordering/usage.

I am "simply" trying to bring publishing of some projects off from client machines to our build server and build them from the command line with dotnet build.

Sample FolderProfile.pubxml of one of our projects:

<Project>
  <PropertyGroup>
    <Configuration>Release</Configuration>
    <Platform>Any CPU</Platform>
    <PublishDir>bin\publish\</PublishDir>
    <PublishProtocol>FileSystem</PublishProtocol>
    <TargetFramework>net7.0-windows</TargetFramework>
    <RuntimeIdentifier>win-x64</RuntimeIdentifier>
    <SelfContained>false</SelfContained>
    <PublishSingleFile>true</PublishSingleFile>
    <PublishReadyToRun>false</PublishReadyToRun>
    <IncludeNativeLibrariesForSelfExtract>true</IncludeNativeLibrariesForSelfExtract>
  </PropertyGroup>
</Project>

Naive me was thinking i could just do it like this in a PowerShell script, but the document says otherwise.

$targetProjectFile = "C:\Source\Project\Project.csproj"
$targetOutputDir = "C:\Binaries\Project"
dotnet publish $targetProjectFile --configuration Release --output $targetOutputDir /p:PublishProfile=FolderProfile

Is it even possible to pass all the arguments to dotnet publish manually so that it does exactly what Visual Studio does when publishing through the UI?

[baronfel]

Sorry - I'm referring to this list from the docs you linked:

LastUsedBuildConfiguration
Configuration
Platform
LastUsedPlatform
TargetFramework
TargetFrameworks
RuntimeIdentifier
RuntimeIdentifiers

These are the properties that VS reads from the PublishProfile (if present) and applies to the build to create the assets to publish.

If you wanted to recreated the VS logic yourself, you'd do essentially what I did in this PR:

• compute the publish profile to read values from
• read the property values from the publish profile file that impact the build (the list above)
• apply those properties as 'global properties' to the build when calling dotnet publish using -p again and again: -p Configuration=Release -p RuntimeIdentifier=linux-x64, and so on. as well as including the PublishProfile property as you did in your example

Hope that clarifies - let me know if there's anything else I can do to help.

[Balkoth]

So setting properties with \p has a different effect than setting them directly as parameters to dotnet publish?

Is there really a difference between these to scripts?

$targetProjectFile = "C:\Source\Project\Project.csproj"
$targetOutputDir = "C:\Binaries\Project"
dotnet publish $targetProjectFile -c Release -o $targetOutputDir -f net7.0-windows -r win-x64 --no-self-contained /p:PublishSingleFile=true /p:PublishReadyToRun=false /p:IncludeNativeLibrariesForSelfExtract=true

$targetProjectFile = "C:\Source\Project\Project.csproj"
$targetOutputDir = "C:\Binaries\Project"
dotnet publish $targetProjectFile -c Release -o $targetOutputDir --no-self-contained /p:PublishSingleFile=true /p:PublishReadyToRun=false /p:IncludeNativeLibrariesForSelfExtract=true /p:TargetFramework=net7.0-windows /p:RuntimeIdentifier=win-x64

[baronfel]

There are minor differences but they should be functionally equivalent. I used the 'generic' form of passing properties like Configuration and Runtime because I was assuming you were going to do a mechanical translation of "property from pubxml file" to "property to pass on the command line". You can of course use the more specific flags as well.

We're getting a bit off of the topic of this PR, which should be focused on code review/etc - if you're having specific issues with a publish command, can you open a new issue for triage and further discussion?

[Balkoth]

You are correct and put me on the right track, so thanks for all your replies. If any issues arise i will open another issue.

[marcpopMSFT]

Per Chet, this needs to be redone so closing for now.