Skip to content

Support publishing self-contained applications.#151

Merged
eerhardt merged 3 commits intodotnet:masterfrom
eerhardt:SelfContainedPublish
Sep 20, 2016
Merged

Support publishing self-contained applications.#151
eerhardt merged 3 commits intodotnet:masterfrom
eerhardt:SelfContainedPublish

Conversation

@eerhardt
Copy link
Member

I also fixed a minor issue where we were always writing the compile assemblies into the published deps.json file.

Fix #21 - There is nothing to do for "self-contained" apps at build time, since it is a publish time decision.

@brthor @livarcocc @nguerrera

/cc @dotnet/project-system

@eerhardt eerhardt force-pushed the SelfContainedPublish branch from 4e1fc9e to 39033cd Compare September 20, 2016 03:36
Copy link
Contributor

@natidea natidea left a comment

Choose a reason for hiding this comment

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

LGTM

using System.IO;
using System.Linq;
using NuGet.Packaging.Core;
using NuGet.ProjectModel;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: do you need this? don't see it in use

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, LockFileItem

<!-- TODO: https://github.com/dotnet/sdk/issues/143 - find a way to default $(PublishDir) before Microsoft.Common.CurrentVersion.targets defaults it -->
<_ShouldSetPublishDir Condition=" '$(PublishDir)' == '' or '$(PublishDir)' == '$(OutputPath)app.publish\'">true</_ShouldSetPublishDir>
<PublishDir Condition=" '$(_ShouldSetPublishDir)' == 'true' ">$(OutDir)$(_PublishDirName)\</PublishDir>
<PublishDir Condition=" '$(_ShouldSetPublishDir)' == 'true' and '$(RuntimeIdentifier)' != '' ">$(OutDir)$(RuntimeIdentifier)\$(_PublishDirName)\</PublishDir>
Copy link
Contributor

Choose a reason for hiding this comment

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

I had thought you were going with <OutDir>/publish/<RID> ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was, but I got cold feet.

The issue that made me go back to this folder structure is because when you publish "portable" and then publish "self-contained" (or vice versa), your "portable" publish output will contain the RID-specific publish outputs as well:

/<OutDir>
  /publish
    App.dll
    App.deps.json
    ...
    /win10-x64
      App.dll
      App.deps.json
      ...

Thus, you can't just copy the "publish" folder for portable, you need to exclude all the RID specific publish folders. So I felt it would be better to keep the current folder structure, since it will be a better user experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay I see, the xcopy experience isn't as good


<PropertyGroup>
<_DotNetHostExecutableName>dotnet</_DotNetHostExecutableName>
<_DotNetHostExecutableName Condition="'$(OS)'=='Windows_NT'">$(_DotNetHostExecutableName).exe</_DotNetHostExecutableName>
Copy link
Contributor

Choose a reason for hiding this comment

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

For Cross Publish you should check the RID here rather than the current OS

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops - nice catch. I'll fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 It kind of sucks the rid is an opaque string, I guess the best to do here is look for the prefix win?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the best to do here is look for the prefix win?

Yes, that was going to be my approach. Is there a better approach? We could add a C# task that takes in a RID and returns the OS value for the RID, but that seems a bit heavy-weight for something so simple.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah agreed, all of the rid handling logic I've seen is just doing a split anyways, we don't need c# for that

@brthor
Copy link
Contributor

brthor commented Sep 20, 2016

After the RID comment :shipit:

@eerhardt eerhardt merged commit 0d3dd5d into dotnet:master Sep 20, 2016
@eerhardt eerhardt deleted the SelfContainedPublish branch September 20, 2016 20:40
nguerrera pushed a commit that referenced this pull request Oct 10, 2016
Support publishing self-contained applications.
sbomer pushed a commit to sbomer/sdk that referenced this pull request Sep 19, 2017
Add NETStandard assets only for NETStandard projects
mmitche pushed a commit to mmitche/sdk that referenced this pull request Jun 5, 2020
GangWang01 pushed a commit to GangWang01/sdk that referenced this pull request Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement VerifyCoreClrPresenceInPackageGraph and copy a DotNetHost for self-contained apps

4 participants