Single-File: Update Bundler Invocation. #11586

swaroop-sridhar · 2020-05-08T09:55:11Z

This change updates the SDK to:

Pass the correct target-OS settings to the bundler
Copy files excluded by the Bundler to the publish directory.
- In .net 5, single-file apps will leave native binaries unbundled in the publish directory by default.
- Since Single-file bundles are processed in the framework, hostpolicy and hostfxr DLLs cannot themselves be in the bundle (until statically linked singlefilehost is supported).

This change updates the SDK to: * Pass the correct target-OS settings to the bundler * Copy files excluded by the Bundler to the publish directory. * In .net 5, single-file apps will leave native binaries unbundled in the publish directory by default. * Since Single-file bundles are processed in the framework, hostpolicy and hostfxr DLLs cannot themselves be in the bundle (until statically linked singlefilehost is supported).

swaroop-sridhar · 2020-05-13T00:06:49Z

CC: @MSLukeWest

swaroop-sridhar · 2020-05-13T02:26:09Z

Regarding PublishedItemsOutputGroup and FilesCopiedToPublishDir items, when the list is computed without actually building the output, the list of files computed may be inaccurate in the following situations:

PublishTrimmed=true
PublishSingleFile=true without IncludeAllContentInSingleFile=true,

In these situations, It is not possible to accurately compute the list of published files without inspecting the file. For example, ILLink walks the IL in assemblies to determine reachability and drops assemblies that are unnecessary. (CC: @sbomer)

The constraints for PublishedItemsOutputGroup required by WPF are:

Design time build where the build files don't exist:
- Computed list of items doesn't reflect final list,
- The targets that need the build files to exist get skipped using a condition
- The computed list correctly identifies the KeyOutput.
Full build: Computed list of items is correct

@MSLukeWest verified that the constraints are met after the change.

swaroop-sridhar · 2020-05-13T02:26:19Z

@dsplaisted @wli3 I'd like to get this fix in for Preview5. Thanks.

wli3 · 2020-05-13T17:26:29Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

    <GenerateBundle FilesToBundle="@(_FilesToBundle)"
                    AppHostName="$(PublishedSingleFileName)"
                    IncludeSymbols="$(IncludeSymbolsInSingleFile)"
+                    IncludeNativeLibraries="$(IncludeNativeLibrariesInSingleFile)"


There are 4 new inputs. But there in only 1 set of tests.

Also if IncludeNativeLibraries is not currently used, let's not add it today

OK Thanks @wli3, I've fixed this now.

Hi @wli3, the PR tests have passed after the fix you recommended. Can you please take a look again? Thanks.

wli3 · 2020-05-14T16:44:58Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

+      <TraceSingleFileBundler Condition="'$(TraceSingleFileBundler)' == ''">false</TraceSingleFileBundler>
+      <IncludeSymbolsInSingleFile Condition="'$(IncludeSymbolsInSingleFile)' == ''">false</IncludeSymbolsInSingleFile>
+      <IncludeNativeLibrariesInSingleFile Condition="'$(IncludeNativeLibrariesInSingleFile)' == ''">false</IncludeNativeLibrariesInSingleFile>
+      <IncludeAllContentInSingleFile Condition="'$(IncludeAllContentInSingleFile)' == ''">false</IncludeAllContentInSingleFile>


IncludeAllContentInSingleFile not used?

wli3 · 2020-05-14T16:45:18Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

+    <PropertyGroup>
+      <TraceSingleFileBundler Condition="'$(TraceSingleFileBundler)' == ''">false</TraceSingleFileBundler>
+      <IncludeSymbolsInSingleFile Condition="'$(IncludeSymbolsInSingleFile)' == ''">false</IncludeSymbolsInSingleFile>
+      <IncludeNativeLibrariesInSingleFile Condition="'$(IncludeNativeLibrariesInSingleFile)' == ''">false</IncludeNativeLibrariesInSingleFile>


Thanks, removed the initialization.

wli3 · 2020-05-14T16:50:13Z

src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishASingleFileApp.cs

+        [InlineData("netcoreapp3.1", true)]
+        [InlineData("netcoreapp5.0", false)]
+        [InlineData("netcoreapp5.0", true)]
+        public void It_runs_single_file_apps(string targetFramework, bool selfContained)


Could you change the assertion(assert different files are present or not) to reflect:

In .net 5, single-file apps will leave native binaries unbundled in the publish directory by default.
Since Single-file bundles are processed in the framework, hostpolicy and hostfxr DLLs cannot themselves be in the bundle (until statically linked singlefilehost is supported).

@wli3 I've added a new test It_leaves_host_components_unbundled_when_necessary for this.

I thought about this, but I originally didn't add the test here because it is a temporary idiosyncrasy of the host/hostmodel, and not a requirement for the SDK.
The host-tests do test for this:
https://github.com/dotnet/runtime/blob/master/src/installer/test/Microsoft.NET.HostModel.Tests/AppHost.Bundle.Tests/BundleExtractToSpecificPath.cs#L58
https://github.com/dotnet/runtime/blob/master/src/installer/test/Microsoft.NET.HostModel.Tests/Helpers/BundleHelper.cs#L63-L66

If this is covered in runtime we can skip that. These are very slow tests * 6.

Maybe just have one test to ensure

ExcludedFiles = FilesToBundle.Zip(fileSpec, (item, spec) => (spec.Excluded) ? item : null).Where(x => x != null).ToArray(); works. As in assert one file in one tfm that should be exclude not being there.

And is all these TFM combination necessary? Again, these are really slow tests. There is no SDK logic have if-else about TFM. Only one test in SDK is enough. Other coverage should be in dotnet/Runtime

OK @wli3. I'll keep the one interesting case of .net5 self-contained in the It_rewrites_the_apphost_for_non_single_file_publish test.

I think it is useful to have all the cases in the It_runs_single_file_apps case.

@wli3 This is done in 85c42de

…en_necessary test

wli3 · 2020-05-15T00:04:21Z

src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishASingleFileApp.cs

+        }
+
+        //  Core MSBuild only due to https://github.com/dotnet/sdk/issues/4244
+        [CoreMSBuildOnlyTheory]


are all these combinations necessary too?

Yes, for It_runs_single_file_apps I think we should have all combinations.
This guards against incompatibility between SDK and hostmodel that creates single-file apps that don't run correctly. I'm planning to add this test to integration tests in the installer branch, as recommended by Daniel.

swaroop-sridhar · 2020-05-15T00:12:40Z

@wli3 I published the PR against Preview 5 branch here: #11659

swaroop-sridhar · 2020-05-15T01:07:53Z

/azp run dotnet-sdk-public-ci (Build Darwin Build_Release)

azure-pipelines · 2020-05-15T01:07:59Z

No pipelines are associated with this pull request.

swaroop-sridhar force-pushed the params branch 6 times, most recently from 74183f6 to 0523052 Compare May 12, 2020 23:29

swaroop-sridhar force-pushed the params branch from 0523052 to b78bd59 Compare May 12, 2020 23:55

swaroop-sridhar requested a review from dsplaisted May 13, 2020 00:04

MSLukeWest approved these changes May 13, 2020

View reviewed changes

wli3 reviewed May 13, 2020

View reviewed changes

Address feedback by @wli3

8f99f05

wli3 suggested changes May 14, 2020

View reviewed changes

Address more feedback from @wli3

a2c068e

swaroop-sridhar force-pushed the params branch from 232004e to a2c068e Compare May 14, 2020 22:26

Reduce the number of cases for It_leaves_host_components_unbundled_wh…

85c42de

…en_necessary test

wli3 approved these changes May 15, 2020

View reviewed changes

wli3 reviewed May 15, 2020

View reviewed changes

wli3 merged commit 7d3fecc into dotnet:master May 15, 2020

swaroop-sridhar mentioned this pull request May 16, 2020

Support Single-File Apps in .NET 5 dotnet/runtime#36590

Closed

30 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Single-File: Update Bundler Invocation. #11586

Single-File: Update Bundler Invocation. #11586

swaroop-sridhar commented May 8, 2020 •

edited

Loading

swaroop-sridhar commented May 13, 2020

swaroop-sridhar commented May 13, 2020

swaroop-sridhar commented May 13, 2020

wli3 May 13, 2020

swaroop-sridhar May 13, 2020

swaroop-sridhar May 13, 2020

wli3 May 14, 2020

wli3 May 14, 2020

swaroop-sridhar May 14, 2020

wli3 May 14, 2020

swaroop-sridhar May 14, 2020

wli3 May 14, 2020

wli3 May 14, 2020

swaroop-sridhar May 14, 2020

swaroop-sridhar May 14, 2020

wli3 May 15, 2020

swaroop-sridhar May 15, 2020

swaroop-sridhar commented May 15, 2020

swaroop-sridhar commented May 15, 2020

azure-pipelines bot commented May 15, 2020

Single-File: Update Bundler Invocation. #11586

Single-File: Update Bundler Invocation. #11586

Conversation

swaroop-sridhar commented May 8, 2020 • edited Loading

swaroop-sridhar commented May 13, 2020

swaroop-sridhar commented May 13, 2020

swaroop-sridhar commented May 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swaroop-sridhar commented May 15, 2020

swaroop-sridhar commented May 15, 2020

azure-pipelines bot commented May 15, 2020

swaroop-sridhar commented May 8, 2020 •

edited

Loading