Skip to content

Enable link during portable publish#263

Merged
marek-safar merged 8 commits into
dotnet:masterfrom
sbomer:portablePublish
Feb 27, 2018
Merged

Enable link during portable publish#263
marek-safar merged 8 commits into
dotnet:masterfrom
sbomer:portablePublish

Conversation

@sbomer
Copy link
Copy Markdown
Member

@sbomer sbomer commented Feb 26, 2018

This enables the linker to run during a portable publish.

During portable publish, the framework is shared. The set of assemblies to be excluded from publish is determined by a platform manifest that's part of the NETCore.App package. The portable publish set can include references to parts of the framework that are filtered by this mechanism, and aren't exposed as reference assemblies (for example, platform-specific assemblies that we don't want developers to compile against directly). With tolerate resolution errors, the linker can tolerate such missing references.

However, we still want to pass ReferencePath to the linker to prevent a large number of resolution warnings for parts of the framework that don't get published. This doesn't cover everything (as noted above), but limits the number of warnings. These reference assemblies are made part of the linker "platform", with a default action of skip during portable publish. This way the ref assemblies are used to prevent resolution errors, but they aren't part of the linker output.

Portable publish also depends on find more native dependencies.

In addition to the mentioned changes, this includes integration tests that exercise portable publish on helloworld, webapi, and musicstore.

@erozenfeld, @ericstj PTAL.
/cc @richlander @Petermarcu

JosephTremoulet and others added 7 commits February 26, 2018 09:43
Include anything in ReferencePath that is not part of the published set
to make sure the linker can resolve references in the published set.
Include them as "platform" libraries since the link action for these
should always be 'Skip' when doing a portable publish.
Factor the integration tests so that project setup is done once per
project, allowing multiple testcases to invoke the linker on the same
project without excess overhead. Also add testcases exercising
portable publish for helloworld and musicstore.
We want to avoid including in _ReferencedLibraries any ref assemblies
from ReferencePath that also exist with the same filename as publish
assemblies. Otherwise, the ref assemblies become part of the linker
"platform" assemblies, and they get skipped during portable
publish. This would be incorrect if ReferencePath includes any ref
assemblies for non-shared libraries.
Copy link
Copy Markdown
Member

@erozenfeld erozenfeld left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of nits.

<!-- Portable publish: need to supply the linker with any needed
reference assemblies as well.

We want to exclude assemblies that are going to be published
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you mean "assemblies that are not going to be published" ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I meant implementation assemblies that are going to be published, and also exist as a reference assembly. That sentence was unclear - I'll just remove it entirely.

RedirectStandardOutput = true,
RedirectStandardError = true,
};
outputHelper.WriteLine($"from caller working directory {Environment.CurrentDirectory}");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indentation is off. Also I would remove "from" and add ":" after directory for consistency with the message on line 78.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Those spaces always seem to sneak in... thanks! Updating the message per your suggestion as well.

@Petermarcu
Copy link
Copy Markdown
Member

Cool, tell me when I can reference a new package to try it out.

In the current linker, only things in netcoreapp are linked. In this world, will linking happen on the things in my app?

@erozenfeld
Copy link
Copy Markdown
Member

By default, things in used assemblies in your app are untouched (unused assemblies are deleted). If you want things in your app to be linked, you can override this with /p:UsedApplicationAssemblyAction=Link.

Copy link
Copy Markdown
Member

@erozenfeld erozenfeld left a comment

Choose a reason for hiding this comment

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

LGTM

@marek-safar marek-safar merged commit 16d3016 into dotnet:master Feb 27, 2018
tkapin pushed a commit to tkapin/runtime that referenced this pull request Jan 31, 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.

5 participants