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

--no-restore should specify -restore:false when calling MSBuild to override response file arguments #39821

Open
jeffkl opened this issue Mar 27, 2024 · 3 comments · May be fixed by #40579
Open
Assignees
Labels
Milestone

Comments

@jeffkl
Copy link
Contributor

jeffkl commented Mar 27, 2024

Is your feature request related to a problem? Please describe.

When running commands that accept --no-restore, a repository with a Directory.Build.rsp will still inject its own -restore argument. I think when I specify something like:

dotnet test --no-restore

That the call to MSBuild should include the -restore:false command-line argument to override the arguments in my Directory.Build.rsp. Otherwise, I have to specify it myself which looks funny:

dotnet test --no-restore -restore:false

Describe the solution you'd like

Commands that use RestoringCommand could check for the option and set the MSBuild command-line argument.

For example:

bool noRestore = parseResult.GetResult(BuildCommandParser.NoRestoreOption) is not null;
BuildCommand command = new(
msbuildArgs,
noRestore,
msbuildPath);

bool noRestore = parseResult.GetResult(BuildCommandParser.NoRestoreOption) is not null;

+ if (noRestore)
+ {
+     msbuildArgs.Add("-restore:false");
+ }

BuildCommand command = new(
    msbuildArgs,
    noRestore,
    msbuildPath);

Or perhaps the argument itself could be marked to forward in such a way?

public static readonly CliOption<bool> NoRestoreOption = CommonOptions.NoRestoreOption;

- public static readonly CliOption<bool> NoRestoreOption = CommonOptions.NoRestoreOption;
+ public static readonly CliOption<bool> NoRestoreOption = new ForwardedOption<bool>("--no-restore").FowardedAs("-restore:false");

Additional context

I'm trying to streamline the build in the NuGet client repo and because we have a Directory.Build.rsp which injects -restore so that when you run MSBuild.exe you get the implicit restore, other commands where I don't want to restore I have to use both --no-restore and -restore:false.

I'd be willing to contribute if this a change that would be accepted.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-CLI untriaged Request triage from a team member labels Mar 27, 2024
@baronfel
Copy link
Member

This seems useful to me - the second approach with the ForwardedOption seems more natural to the way the codebase is designed right now.

@nagilson nagilson added needs team triage Requires a full team discussion and removed untriaged Request triage from a team member labels Apr 9, 2024
@marcpopMSFT
Copy link
Member

Triage: @jeffkl this change seems reasonable and the forwarded options as Chet described would be a good way to do it. Feel free to put up a PR into main.

@marcpopMSFT marcpopMSFT removed the needs team triage Requires a full team discussion label Apr 16, 2024
@marcpopMSFT marcpopMSFT added this to the 9.0.1xx milestone Apr 16, 2024
@jeffkl
Copy link
Contributor Author

jeffkl commented Apr 30, 2024

Thanks, please review at #40579

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