search

dotnet / linker

389 stars 126 forks source link

TrimMode is not included in linker up-to-date check #2102

Open agocke opened 3 years ago

agocke commented 3 years ago

This means that changing the trim options can cause MSBuild to reuse files from previous trim operations with different settings.

marek-safar commented 3 years ago

Why is the issue here, isn't this SDK issue?

agocke commented 3 years ago

The linker targets + task are owned by the linker, even if some of the code is in the SDK repo. It's easier to default to tracking all linker issues in mono/linker

sbomer commented 3 years ago

Setting properties in the project file should invalidate the up-to-date check here https://github.com/dotnet/sdk/blob/58ce20521426792c7d491a9c4b0ef8ad1598b432/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L115, but I'm not sure if there's a great way to handle this for properties passed on the command-line. I guess we could add a step that writes certain properties to a file on each build, but that seems like an unfortunate workaround. Maybe I'm missing some MSBuild knowledge - is there a better way to make global properties part of the up-to-date check? @rainersigwald

rainersigwald commented 3 years ago

No, no easy way to do this (https://github.com/dotnet/msbuild/issues/701 tracks inventing one but no work is currently planned for it). What we do for CoreCompile is hash a set of compiler inputs and write that to disk (if changed), then use that as compiler input.

https://github.com/dotnet/msbuild/blob/e9593e841ab16e9792894267548e10b17c98c535/src/Tasks/Microsoft.Common.CurrentVersion.targets#L3520-L3550

Here's where DefineConstants was added to that, which is analogous to your scenario: dotnet/msbuild#3978.

sbomer commented 1 year ago

For future reference: EnableCompressionInSingleFile has the same problem (and there are probably many instances).