Skip to content

Fix escaping of list items from triplets#12718

Closed
Deadpikle wants to merge 9 commits intomicrosoft:masterfrom
Deadpikle:fix/list-escaping
Closed

Fix escaping of list items from triplets#12718
Deadpikle wants to merge 9 commits intomicrosoft:masterfrom
Deadpikle:fix/list-escaping

Conversation

@Deadpikle
Copy link
Copy Markdown
Contributor

This PR fixes some escaping issues of list items that could come from triplets. e.g., if you set multiple architectures for VCPKG_OSX_ARCHITECTURES, the arguments were not escaped properly. This PR fixes that.

The code from this PR was pulled from #12657 per the discussions there and was done entirely by @strega-nil.

  • What does your PR fix?

No closed issues.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

No updates to CI. This doesn't directly affect any triplets, but in theory, it will allow you to set multiple architectures (e.g. for a macOS build) in a triplet.

e.g., if you set multiple architectures for VCPKG_OSX_ARCHITECTURES, the arguments were not escaped properly. This PR fixes that.
The code from this PR was pulled from microsoft#12657 and was done entirely by @strega-nil.
@Neumann-A
Copy link
Copy Markdown
Contributor

Wondering if usage of

 cmake_parse_arguments(PARSE_ARGV <N> <prefix> <options>
                      <one_value_keywords> <multi_value_keywords>)

would be better within VCPKG

@strega-nil
Copy link
Copy Markdown
Contributor

@Neumann-A seems reasonable

@JackBoosY JackBoosY added the category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) label Aug 4, 2020
@JackBoosY
Copy link
Copy Markdown
Contributor

pbc regression will be fixed in #12732

@JackBoosY
Copy link
Copy Markdown
Contributor

Pinging @Deadpikle for response. Is work still being done for this PR?

@Deadpikle
Copy link
Copy Markdown
Contributor Author

@JackBoosY Hi. Sorry for dropping off the face of the planet in this PR. I had to do some other things for work, and I haven't been able to get back to this. I don't think I'll be able to get back to this in the near future, so either someone else needs to pick up this PR and its associated fixes (are they even necessary with other work that's been done since August? I haven't been following.) or this PR needs to be closed.

Apologies for not finishing it up. I was having trouble with the CI failing on something different every time.

@JackBoosY
Copy link
Copy Markdown
Contributor

@Deadpikle That's okay, please reopen this PR when you return to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants