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

[NETSDKE2E][Workload Sets] Running dotnet workload restore should install the workload set when using global.json #42582

Closed
NicoleWang001 opened this issue Aug 7, 2024 · 4 comments · Fixed by #42606
Assignees
Labels
Area-Workloads untriaged Request triage from a team member

Comments

@NicoleWang001
Copy link
Member

Describe the bug

Running dotnet workload restore` should install the workload set when using global.json

To Reproduce

  1. Install .NET 8.0.400 SDK

  2. Install workload within manifest mode. eg. aspire workload
    image

  3. New global.json and add workloadverison
    dotnet new global.json

    {
      "sdk": {
        "version": "8.0.400",
        "workloadVersion": "8.0.400"
      }
    }
  1. Try to run dotnet workload restore to install the workload set.

Expected Result

Run dotnet workload restore should install the workload set eg. 'dotnet workload list'
image

Actual Result

Unhandled exception:
image

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Workloads untriaged Request triage from a team member labels Aug 7, 2024
@NicoleWang001 NicoleWang001 changed the title [NETSDKE2E][Workload Sets] Running dotnet workload restore` should install the workload set when using global.json [NETSDKE2E][Workload Sets] Running dotnet workload restore should install the workload set when using global.json Aug 7, 2024
@Forgind
Copy link
Member

Forgind commented Aug 7, 2024

Thanks for the bug! This is an interesting chicken-and-egg type problem in that the workload restore command creates and executes a WorkloadInstallCommand that would install the correct workload set, but before it has a chance to do that, we need to know which workloads to install, so we create a new ProjectInstance and execute a special target that hunts for any workloads we might need, but as part of that, it creates a workload resolver and searches for the manifests it needs, which triggered this error. In other words, we need to run Install before we'll have the right workload set, but we can't run Install until we know which workloads to install. We can't figure out the workloads to install without creating a project, and to do that, we have to already have the workload set installed.

I should be able to make a fix to this soon 🙂

@vdanche
Copy link
Member

vdanche commented Oct 15, 2024

@Forgind checked on 9.0.100-rtm.24514.22
workload can be updated via dotnet workload restore when using globaljson even though there is no project. but there is an error message. is it expected?
Image

@Forgind
Copy link
Member

Forgind commented Nov 8, 2024

@vdanche, I think that's right. The global.json can tell you the version (of the workload set and hence all workloads) that we should use, but we still need to know which workloads the user should have. We can't just use what the user currently has because they might not have the set of workloads they need installed yet. So we do need a project for restore to work properly.

We do not need a project for update to work properly. The way restore now works is that it updates then looks for projects. I do think it would be good to have that error message before the update, so perhaps we should change the ordering of that to look for projects, then update, then install the updates. All we would have to do to implement that would be switching the order of these two lines:

new WorkloadUpdateCommand(_result, recorder: recorder, isRestoring: true).Execute();
var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct();

I'd be in favor of making that change. I think the error with the workload set not being installed came from running the target, not from discovering projects, but it's been a bit since I've thought about it; is that correct @dsplaisted? If so, then switching those two lines should be safe.

@dsplaisted
Copy link
Member

It looks like technically we could switch those two lines. I'm not really sure what the best user experience would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Workloads untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants