Skip to content

Fix .cmd scripts help arguments#1043

Merged
safern merged 2 commits into
dotnet:masterfrom
safern:FixCmdHelp
Dec 19, 2019
Merged

Fix .cmd scripts help arguments#1043
safern merged 2 commits into
dotnet:masterfrom
safern:FixCmdHelp

Conversation

@safern
Copy link
Copy Markdown
Member

@safern safern commented Dec 19, 2019

In windows if we call any of the subset helper scripts, coreclr.cmd, libraries.cmd or installer.cmd with -h, -help or -? we get arcade's help which is miss leading in some arguments like, -arch is shown as -platform. The reason for that is because subset scripts pass -subsetCategory <subset> as the first argument when calling build.cmd and in build.cmd, in order to print help, we check for %~1.

Also without this change, it doesn't match Unix behavior, as if we call coreclr.sh-h then we get the expected output of our eng\build usage output.

@safern safern requested a review from a team December 19, 2019 01:23
Copy link
Copy Markdown
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work with /? (or -?) which was the reason why we introduced the helper checks in the cmd file.

@safern
Copy link
Copy Markdown
Member Author

safern commented Dec 19, 2019

That won't work with /? (or -?)

It actually works with /?, the only one that didn't work is -?... I just added support for it in the .cmd scripts.

@safern
Copy link
Copy Markdown
Member Author

safern commented Dec 19, 2019

@jkoritzinsky @ViktorHofer could you please review again whenever you have time?

@safern
Copy link
Copy Markdown
Member Author

safern commented Dec 19, 2019

Thanks @jkoritzinsky.

will go ahead and merge since @ViktorHofer is OOF already. Viktor let me know if you have any concerns and I will address them as a follow up PR.

@safern safern merged commit 0a29c61 into dotnet:master Dec 19, 2019
@safern safern deleted the FixCmdHelp branch December 19, 2019 19:38
jkotas added a commit that referenced this pull request Dec 21, 2019
Comment thread build.cmd
Comment thread eng/build.ps1
@@ -57,6 +58,11 @@ if ($MyInvocation.InvocationName -eq ".") {
exit 0
}
Copy link
Copy Markdown
Member

@ViktorHofer ViktorHofer Jan 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we still dot sourcing this script anywhere? If not, we can remove the if block.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm I'm not sure about that. I don't think so. Would searching for -Command be enough?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm relatively sure that this is dead code and you can just remove it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Will remove it then.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants