Update vcpkg ports to use a manifest json file instead of a CONTROL file.#2132
Conversation
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||
| # SPDX-License-Identifier: MIT | ||
| # |
There was a problem hiding this comment.
I am wondering if we should remove these comment now, since the file is JSON, and JSON doesn't support comments.
There was a problem hiding this comment.
Is there any json-parser which will fail because if this file header?
Are we auto-removing this with release-scripts anyway?
There was a problem hiding this comment.
Is there any json-parser which will fail because if this file header?
Yes, if they ingest it directly. Many RFC-compliant json parsers trip up on this.
Are we auto-removing this with release-scripts anyway?
Yes.
| # | ||
| { | ||
| "name": "azure-security-keyvault-common-cpp", | ||
| "version-string": "@AZ_LIBRARY_VERSION@", |
There was a problem hiding this comment.
This is now a JSON string, so wrapped in quotes, where previously it was just:
Version: @AZ_LIBRARY_VERSION@
for remaining packages.
that CONTROL is gone.
|
|
||
| macro(az_vcpkg_export targetName macroNamePart dllImportExportHeaderPath) | ||
| # CONTROL file has an extra '#' in the comment section, because VcPkg can't handle an empty line at that position, | ||
| # The vcpkg.json file has an extra '#' in the comment section, because VcPkg can't handle an empty line at that position, |
There was a problem hiding this comment.
@antkmsft I don't think this holds true anymore, since we are not using a CONTROL file.
| # and exit. Other steps will check SkipVcpkgUpdate to decide whether to move | ||
| # forward. | ||
| if (!(Get-ChildItem -Path "$SourceDirectory/port/CONTROL")) { | ||
| if (!(Get-ChildItem -Path "$SourceDirectory/port/vcpkg.json")) { |
There was a problem hiding this comment.
@danieljurek, can you fork off this branch https://github.com/ahsonkhan/azure-sdk-for-cpp/tree/UpdatePorts to make the engsys fixes in the release pipeline, if we need to make any other fixes, and test it with the daily vcpkg test PR.
Let me know if you want me to keep this change in as part of this PR, or remove it.
|
We are now using vcpkg.json instead of CONTROL files in the packages published to vcpkg: https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/pr-review-checklist.md#c000013 |
ahsonkhan
requested review from
a team,
Jinming-Hu,
RickWinter,
antkmsft,
danieljurek,
katmsft,
schaabs,
vhvb1989 and
vinjiang
as code owners
| # | ||
| { | ||
| "name": "azure-core-cpp", | ||
| "version-string": "@AZ_LIBRARY_VERSION@", |
There was a problem hiding this comment.
We should be using version-semver.
There was a problem hiding this comment.
What do you mean? This is replaced by our tooling with the version string from version.hpp on release.
3 participants
This is step 1 of #2085
We have to also fix up some of the engsys scripts and release pipeline.
We are now using vcpkg.json instead of CONTROL files in the packages published to vcpkg:
microsoft/vcpkg#17142
microsoft/vcpkg#17143
https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/pr-review-checklist.md#c000013
cc @danieljurek, @antkmsft