Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling trimming per assembly with trimming does change the assembly #1669

Closed
vitek-karas opened this issue Dec 4, 2020 · 15 comments
Closed

Comments

@vitek-karas
Copy link
Member


Issue moved from dotnet/runtime#45598


From @NN--- on Friday, December 4, 2020 4:50:39 PM

Description

Specifying TrimmerRootAssembly and even IsTrimmable false does change the package file.

Configuration

.NET Core 3.1 or .NET 5.0

project.csproj:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <TrimmerRootAssembly Include="SMBLibrary" />

    <PackageReference Include="SMBLibrary" Version="1.4.3">
      <IsTrimmable>false</IsTrimmable>
    </PackageReference>
  </ItemGroup>
</Project>

program.cs

class Program { public static void Main() {} }

Run:
dotnet publish project.csproj -c Release -r win-x64 -o publish /p:PublishSingleFile=true /p:PublishTrimmed=true

Check file inside: obj\Release\netcoreapp3.1\win-x64\linked\SMBLibrary.dll

The file size is not the same as original file in
%userprofile%.nuget\packages\smblibrary\1.4.3\lib\netstandard2.0\SMBLibrary.dll

If you don't specify PublishTrimmed=true then the file is preserved but the resulting output is much bigger.

The expectation is that TrimmerRootAssembly preserves original file as described in documentation

Regression?

Not a regression.

@vitek-karas
Copy link
Member Author


Issue moved from dotnet/runtime#45598

  • Please respond to @msftbot[bot].

From @msftbot[bot] on Friday, December 4, 2020 4:50:43 PM

Tagging subscribers to this area: @vitek-karas, @agocke, @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Specifying TrimmerRootAssembly and even IsTrimmable false does change the package file.

Configuration

.NET Core 3.1 or .NET 5.0

project.csproj:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <TrimmerRootAssembly Include="SMBLibrary" />

    <PackageReference Include="SMBLibrary" Version="1.4.3">
      <IsTrimmable>false</IsTrimmable>
    </PackageReference>
  </ItemGroup>
</Project>

program.cs

class Program { public static void Main() {} }

Run:
dotnet publish project.csproj -c Release -r win-x64 -o publish /p:PublishSingleFile=true /p:PublishTrimmed=true

Check file inside: obj\Release\netcoreapp3.1\win-x64\linked\SMBLibrary.dll

The file size is not the same as original file in
%userprofile%.nuget\packages\smblibrary\1.4.3\lib\netstandard2.0\SMBLibrary.dll

If you don't specify PublishTrimmed=true then the file is preserved but the resulting output is much bigger.

The expectation is that TrimmerRootAssembly preserves original file as described in documentation

Regression?

Not a regression.

Author: NN---
Assignees: -
Labels:

area-AssemblyLoader-coreclr, untriaged

Milestone: -

@vitek-karas
Copy link
Member Author


Issue moved from dotnet/runtime#45598


From @vitek-karas on Friday, December 4, 2020 5:07:38 PM

This is currently expected. The trimmer still processes the assembly, it just doesn't remove any type/method from it in that mode. What it will do is remove assembly references to assemblies which are removed from the output. In this case it removes reference to mscorlib and netstandard and replaces it with references to .NET Core assemblies.

Does the fact that assembly is modified (but not actually trimmed) cause problems for your application?

@vitek-karas
Copy link
Member Author


Issue moved from dotnet/runtime#45598


From @NN--- on Friday, December 4, 2020 5:38:24 PM

It doesn't cause any issue but there are plenty of licenses which require you to state changes even if you just modify the date.
Some licenses may even disallow such usage if you change the original file.
For the safety I would like to have the original file in the output.

https://tldrlegal.com/search?reverse=true&must[]=52c5e04809728f9723000009

@vitek-karas
Copy link
Member Author


Issue moved from dotnet/runtime#45598


From @vitek-karas on Friday, December 4, 2020 6:04:01 PM

Thanks for pointing that out!

@vitek-karas
Copy link
Member Author

/cc @LakshanF @sbomer

This is effectively similar to issues with C++/CLI which also must not be modified at all.

@sbomer
Copy link
Member

sbomer commented Dec 4, 2020

I believe we need a linker action that will analyze an assembly, but is guaranteed not to modify it. We would need to decide how to handle:

  • Removed references (maybe it is fine to leave dangling references when they are unused)
  • Removed type forwarders (this could invalidate typerefs - so we need to either keep type forwarders referenced from used code in the assembly, or emit an error if there is a used reference to a removed type forwarder)

This action on its own should not root the entire assembly. My read is that copy was originally designed to behave this way, but it has drifted:
https://github.com/mono/linker/blob/b24d0e663d5bf4772b0896e2bc3dab3391fae744/src/linker/Linker/AssemblyAction.cs#L36-L38

@vitek-karas
Copy link
Member Author

Agreed - we need new assembly action. I think we will have to keep facades, but somehow figure out which parts of the facade are necessary and "ignore" the rest. So basically mark the facade, but don't root anything in it and also make sure we copy it verbatim. I don't want to rewrite facades, that's probably just trouble (and size-wise doesn't help that much, and means nothing for AOT anyway).

@vitek-karas
Copy link
Member Author

@NN--- just curious: what if I want to use AOT on code like that? Is that permissible? (I know we should ask lawyers, but maybe you have experience with that kind of thing already).

@NN---
Copy link

NN--- commented Dec 4, 2020

That's great question :)
I assume that such licenses doesn't allow AOT as well if they don't allow any modification.

Also license such as LGPL doesn't allow static linking if you want to preserve your own license.
It means that AOT is also not an option in such case unless you want your program license to be compatible with the package license.

@marek-safar
Copy link
Contributor

I agree we'll probably have to add something like copyoriginal action but not sure how well we need to handle all the odd cases which lead us to change copy to do more.

@NN---
Copy link

NN--- commented Dec 5, 2020

If I understand correctly LGPL 3.0 as long as you state library changes and use it dynamically you don't need your program to be licensed as LGPL.
AOT does static linking and self-contained application with extracted files produces dynamic linking.

If it was possible to make AOT for all libraries except one it could solve this in the most efficient way.

@vitek-karas
Copy link
Member Author

Thanks for the comment @NN--- : Partial AOT is possible in some cases (that's basically what for example ReadyToRun is), but it's not really possible in other cases - platforms which don't allow JIT for example. In some cases there are interpreters (big perf penalty, but at least it runs), but not always.

Honestly even JIT is somewhat weird in these cases - it takes the code of the library, translates it to native (just like AOT does) and then runs it. The only difference is that it doesn't write it to disk... but I'm not a lawyer, maybe that does make a difference.

@NN---
Copy link

NN--- commented Dec 7, 2020

@vitek-karas I want to focus on my original issue which is that single-file application changes the original file and there is no option to disable it.

@vitek-karas
Copy link
Member Author

Sorry - you're right. I got carried away.

@mateoatr
Copy link
Contributor

This was solved by fixing the copy action behavior -- copy assemblies will now remain unmodified. This option can be used to instruct linker to analyze but keep the assembly as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants