dotnet clean (--all?) can always fail before a build is run for apps with a RuntimeIdentifier

[nagilson]

As reported by @pimbrouwers in: #29859
If you do something like dotnet new console -lang "F#" and add a RuntimeIdentifier to the project and try dotnet clean, it will fail every time until you run dotnet build, in which case it will work.

This is because clean only cleans the folder for the exact RID, Config, TFM, etc, and there is no runtime config assets.json as that is created by build but never cleaned up, clean only cleans the output directory, and relies on this file to do so. We would maybe like to have an error instead of crash, though we could also design a dotnet clean --all which would clean all output... though we are concerned about deleting people's files.

[BogdanYarotsky]

@nagilson hi! I want to contribute, can I work on this task? Do I understand correctly that dotnet clean must show a meaningful error instead of a crash if assets.json has not been generated yet? And --all flag doesn't exist yet. Is adding it to SDK also a part of this issue? Best regards, Bogdan

[nagilson]

@BogdanYarotsky Thank you for your interest in working on the .NET SDK 😄

dotnet clean --all would not be a good first issue, it is an idea and related to this issue, but they are really 2 separate work items. We would be happy to include a PR that simply errors gracefully with a message.

[BogdanYarotsky]

@nagilson thanks for the quick reply! I'll work on the friendly error message then.

[nagilson]

Not as quick as I'd prefer given the long weekend, but thanks 😉

[BogdanYarotsky]

Hi @nagilson,
I've looked into the issue you described. Here's what I found:

1. The dotnet clean command is forwarded to msbuild.
2. This command runs without error only when project.assets.json targets and the project's RuntimeIdentifier match.
3. project.assets.json is updated during the restore. The implicit restore during dotnet build is the reason why dotnet clean succeeds.
4. The error appears every time RuntimeIdentifier was changed but the project was not restored.

It's necessary to always have a restored project before invoking dotnet clean. Running dotnet clean -restore also works.
Because of this, I propose to add implicit restore during dotnet clean. This will solve the error and simplify the flow. Also, it will be consistent with other dotnet commands like new, build, and pack which also restore the project by default.

Other options would be:

1. Check the targets in project.assets.json before invoking msbuild. This will also work but there will be some logic duplication since dotnet CLI will do the same checks as msbuild.

2. Intercept and parse the output of msbuild. Not a reliable solution because the exit code is not descriptive (1), and error strings may change.

What approach do you think would make the most sense?
I look forward to your feedback to proceed with the implementation.
Thank you for your time 😃

[nagilson]

Hi @BogdanYarotsky,

I am impressed by your investigation -- thank you for taking such a deep dive. Clearly you are very knowledgeable about .NET. 😄

Running dotnet clean -restore is a smart idea, though it would mean a performance hit is taken, and running restore each time is technically against MSBuild's philosophy of only building/restoring one time and only doing so when necessary. There may be other historical reasons for not doing it this way. Doing #1 seems like a wise idea to me and the best one proposed. I think one way you could go about it is to run the implicit restore on clean if and only if it is necessary by looking at the project.assets.json.

We definitely wouldn't take a change with #2 because we want to have reliable behavior.

Based on that, I hope that provides you enough to go off of in the short term; I think @baronfel would also be able to chime in here on if he thinks dotnet clean -restore is a change we can take, and if we'd want it to only be conditional

[nagilson]

So, dotnet restore can require an internet connection, but we don't want clean to require that. I chatted with all of the SDK team and most folks are against adding an implicit restore. I see where you were coming from though and it's a good idea, a lot of things in the SDK are like this unfortunately where the seemingly clear solution breaks other stuff. 😆 We are still in favor of reading the project assets json and erroring / restoring in that condition.

Still thinking if there's a better approach.

[BogdanYarotsky]

Thank you for the detailed response 🙂
Makes total sense to me. I'll try to compose a pull request in the following week.

[nagilson]

Awesome, thanks and I look forward to it 😄