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

Fix invalid PGO optimization path on Nuget cache containing spaces. #90729

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

c272
Copy link
Contributor

@c272 c272 commented Aug 17, 2023

Since CMD does not strip quotes from arguments passed in with quotes by default, if a Nuget package cache path containing a space is passed to build-runtime.cmd, it will pass it into an additional CMake argument with an additional set of quotes.

This causes string semantics to be disabled upon reaching the second opening quote, thus separating the cache path into two separate arguments incorrectly, resulting in the PGO data optimization pointing to an invalid path.

This patch fixes this issue by simply removing the redundant quotes when accepting the arguments in build-runtime.cmd.

Since CMD does not strip quotes from arguments passed in with quotes
by default, if a Nuget package cache path containing a space is
passed to build-runtime.cmd, it will pass it into an additional CMake
argument with an additional set of quotes.

This causes string semantics to be disabled upon reaching the
second opening quote, thus separating the cache path into two
separate arguments incorrectly, resulting in the PGO data optimization
pointing to an invalid path.

This patch fixes this issue by simply removing the redundant quotes
when accepting the arguments in build-runtime.cmd.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 17, 2023
@ghost
Copy link

ghost commented Aug 17, 2023

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Since CMD does not strip quotes from arguments passed in with quotes by default, if a Nuget package cache path containing a space is passed to build-runtime.cmd, it will pass it into an additional CMake argument with an additional set of quotes.

This causes string semantics to be disabled upon reaching the second opening quote, thus separating the cache path into two separate arguments incorrectly, resulting in the PGO data optimization pointing to an invalid path.

This patch fixes this issue by simply removing the redundant quotes when accepting the arguments in build-runtime.cmd.

Author: c272
Assignees: -
Labels:

area-Infrastructure-coreclr, community-contribution

Milestone: -

@a74nh
Copy link
Contributor

a74nh commented Aug 22, 2023

We (Arm) saw this issue when doing testing directly on a Windows On Arm box.
I'm not familiar with .cmd syntax, but this patch fixes it for us.

@a74nh
Copy link
Contributor

a74nh commented Aug 22, 2023

Looks like the (Build coreclr Pri0 Runtime Tests Run osx x64 checked) failures are nothing to do with this patch

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM

@kunalspathak kunalspathak merged commit 1a1ce1f into dotnet:main Sep 12, 2023
106 of 108 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants