Skip to content

Conversation

stanciuadrian
Copy link
Contributor

@stanciuadrian stanciuadrian commented Oct 9, 2021

Fixes #6689

Context

Validation was failing during the parameter name extraction because of an off-by-1 offset & length.
The original code was hard-coded for a 1-char switch indicator.

Changes Made

Code handles both 1 and 2 character(s) switch indicators.

Testing

ExtractSwitchParameters + GatherCommandLineSwitches

Notes

@stanciuadrian stanciuadrian marked this pull request as draft October 10, 2021 06:25
@rainersigwald
Copy link
Member

I have some issues running tests locally.

Want to start up a Discussion thread about what's going wrong for you?

@stanciuadrian
Copy link
Contributor Author

stanciuadrian commented Oct 11, 2021

I'm running VS2022 and the second execution of build.cmd -msbuildEngine dotnet -test fails with 216 errors. The [same] error is about assemblies used by another process that cannot be copied to the target directory.

There are a lot of orphaned processes:

image

and I have to kill them all for testing to work again.

I'll use the github CI for tests.

@stanciuadrian stanciuadrian marked this pull request as ready for review October 12, 2021 06:27
@stanciuadrian stanciuadrian changed the title WIP double dash validation fix double dash validation fix Oct 12, 2021
Copy link
Contributor

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

I'm assuming you also tested this with your case? Maybe change your test to use --maxcupcount:8 instead of --p:c=d?

@stanciuadrian
Copy link
Contributor Author

Indeed, I tested with maxcpucount. The test was updated to use this flag.

@stanciuadrian stanciuadrian requested a review from Forgind October 15, 2021 18:40
Copy link
Contributor

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

Great! LGTM, thanks!

@ladipro ladipro added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Oct 21, 2021
@ladipro ladipro merged commit b4aea91 into dotnet:main Oct 22, 2021
@stanciuadrian stanciuadrian deleted the switchIndicatorsLength branch October 23, 2021 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Internal MSBuild error when --maxcpucount:8 supplied in cmdline (dotnet build)

4 participants