Publish to Single-File #3132

swaroop-sridhar · 2019-04-17T09:16:03Z

This change implements support for publishing apps to a single file.

dotnet publish /p:PublishSingleFile=true causes the contents of the "original" publish directory to a single file in the actual publish directory
Files marked with the meta-data <ExcludeFromSingleFile>false<ExcludeFromSingleFile> are left in the publish directory unbundled. This includes PDB files by default
PDB files can be bundled into the single file by setting /p:IncludeSymbolsInSingleFile=true

Publishing to a single file requires publishing wrt a RID using an apphost, because the generated file is the platform-specific AppHost executable with embedded dependencies.

swaroop-sridhar · 2019-04-17T09:21:04Z

@nguerrera @fadimounir @sbomer please review.
CC: @ericstj

swaroop-sridhar · 2019-04-17T18:37:59Z

@dotnet-bot test this please

sbomer · 2019-04-17T19:53:06Z

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

@@ -85,6 +86,9 @@ Copyright (c) .NET Foundation. All rights reserved.
    -->
  <Target Name="PrepareForPublish">

+    <NETSdkError Condition="'$(_IsExecutable)' != 'true'"


Should this be checking PublishSingleFile as well?

Oh you already fixed it. :)

Thanks @sbomer; as you know, I saw the feedback over your shoulder :)

swaroop-sridhar · 2019-04-17T21:17:23Z

Thanks @sbomer. Once the ILLinker change (#3125) goes through, I'll add another test for Illink + R2R + Bundled app.

fadimounir · 2019-04-17T22:13:47Z

src/Tasks/Common/Resources/Strings.resx

+    <comment>{StrBegin="NETSDK1098: "}</comment>
+  </data>
+  <data name="CannotHaveSingleFileWithoutExecutable" xml:space="preserve">
+    <value>NETSDK1099: Publishing to a single-file is currently only supported for executable applications.</value>


currently [](start = 54, length = 9)

I feel we should remove "currently". If we support this in the future, we can delete this error message

fadimounir · 2019-04-17T22:18:42Z

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

+    <PropertyGroup>
+      <!-- Handle the case where IncludeSymbolsInSingleFile may not be defined -->
+      <EmbedPDBs Condition="'$(IncludeSymbolsInSingleFile)' == 'true'">true</EmbedPDBs>
+      <EmbedPDBs Condition="'$(IncludeSymbolsInSingleFile)' != 'true'">false</EmbedPDBs>


We should also handle the .pdb/.map files created by crossgen here. We can do this later (I want to make some changes to the R2R related stuff first). Please file an issue to track this.

https://github.com/dotnet/core-setup/issues/5903

This change implements support for publishing apps to a [single file](https://github.com/dotnet/designs/blob/master/accepted/single-file/design.md). * ``dotnet publish /p:PublishSingleFile=true`` causes the contents of the "original" publish directory to a single file in the actual publish directory * Files marked with the meta-data ``<ExcludeFromSingleFile>false<ExcludeFromSingleFile>`` are left in the publish directory unbundled. This includes PDB files by default * PDB files can be bundled into the single file by setting ``/p:IncludeSymbolsInSingleFile=true`` Publishing to a single file requires publishing wrt a RID using an apphost, because the generated file is the platform-specific AppHost executable with embedded dependencies.

The property to trigger R2R compilation was changed from "Ready2Run" to "PublishReadyToRun". Update this in a test case.

swaroop-sridhar · 2019-04-18T20:19:14Z

In this change 1cc9352 @fadimounir I updated a test case to use the new name PublishReadyToRun for triggering R2R compilation, now that #3134 is checked in.

nguerrera

Looks good overall. Some minor feedback.

nguerrera · 2019-04-18T22:17:43Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs

+            foreach(var item in FilesToBundle)
+            {
+                fileSpec.Add(new FileSpec(sourcePath: item.ItemSpec,
+                                          bundleRelativePath: item.GetMetadata("RelativePath")));


Suggested change

bundleRelativePath: item.GetMetadata("RelativePath")));

bundleRelativePath: item.GetMetadata(MetadataKeys.RelativePath)));

Not sure if I got that exactly right, but there's a class with constants for metadata. Use that.

nguerrera · 2019-04-18T22:18:00Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs

+            Bundler bundler = new Bundler(AppHostName, OutputDir, EmbedPDBs, ShowDisgnosticOutput);
+            var fileSpec = new List<FileSpec>(FilesToBundle.Length);
+
+            foreach(var item in FilesToBundle)


Suggested change

foreach(var item in FilesToBundle)

foreach (var item in FilesToBundle)

nit: spacing

nguerrera · 2019-04-18T22:19:04Z

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

    ============================================================
    -->
  <Target Name="_CopyResolvedFilesToPublishPreserveNewest"
          DependsOnTargets="_ComputeResolvedFilesToPublishTypes"
          Inputs="@(_ResolvedFileToPublishPreserveNewest)"
          Outputs="@(_ResolvedFileToPublishPreserveNewest->'$(PublishDir)%(RelativePath)')">

+    <ItemGroup>
+      <_ResolvedUnBundledFileToPublishPreserveNewest 


Suggested change

<_ResolvedUnBundledFileToPublishPreserveNewest

<_ResolvedUnbundledFileToPublishPreserveNewest

nit on casing. Un is not a word on its own.

(Obviously, rename all references to it too.)

nguerrera · 2019-04-18T22:25:05Z

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

+    <PropertyGroup>
+      <!-- Handle the case where IncludeSymbolsInSingleFile may not be defined -->
+      <EmbedPDBs Condition="'$(IncludeSymbolsInSingleFile)' == 'true'">true</EmbedPDBs>
+      <EmbedPDBs Condition="'$(IncludeSymbolsInSingleFile)' != 'true'">false</EmbedPDBs>


EmbedPDBs is a pretty generic name since there is also a concept of embeddign PDB in dll (DebugType=Embedded). You can avoid having a new name, by defaulting IncludeSymbolsInSingleFile to false when empty. This is very idiomatic in msbuild, you will see it all over the place.

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

I don't even think you need the comment if you do that, it is pretty obvious what this handles.

Then pass EmbedPdbs="$(IncludeSymbolsInSingleFile)

nguerrera · 2019-04-18T22:25:49Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs

+        [Required]
+        public string AppHostName { get; set; }
+        [Required]
+        public bool EmbedPDBs { get; set; }


Suggested change

public bool EmbedPDBs { get; set; }

public bool IncludeSymbols { get; set; }

I think it is worth using the same terminology throughout, and the public facing name is symbols for this feature.

nguerrera · 2019-04-18T22:26:02Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs

+
+        protected override void ExecuteCore()
+        {
+            Bundler bundler = new Bundler(AppHostName, OutputDir, EmbedPDBs, ShowDisgnosticOutput);


Suggested change

Bundler bundler = new Bundler(AppHostName, OutputDir, EmbedPDBs, ShowDisgnosticOutput);

var bundler = new Bundler(AppHostName, OutputDir, EmbedPDBs, ShowDisgnosticOutput);

nit: type is obvious

nguerrera · 2019-04-18T22:27:59Z

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

+          Condition="'$(PublishSingleFile)' == 'true'"
+          DependsOnTargets="_ComputeFilesToBundle" 
+          Inputs="@(_FilesToBundle)"
+          Outputs="$(PublishDir)\$(AssemblyName)$(_NativeExecutableExtension)">


Suggested change

Outputs="$(PublishDir)\$(AssemblyName)$(_NativeExecutableExtension)">

Outputs="$(PublishDir)$(AssemblyName)$(_NativeExecutableExtension)">

We ensure trailing slash and you can see this is how PublishDir is used elsewhere. This is another common idiom in msbuild, it prevents accidental writing to root of disk if something is undefined. I once deleted my entire hard drive this way so I stick to this now :)

Funny story, a similar issue caused CoreFx build perf to tank once, the cause $(SomeProperty)\**\*.extension. Someone renamed SomeProperty and CI passed...

swaroop-sridhar · 2019-04-18T23:08:13Z

@nguerrera: I've made the corrections you suggested in this commit: 9246cad
Thanks.

nguerrera · 2019-04-18T23:09:53Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs

+            var bundler = new Bundler(AppHostName, OutputDir, IncludeSymbols, ShowDisgnosticOutput);
+            var fileSpec = new List<FileSpec>(FilesToBundle.Length);
+
+            Array.ForEach(FilesToBundle, 


Why did you change this to Array.ForEach? I liked the foreach loop better.

OK @nguerrera, I pushed 25c2b42 which revers back to the foreach() loop.

nguerrera · 2019-04-18T23:28:20Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs

+        [Required]
+        public bool ShowDisgnosticOutput { get; set; }
+
+        protected override void ExecuteCore()


I love how delightfully simple this task ended up being. :)

nguerrera · 2019-04-19T00:10:11Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs

+        [Required]
+        public string OutputDir { get; set; }
+        [Required]
+        public bool ShowDisgnosticOutput { get; set; }


Suggested change

public bool ShowDisgnosticOutput { get; set; }

public bool ShowDiagnosticOutput { get; set; }

Just spotted a typo

Thanks, fixed it in 671de2d

swaroop-sridhar · 2019-04-19T00:23:45Z

@dotnet-bot test dotnet-sdk-public-ci

@nguerrera

Thanks @nguerrera

ericstj

Nice!

…build 20191007.2 (dotnet#3132) - Microsoft.NET.Sdk.Razor - 3.0.1-servicing.19507.2

swaroop-sridhar mentioned this pull request Apr 17, 2019

Add support for Single-file publishing #3074

Closed

swaroop-sridhar force-pushed the bundle-lib branch 3 times, most recently from d84a6a1 to 3bd01b7 Compare April 17, 2019 21:07

sbomer approved these changes Apr 17, 2019

View reviewed changes

fadimounir reviewed Apr 17, 2019

View reviewed changes

fadimounir approved these changes Apr 17, 2019

View reviewed changes

swaroop-sridhar added 3 commits April 18, 2019 12:41

Update an error message.

456c13f

Change the R2R property in a test

1cc9352

The property to trigger R2R compilation was changed from "Ready2Run" to "PublishReadyToRun". Update this in a test case.

swaroop-sridhar force-pushed the bundle-lib branch from ff8ef28 to 1cc9352 Compare April 18, 2019 20:17

nguerrera reviewed Apr 18, 2019

View reviewed changes

Address feedback by @nguerrera

9246cad

nguerrera reviewed Apr 18, 2019

View reviewed changes

Change an Array.Foreach() to foreach() loop.

25c2b42

nguerrera reviewed Apr 18, 2019

View reviewed changes

nguerrera approved these changes Apr 18, 2019

View reviewed changes

nguerrera reviewed Apr 19, 2019

View reviewed changes

swaroop-sridhar force-pushed the bundle-lib branch from 77e4d36 to 671de2d Compare April 19, 2019 00:18

Fix a typo in a variablename

4bb3c69

Thanks @nguerrera

swaroop-sridhar force-pushed the bundle-lib branch from 671de2d to 4bb3c69 Compare April 19, 2019 00:24

ericstj approved these changes Apr 19, 2019

View reviewed changes

nguerrera merged commit aacf124 into dotnet:master Apr 19, 2019

dsplaisted mentioned this pull request Apr 23, 2019

Ignore NuGet warning NU5123 (for long paths) #3172

Merged

swaroop-sridhar mentioned this pull request Jan 30, 2020

Single-file: Handle PDB/Map files generated by R2R dotnet/runtime#3553

Closed

dsplaisted pushed a commit to dsplaisted/sdk that referenced this pull request Feb 19, 2020

Update dependencies from https://github.com/aspnet/AspNetCore-Tooling …

f9c4ba0

…build 20191007.2 (dotnet#3132) - Microsoft.NET.Sdk.Razor - 3.0.1-servicing.19507.2

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

Publish to Single-File #3132

Publish to Single-File #3132

swaroop-sridhar commented Apr 17, 2019

swaroop-sridhar commented Apr 17, 2019

swaroop-sridhar commented Apr 17, 2019

sbomer Apr 17, 2019

sbomer Apr 17, 2019

swaroop-sridhar Apr 17, 2019

swaroop-sridhar commented Apr 17, 2019

fadimounir Apr 17, 2019

fadimounir Apr 17, 2019

swaroop-sridhar Apr 17, 2019

swaroop-sridhar commented Apr 18, 2019

nguerrera left a comment

nguerrera Apr 18, 2019

nguerrera Apr 18, 2019

nguerrera Apr 18, 2019

nguerrera Apr 18, 2019

nguerrera Apr 18, 2019

nguerrera Apr 18, 2019

nguerrera Apr 18, 2019

nguerrera Apr 18, 2019

ericstj Apr 19, 2019

swaroop-sridhar commented Apr 18, 2019

nguerrera Apr 18, 2019

swaroop-sridhar Apr 18, 2019

nguerrera Apr 18, 2019

nguerrera Apr 19, 2019

swaroop-sridhar Apr 19, 2019 •

edited

Loading

swaroop-sridhar commented Apr 19, 2019

ericstj left a comment

	bundleRelativePath: item.GetMetadata("RelativePath")));
	bundleRelativePath: item.GetMetadata(MetadataKeys.RelativePath)));

	foreach(var item in FilesToBundle)
	foreach (var item in FilesToBundle)

	<_ResolvedUnBundledFileToPublishPreserveNewest
	<_ResolvedUnbundledFileToPublishPreserveNewest

	public bool EmbedPDBs { get; set; }
	public bool IncludeSymbols { get; set; }

	Bundler bundler = new Bundler(AppHostName, OutputDir, EmbedPDBs, ShowDisgnosticOutput);
	var bundler = new Bundler(AppHostName, OutputDir, EmbedPDBs, ShowDisgnosticOutput);

	Outputs="$(PublishDir)\$(AssemblyName)$(_NativeExecutableExtension)">
	Outputs="$(PublishDir)$(AssemblyName)$(_NativeExecutableExtension)">

	public bool ShowDisgnosticOutput { get; set; }
	public bool ShowDiagnosticOutput { get; set; }

Publish to Single-File #3132

Publish to Single-File #3132

Conversation

swaroop-sridhar commented Apr 17, 2019

swaroop-sridhar commented Apr 17, 2019

swaroop-sridhar commented Apr 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swaroop-sridhar commented Apr 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swaroop-sridhar commented Apr 18, 2019

nguerrera left a 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 Apr 18, 2019

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 Apr 19, 2019 • edited Loading

Choose a reason for hiding this comment

swaroop-sridhar commented Apr 19, 2019

ericstj left a comment

Choose a reason for hiding this comment

swaroop-sridhar Apr 19, 2019 •

edited

Loading