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

Enabling AOT with package lock files is adding a dependency to Microsoft.NET.ILLink.Tasks #38200

Open
nachtjasmin opened this issue Jan 23, 2024 · 29 comments

Comments

@nachtjasmin
Copy link

Describe the bug

When enabling the IsAotCompatible property together with RestorePackagesWithLockFile, dotnet adds Microsoft.NET.ILLink.Tasks as a direct dependency to the lock file. As an effect, even patch updates of the .NET SDK (e.g. from 8.0.100 to 8.0.101) ends up in breaking package builds.

Therefore, even a patch update ends up in being a breaking change, although it should not have an effect on the NuGet package.

To Reproduce

  1. Create a new project, e.g. a console app with dotnet new console.
  2. Enable IsAotCompatible and RestorePackagesWithLockFile in the .csproj.
  3. As a result, the packages.lock.json looks like this (on my system with .NET 8.0.100):
{
  "version": 1,
  "dependencies": {
    "net8.0": {
      "Microsoft.NET.ILLink.Tasks": {
        "type": "Direct",
        "requested": "[8.0.0, )",
        "resolved": "8.0.0",
        "contentHash": "B3etT5XQ2nlWkZGO2m/ytDYrOmSsQG1XNBaM6ZYlX5Ch/tDrMFadr0/mK6gjZwaQc55g+5+WZMw4Cz3m8VEF7g=="
      }
    }
  }
}

An example gist with the project files shows the minimum code required to reproduce it.

Further technical details

Output of dotnet --info

.NET SDK:
 Version:           8.0.100
 Commit:            57efcf1350
 Workload version:  8.0.100-manifests.2d90560f

Runtime Environment:
 OS Name:     fedora
 OS Version:  39
 OS Platform: Linux
 RID:         fedora.39-x64
 Base Path:   /usr/lib64/dotnet/sdk/8.0.100/

.NET workloads installed:
 Workload version: 8.0.100-manifests.2d90560f
There are no installed workloads to display.

Host:
  Version:      8.0.0
  Architecture: x64
  Commit:       5535e31a71

.NET SDKs installed:
  6.0.126 [/usr/lib64/dotnet/sdk]
  8.0.100 [/usr/lib64/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.26 [/usr/lib64/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0 [/usr/lib64/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.26 [/usr/lib64/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0 [/usr/lib64/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/usr/lib64/dotnet]

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Jan 23, 2024
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

nachtjasmin added a commit to anexia/dotnet-e5e that referenced this issue Jan 24, 2024
This is likely a bug which is tracked in the SDK repository:
dotnet/sdk#38200

Closes SIANXSVC-1202
nachtjasmin added a commit to anexia/dotnet-e5e that referenced this issue Jan 24, 2024
This is likely a bug which is tracked in the SDK repository:
dotnet/sdk#38200

Closes SIANXSVC-1202
nachtjasmin added a commit to anexia/dotnet-e5e that referenced this issue Jan 24, 2024
This is likely a bug which is tracked in the SDK repository:
dotnet/sdk#38200

Closes SIANXSVC-1202
nachtjasmin added a commit to anexia/dotnet-e5e that referenced this issue Jan 24, 2024
This is likely a bug which is tracked in the SDK repository:
dotnet/sdk#38200

Closes SIANXSVC-1202
@glen-84
Copy link

glen-84 commented Jan 26, 2024

I'm not doing anything with AOT, but I keep seeing a change to lock files like this:

image

@TomGathercole
Copy link

We're seeing the same issue, which has broken our build pipelines a couple of times in the last few months (We use --locked-mode to restore, which will break if build agents have a newer patch version).

I think we've got a workaround for this by using a global.json with rollforward set to disable:

{
  "sdk": {
    "version": "8.0.202",
    "rollForward": "disable"
  }
}

But we expect this to cause some friction with local development, since Visual Studio will often uninstall older SDK versions when updated. We'll also need to manually update the SDK version rather than relying on the rollforward behaviour.

@TomGathercole
Copy link

Note: We're seeing this in projects with Sdk="Microsoft.NET.Sdk.BlazorWebAssembly", not setting IsAotCompatible directly.

@JL03-Yue JL03-Yue added needs team triage Requires a full team discussion and removed untriaged Request triage from a team member labels Apr 4, 2024
@nagilson nagilson added Area-ILLink and removed needs team triage Requires a full team discussion labels Apr 9, 2024
Copy link
Contributor

@dotnet/illink-contrib a new issue has been filed in the ILLink area, please triage

@dsplaisted
Copy link
Member

@dotnet/illink-contrib Would it make sense to change Microsoft.NET.ILLink.Tasks so that it is consumed as a PackageDownload instead of a PackageReference? We think this would fix the issue where lock files for AOT projects get invalidated for every servicing update.

@sbomer
Copy link
Member

sbomer commented Apr 9, 2024

The package needs to participate in the restore package imports since it contains build logic. I don't think PackageDownload supports that, does it?

@vedion
Copy link

vedion commented Apr 10, 2024

We're seeing the same issue, which has broken our build pipelines a couple of times in the last few months (We use --locked-mode to restore, which will break if build agents have a newer patch version).

I think we've got a workaround for this by using a global.json with rollforward set to disable:

{
  "sdk": {
    "version": "8.0.202",
    "rollForward": "disable"
  }
}

But we expect this to cause some friction with local development, since Visual Studio will often uninstall older SDK versions when updated. We'll also need to manually update the SDK version rather than relying on the rollforward behaviour.

@TomGathercole I also thought this could be a workaround for us but we ran into the issue with "dotnet restore --locked-mode" not respecting global.json:
NuGet/Home#13344

@TomGathercole
Copy link

@vedion You're right disabling rollforward doesn't work. This broke for us again today, and the only solution appears to be to update to the latest SDK as soon as it becomes available (Which seems to happen monthly). I do wonder why more people aren't hitting this - it seems to be something that'll fundamentally happen when using a combination of blazor and --locked-mode, both of which I would expect to be fairly widely used.

@TomGathercole
Copy link

This seems to be related:
#39635

@sliekens
Copy link

sliekens commented May 20, 2024

I have the same problem where locked restore fails when the lock file was generated using a different SDK version. It only works when the machine where the packages are restored is using an identical SDK version.

I suspect not many people use RestorePackagesWithLockFile in combination with IsAotCompatible. Otherwise, much more people would have the same problem.

As a workaround, I added a direct package reference for Microsoft.NET.ILLink.Tasks to my solution, overriding the version provided by the SDK. I have no idea if this workaround is "safe" for multi-targeting scenarios, when you produce AOT builds for multiple target frameworks using the pinned ILLink.Tasks version. I suspect it's not safe.

@smkanadl
Copy link

@sliekens it is exactly the workaround that I am using. When multi-targeting, I override the package per target framework.

@baronfel
Copy link
Member

The Runtime/Linker team strongly recommend not pinning the package and letting the SDK resolve it to ensure compatibility between the runtime in use (which is determined by the SDK version used) and the linker/tools used (which as has been discussed here is also determined by the SDK version in use).

@dsplaisted this scenario is one case where your proposal for source-build would also solve the problem here. Since it seems like implicit packages are not suited to lock files, the SDK could make use of your proposed mechanism for all such implicit packages, removing the need (or even ability) for the user to pin the different versions and letting the SDK control the entire set of used versions.

@smkanadl
Copy link

The Runtime/Linker team strongly recommend not pinning the package and letting the SDK resolve it

@baronfel That is easier said then done. Currently it is the only option. Otherwise I have to choose between AoT or reproducible builds.

@baronfel
Copy link
Member

baronfel commented May 20, 2024

Our position is that you should pin the SDK itself in this case - use global.json to enforce uniformity across your team. If you're not doing that then you are subject to nonreproducible builds in hundreds of other ways, because the compilers themselves (to say nothing of the build logic) can and do change with every SDK. Essentially I think you're missing the forest for the trees - since the lockfile gives you an easy way to see thing that have changed, you're missing all of the other things that you can't see that also change.

@smkanadl
Copy link

@baronfel I get the point here, but pinning the exact SDK version is problematic as I am using Azure DevOps and SDKs get added and removed constantly. And I am again I a situation where I would have to choose between two things that I want to have both. It's a lot of little things (including this issue) which makes things more complicated and frustrating as they should be (from my POV).

@baronfel
Copy link
Member

If you use a global.json, you can control the SDKs in use for a given pipeline using the UseDotNet task in AzDo - there's an analogue for this in GitHub Actions as well (actions/setup-dotnet). This is a supported mechanism that we encourage anyone interested in consistency to use!

@sliekens
Copy link

This is why I like Dev Containers, to containerize all these devtools and make it easy to pin the SDK version. Even my CI pipeline runs in a container, using the same Docker image that I use in VSCode. It's really too bad that VS2022 doesn't support dev containers.

@jaredpar
Copy link
Member

Otherwise I have to choose between AoT or reproducible builds.

Build reproducibility is already tied to the .NET SDK version. For example if you create a simple "Hello World" style app and then build it on the 8.0.100 and 8.0.105 .NET SDK you will get a different binary. That is because the PDB embeds the version of the .NET Runtime, compiler and a few other tools, that were used to create the binary.

It's unclear how deep of reproducible you're looking for here but if it's byte for byte then switching .NET SDK versions is already breaking that.

@TomGathercole
Copy link

Our position is that you should pin the SDK itself in this case - use global.json to enforce uniformity across your team. If you're not doing that then you are subject to nonreproducible builds in hundreds of other ways, because the compilers themselves (to say nothing of the build logic) can and do change with every SDK. Essentially I think you're missing the forest for the trees - since the lockfile gives you an easy way to see thing that have changed, you're missing all of the other things that you can't see that also change.

I don't think this is true. In Azure Pipelines, this breaks for us even with a global.json. (See #38200 (comment) above from @vedion).

The restore seems to break simply from having the newer SDKs available.

@smkanadl
Copy link

Otherwise I have to choose between AoT or reproducible builds.

Build reproducibility is already tied to the .NET SDK version. For example if you create a simple "Hello World" style app and then build it on the 8.0.100 and 8.0.105 .NET SDK you will get a different binary. That is because the PDB embeds the version of the .NET Runtime, compiler and a few other tools, that were used to create the binary.

It's unclear how deep of reproducible you're looking for here but if it's byte for byte then switching .NET SDK versions is already breaking that.

Many thanks for pointing that out. Always good to learn new things. In most cases my requirements for a reproducible build is at dependency/package level. That's why I use the package lock and ended up here :-).

@smkanadl
Copy link

If you use a global.json, you can control the SDKs in use for a given pipeline using the UseDotNet task in AzDo - there's an analogue for this in GitHub Actions as well (actions/setup-dotnet). This is a supported mechanism that we encourage anyone interested in consistency to use!

I considered this some time ago but found that the task is messing with the pathes and would make any pre-installed runtimes "invisible", which I need for testing a multi-targeted library project. But thanks for the hint. I'll give it a try. Maybe it's an option for application projects.

@baronfel
Copy link
Member

@smkanadl you should be able to use the task multiple times in the same pipeline to set up any arrangement of SDKs you need - I've certainly done similar with GitHub actions for my own projects.

@smkanadl
Copy link

@baronfel I know - but contradicts the bonus of having pre-installed runtimes/SDKs and adds quite to the execution time. But fair point 👍

@smkanadl
Copy link

smkanadl commented May 22, 2024

EDIT: Solved it myself. Stupid me forgot to delete the "rollForward": "latestMinor" from the global.json.

@baronfel I gave your suggestions another try and for some cases they work pretty well. Thanks for reminding me!

It get's a bit more troublesome for multi-target cases. I use SDK 8.0.205 for build a trimmable library for net8.0 and net7.0 together with a locked restore.

On my dev machine (win) the lock files contain references to Microsoft.NET.ILLink.Tasks in 8.0.5 for net8.0. For net7.0 it has Microsoft.NET.ILLink.Tasks in version 7.0.100-1.23401.1.

In my CI pipeline I use the latest mcr.microsoft.com/dotnet/sdk:8.0-jammy-amd64 container and install the exact matching SDK 8.0.205.

Unfortunately the restore failes as it tries to restore Microsoft.NET.ILLink.Tasks in version 7.0.100-1.23211.1.

How does the SDK determines the package version for older TFMs? The main difference between local dev and CI seems to be the OS (win/linux) and the fact that I have various other versions of the DSK installed.

@sbomer
Copy link
Member

sbomer commented May 22, 2024

How does the SDK determines the package version for older TFMs?

The version is determined by KnownILLinkPack in <dotnet-install-dir>/sdk/<sdk-version>/Microsoft.NETCoreSdk.BundledVersions.props. Odd that you're not seeing the same Microsoft.NET.ILLink.Tasks version used for identical SDK versions. Could you confirm whether both BundledVersions.props have the same package version?

@smkanadl
Copy link

@sbomer it was actually me being stupid. I had a rollForward to the next minor release in my global.json, so it was 8.0.300 used to generate the locks and 8.0.205 to restore them. Once fixed it worked flawlessly.

Thank you very much for your explanation!

@cataggar
Copy link

cataggar commented Aug 2, 2024

We have one project that is enabling <PublishTrimmed>true</PublishTrimmed> and have the same problems as described above with locking.

I have 8.0.303 installed, so with rollForward set to latestMinor in global.json, it resolves 8.0.303 and adds this in the packages.lock.json:

      "Microsoft.NET.ILLink.Tasks": {
        "type": "Direct",
        "requested": "[8.0.7, )",
        "resolved": "8.0.7",
        "contentHash": "iI52ptEKby2ymQ6B7h4TWbFmm85T4VvLgc/HvS45Yr3lgi4IIFbQtjON3bQbX/Vc94jXNSLvrDOp5Kh7SJyFYQ=="
      },

If I set rollForwrard to disable it resolves 8.0.301 and sets this in the packages.lock.json:

      "Microsoft.NET.ILLink.Tasks": {
        "type": "Direct",
        "requested": "[8.0.6, )",
        "resolved": "8.0.6",
        "contentHash": "E+lDylsTeP4ZiDmnEkiJ5wobnGaIJzFhOgZppznJCb69UZgbh6G3cfv1pnLDLLBx6JAgl0kAlnINDeT3uCuczQ=="
      },

The PublishTrimmed drops the size of the app from 70 MB to MB, but then to we must disable rollForward. If it changes a version for each SDK that is installed, I'm not sure it should be in the lock file.

@sliekens
Copy link

sliekens commented Aug 2, 2024

I like the idea of excluding SDK dependencies from the packages lock file. In essence, global.json already fulfills the role of locking the sdk version and its dependencies like the ILLink stuff.

Currently, you have to pin the SDK version to an exact patch number if you want to use lock files.

If SDK dependencies were removed from the lock file, the behavior would remain exactly the same for those who pinned their SDK version to a patch, while enabling others to pin to a SDK channel, or not even pin the SDK at all.

alexaka1 added a commit to alexaka1/serilog-extensions that referenced this issue Aug 19, 2024
alexaka1 added a commit to alexaka1/serilog-extensions that referenced this issue Aug 19, 2024
* Remove dotnet restore locked mode

Until dotnet/sdk#38200 is resolved

* Update lockfile

* Wildcard sdk version

* Harden runners
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