xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.48k stars 514 forks source link

Add support for `dotnet publish --no-build` #19132

Open jwosty opened 1 year ago

jwosty commented 1 year ago

Migrated from: https://github.com/dotnet/sdk/issues/35705


Using a very specific combination of dotnet CLI arguments and msbuild properties, .NET 7 (and lower?) would allow you to run a publish with --no-build. This no longer works when I upgrade my solution to use .NET 8, but only specifically breaks for Xamarin projects (i.e. has net8.0-macos target framework).

Steps to Reproduce

  1. Clone the following repo and checkout the dotnet8/dotnet8 branch: https://github.com/jwosty/DotnetMacosNoBuildBug/tree/dotnet8/dotnet8
  2. Run build.sh.
  3. Observe the following failure:
    /usr/local/share/dotnet/packs/Microsoft.macOS.Sdk/13.3.8825-net8-rc1/targets/Xamarin.Shared.Sdk.targets(303,3): error : Both RuntimeIdentifier and RuntimeIdentifiers were passed on the command line, but only one of them can be set at a time. [/Users/jwostenberg/Code/DotnetMacosNoBuildBug/src/TestApp/TestApp.csproj]

It works if you change everything to use .NET 7 (or checkout the dotnet7/dotnet7 branch).

So while this technically a breaking change, this makes sense. However, I cannot find another workaround. There's another branch, dotnet8/rids-in-proj, where I've attempted a workaround by setting RuntimeIdentifiers in the project file instead. I get the following warning:

/usr/local/share/dotnet/packs/Microsoft.macOS.Sdk/13.3.8825-net8-rc1/targets/Xamarin.Shared.Sdk.targets(299,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/jwostenberg/Code/DotnetMacosNoBuildBug/src/TestApp/TestApp.csproj]

which is fine and I can live with, but also the following failure:

/usr/local/share/dotnet/sdk/8.0.100-rc.1.23463.5/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.targets(235,5): error NETSDK1085: The 'NoBuild' property was set to true but the 'Build' target was invoked. [/Users/jwostenberg/Code/DotnetMacosNoBuildBug/src/TestLibrary/TestLibrary.csproj]

Interestingly, this behavior happens on .NET 7 too (I must have been setting it via the CLI already to work around that). The same behavior happens even if you don't even set RuntimeIdentifiers at all for build and publish, specifically.

Additionally, the publish succeeds if you change the FrameworkTarget from net8.0-macos to net8.0 (however it doesn't produce an app executable).

Environment

CLI, no IDE.

Version information ``dotnet --info`` output: ``` .NET SDK: Version: 8.0.100-rc.1.23463.5 Commit: e7f4de8816 Runtime Environment: OS Name: Mac OS X OS Version: 13.5 OS Platform: Darwin RID: osx-arm64 Base Path: /usr/local/share/dotnet/sdk/8.0.100-rc.1.23463.5/ .NET workloads installed: [macos] Installation Source: SDK 8.0.100-rc.1 Manifest Version: 13.3.8825-net8-rc1/8.0.100-rc.1 Manifest Path: /usr/local/share/dotnet/sdk-manifests/8.0.100-rc.1/microsoft.net.sdk.macos/WorkloadManifest.json Install Type: FileBased Host: Version: 8.0.0-rc.1.23419.4 Architecture: arm64 Commit: 92959931a3 RID: osx-arm64 .NET SDKs installed: 6.0.302 [/usr/local/share/dotnet/sdk] 6.0.403 [/usr/local/share/dotnet/sdk] 6.0.404 [/usr/local/share/dotnet/sdk] 6.0.405 [/usr/local/share/dotnet/sdk] 6.0.407 [/usr/local/share/dotnet/sdk] 7.0.100 [/usr/local/share/dotnet/sdk] 7.0.101 [/usr/local/share/dotnet/sdk] 7.0.102 [/usr/local/share/dotnet/sdk] 7.0.200 [/usr/local/share/dotnet/sdk] 7.0.202 [/usr/local/share/dotnet/sdk] 7.0.400 [/usr/local/share/dotnet/sdk] 8.0.100-preview.7.23376.3 [/usr/local/share/dotnet/sdk] 8.0.100-rc.1.23463.5 [/usr/local/share/dotnet/sdk] .NET runtimes installed: Microsoft.AspNetCore.App 6.0.7 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.11 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.12 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.13 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.15 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.1 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.2 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.3 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.4 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.10 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 8.0.0-preview.7.23375.9 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 8.0.0-rc.1.23421.29 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.NETCore.App 6.0.7 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.11 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.12 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.13 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.15 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.1 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.2 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.3 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.4 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.10 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.0-preview.7.23375.6 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.0-rc.1.23419.4 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Other architectures found: x64 [/usr/local/share/dotnet/x64] registered at [/etc/dotnet/install_location_x64] Environment variables: Not set global.json file: /Users/jwostenberg/Code/Bug8/global.json Learn more: https://aka.ms/dotnet/info Download .NET: https://aka.ms/dotnet/download ```

Build Logs

.NET 8 - dotnet publish binlog: publish8.binlog.zip

.NET 7 - dotnet publish binlog for comparison: publish7.binlog.zip

Example Project (If Possible)

https://github.com/jwosty/DotnetMacosNoBuildBug/tree/dotnet8/dotnet8

jwosty commented 1 year ago

Comment migrated from old issue


Just realized that in the dotnet8/dotnet8 and dotnet7/dotnet7 branches, restore should only be setting <RuntimeIdentifiers> (plural), and build/publish should only set --runtime (singular). Doing this gets rid of the Both RuntimeIdentifier and RuntimeIdentifiers were passed on the command line, but only one of them can be set at a time error. However I still get a The 'NoBuild' property was set to true but the 'Build' target was invoked.

For reference, here are the project files and build script with that change in place:

build.sh:

#!/usr/bin/env bash
set -e

cd "$(dirname "$0")"

proj="src/TestApp/TestApp.csproj"
rid="osx-x64"
rids="osx-x64;osx-arm64"
cfg="Release"

run() {
    echo "$(pwd)> $*"
    "$@"
}

run dotnet restore $proj -p "RuntimeIdentifiers=\"$rids\""
run dotnet build $proj --configuration $cfg --no-restore --runtime $rid --self-contained true
run dotnet publish $proj --configuration $cfg --no-restore --no-build --runtime $rid --self-contained true --output publish/$rid

TestApp.csproj:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>net8.0-macos</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
        <OutputType>Exe</OutputType>
        <ApplicationId>com.example.TestApp</ApplicationId>
    </PropertyGroup>

    <ItemGroup>
      <ProjectReference Include="..\TestLibrary\TestLibrary.csproj" />
    </ItemGroup>

</Project>

TestLibrary.csproj:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>net8.0-macos</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
        <LangVersion>11</LangVersion>
    </PropertyGroup>

</Project>
jwosty commented 1 year ago

OK, so looking into this more:

The error happens in .NET 8 because it ends up invoking the ResolveProjectReferences target for TestLibrary.csproj (which is what trigger the spurious build). For comparison, the .NET 7 publish never invokes ResolveProjectReferences.

Under .NET 8, $(BuildDependsOn) seems to end up with this value:

_VerifyValidOutputType;

_WarnRuntimeIdentifiersClash;
_ComputePublishTrimmed;
BuildOnlySettings;
_CollectBundleResources;
_PackLibraryResources;
_UnpackLibraryResources;

BeforeBuild;
CoreBuild;
AfterBuild
;
_CreateAppBundle;
Codesign;
CreateIpa;
_CreateInstaller;
Archive;
;

While .NET 7 lacks BeforeBuild, CoreBuild, and AfterBuild:

_CollectBundleResources;
_RunRidSpecificBuild;
_CompileEntitlements;
_DetectAppManifest;
_ReadAppManifest;
_CopyResourcesToBundle;
_CreateMergedAppBundle;
Codesign;
CreateIpa;
_CreateInstaller;
Archive;

It seems that the discrepancy is because .NET 8 is now taking this "single-RID" path:

<BuildDependsOn Condition="'$(RuntimeIdentifiers)' == '' And '$(_IsMultiRidBuild)' != 'true'">

instead of whatever it used to do, because <RuntimeIdentifiers> is now being cleared by this logic here:

  <PropertyGroup Condition="'$(RuntimeIdentifiers)' != '' And '$(RuntimeIdentifier)' != '' ">
    <!-- Clear RuntimeIdentifier -->
    <RuntimeIdentifier />
    <!-- Check if RuntimeIdentifier was cleared. If it wasn't it was set on the command line, and in that case, try to clear RuntimeIdentifiers instead -->
    <_RuntimeIdentifiersClashMessage Condition="'$(RuntimeIdentifier)' != ''">RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file.</_RuntimeIdentifiersClashMessage>
    <RuntimeIdentifiers Condition="'$(RuntimeIdentifier)' != ''" />
    <!-- If we couldn't clear RuntimeIdentifiers either, then both were set on the command line. This is an error -->
    <_RuntimeIdentifiersClashErrorMessage  Condition="'$(RuntimeIdentifier)' != '' And '$(RuntimeIdentifiers)' != ''">Both RuntimeIdentifier and RuntimeIdentifiers were passed on the command line, but only one of them can be set at a time.</_RuntimeIdentifiersClashErrorMessage>
  </PropertyGroup>

If I revert that part of Xamarin.Shared.Sdk.DefaultItems.Targets to before the offending commit (see https://github.com/xamarin/xamarin-macios/pull/18396), the bug no longer manifests and the project successfully publishes:

  <!-- If the old-style variables aren't set, figure it out using RuntimeIdentifier. -->
  <PropertyGroup Condition="'$(RuntimeIdentifiers)' != '' And '$(RuntimeIdentifier)' != '' ">
    <!-- Check if both RuntimeIdentifier and RuntimeIdentifiers are set, in which case we clear RuntimeIdentifier -->
    <!-- Also set a variable that says if this happens, so that we can show a warning later -->
    <_RuntimeIdentifiersClashMessage>Both RuntimeIdentifier and RuntimeIdentifiers are set. The value for RuntimeIdentifier will be ignored.</_RuntimeIdentifiersClashMessage>
    <!-- Clear RuntimeIdentifier -->
    <RuntimeIdentifier />
  </PropertyGroup>
rolfbjarne commented 1 year ago

I can reproduce this, but the problem is that we don't really support publishing without building (because we basically implemented publishing as "dotnet build /p:CreatePackage=true"). It should be fixable, but it's a significant amount of work, so I'm not sure when (if) we'd be able to do it.

Is there any particular reason you can't build when publishing (i.e. not pass --no-build to. dotnet publish)?

jwosty commented 1 year ago

@rolfbjarne Ah, I see that now. I was operating under the assumption that it worked the same as vanilla .NET and that in .NET 6 and 7 it just happens to not error out.

I just wanted to be able to have these run as separate CI steps (i.e. Build->Test->Publish) without performing duplicate work, so I can optimize the CI minute usage. Having just Publish->Test steps will the job done; it's just that it performs unnecessary work in builds with failing unit tests (i.e. it didn't have to generate the app package and build various assets). I'm also using Fable in my app, and it's Fable build is slower than the .NET F#/C# compiler, so it's nice to have the option to separate those steps as well and perform them after .NET runtime test suites (again in order to fail early). Another factor is that in GitHub Actions, MacOS minutes are the most expensive ones.

Anyways, this is all just optimization and not a blocker, so it's good to know that I should just revert to building and publishing in one CI step for the macOS build for now.

jwosty commented 1 year ago

I've been digging around in some of the Xamarin MSBuild scripts while investigating this. Out of curiosity, where does the main difficulty lie? Couldn't a no-build publish run just the _CreateInstaller target?

There might even be an argument for making app bundle generation (and everything related to that, i.e. AOT, code signing, etc) be a part of publish rather than of build, so that a customer could choose to only build .NET assemblies for unit testing purposes. Although I would understand if this isn't really feasible due to backwards compatibility.

rolfbjarne commented 1 year ago

I've been digging around in some of the Xamarin MSBuild scripts while investigating this. Out of curiosity, where does the main difficulty lie? Couldn't a no-build publish run just the _CreateInstaller target?

The main difficulty is really that the publish logic was implemented with the assumption that build would be executed first; it might be trivial to implement, but changing assumptions has a tendency of uncovering unwelcome surprises.

There might even be an argument for making app bundle generation (and everything related to that, i.e. AOT, code signing, etc) be a part of publish rather than of build

Yes, this is a rather philosophical debate we've had internally: what does "dotnet build" and "dotnet publish" really mean? What's the expected output of each? We went with a definition where "dotnet build" = something that can be used/executed, and for iOS/tvOS that means a complete app bundle (nothing less than a complete app bundle can be installed in the simulator or on device). On macOS (and Mac Catalyst), the lines are a bit fuzzier in that you can run executables that aren't in an app bundle, but since we have to support app bundles anyways (because that's what ends up being published), it turns out it's a lot easier for us to only support app bundles (having two modes: standalone executable + app bundles adds a lot of maintenance cost), and then we're back to the iOS/tvOS case an app bundle is our only useable output.

Also note that when you hit F5 in the IDE, the IDE will do the equivalent of a "dotnet build" (AFAIK there's no way to do debug the output of "dotnet publish"). This also implies that "dotnet build" should produce something executable.