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.

Sign up for GitHub

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

Jump to bottom

Teach x-ci-verify-versions to check that versions exist in the database #1210

Draft
wants to merge 64 commits into
base: main
Choose a base branch
from
Draft

Teach x-ci-verify-versions to check that versions exist in the database #1210

wants to merge 64 commits into from

Conversation

BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Sep 23, 2023 •
edited

Initial results are fixed in microsoft/vcpkg#33948

As part of adding these checks, I also substantially overhauled how x-ci-verify-versions works:

  • Print error messages naming the file we're complaining about so that IDEs etc. can go to the file, (Using the standard path: kind: message format)
  • Record the location in try_load_port and friends instead of requiring callers recover that themselves.
  • Make load_git_versions_file return the expected path of the versions file so that callers need not recover that themselves.
  • Deduplicate 'try vcpkg.json, also try CONTROL' behavior from x-add-version and x-ci-verify-versions.
  • Don't stop validating versions information at the first error.

The remaining places that are still thinking about adding / "vcpkg.json" are now only:

  • format-manifest when deciding the new path after parsing a CONTROL
  • new when deciding the new path
  • inside try_load_port and friends themselves
  • the horrible mess that is how vcpkgpaths loads the consumer-manifest

Before and after output with an intentionally damaged repo follows.

Before:

PS C:\Dev\vcpkg> .\vcpkg.exe x-ci-verify-versions
FAIL: adios2
FAIL: boost
FAIL: cpprestsdk
FAIL: curl
FAIL: zlib
error: Found the following errors:
error: curl is missing a version database file at C:\Dev\vcpkg\versions\c-\curl.json
Run:
vcpkg x-add-version curl
to create the versions file.
error: while attempting to load local port cpprestsdk
C:\Dev\vcpkg\ports\cpprestsdk\vcpkg.json:8:7: error: Unexpected character in middle of array
  on expression:       "platform": "!uwp & !windows"
                       ^
error: while parsing versions for adios2 from C:\Dev\vcpkg\versions\a-\adios2.json
2.8.3#1 is declared with fac4ec886b3d46f0a48ee44988fc8224bce59ad7, but the local port has a different SHA 73405fc9493db67ddbd3ac578e41f01eea80df8c.
Please update the port's version fields and then run:
vcpkg x-add-version adios2
git add versions
git commit -m "Update version database"
to add the new version.
error: while parsing versions for boost from C:\Dev\vcpkg\versions\b-\boost.json
1.83.0#1 is declared with 315139e076ff5ea5672a158688934b04d2b7a815, but the local port has a different SHA 28c60c17f76c5dc525b0c64f7effdc2336ccb81b.
Please update the port's version fields and then run:
vcpkg x-add-version boost
git add versions
git commit -m "Update version database"
to add the new version.
error: while parsing versions for zlib from C:\Dev\vcpkg\versions\z-\zlib.json
1.3#2 is declared with 5ac18c6e6e3e2bf2a9e3d0bc8a845f198e4c4e05, but the local port has a different SHA a4344974697f4860565e4c5ba39d6b3e8086dbef.
Please update the port's version fields and then run:
vcpkg x-add-version zlib
git add versions
git commit -m "Update version database"
to add the new version.
To attempt to resolve all errors at once, run:
vcpkg x-add-version --all

After:

PS C:\Dev\vcpkg> C:\Dev\vcpkg-tool\out\build\Win-x64-Debug-WithArtifacts\vcpkg.exe x-ci-verify-versions
C:\Dev\vcpkg\ports\cpprestsdk\vcpkg.json:8:7: error: Unexpected character in middle of array
  on expression:       "platform": "!uwp & !windows"
                       ^
C:\Dev\vcpkg\versions\a-\adios2.json: error: adios2@2.8.3#1 git tree fac4ec886b3d46f0a48ee44988fc8224bce59ad7 does not match the port directory
C:\Dev\vcpkg\ports\adios2: note: the port directory has git tree 73405fc9493db67ddbd3ac578e41f01eea80df8c
C:\Dev\vcpkg\ports\adios2\vcpkg.json: note: if adios2@2.8.3#1 is already published, update this file with a new version or port-version, commit it, then add the new version by running:
  vcpkg x-add-version adios2
  git add versions
  git commit -m "Update version database"
note: if adios2@2.8.3#1 is not yet published, overwrite the previous git tree by running:
  vcpkg x-add-version adios2 --overwrite-version
  git add versions
  git commit -m "Update version database"
C:\Dev\vcpkg\ports\adios2\vcpkg.json: error: the override of bzip2 names version 1.0#1 which does not exist in the version database. Installing this port at the top level will fail as that version will be unresolvable.
C:\Dev\vcpkg\versions\b-\bzip2.json: note: consider removing the version override or choosing a value declared here
C:\Dev\vcpkg\ports\aliyun-oss-c-sdk\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\aws-lambda-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\aws-sdk-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\azure-c-shared-utility\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\azure-core-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named curl
C:\Dev\vcpkg\versions\b-\boost.json: error: boost@1.83.0#1 git tree 315139e076ff5ea5672a158688934b04d2b7a815 does not match the port directory
C:\Dev\vcpkg\ports\boost: note: the port directory has git tree 28c60c17f76c5dc525b0c64f7effdc2336ccb81b
C:\Dev\vcpkg\ports\boost\vcpkg.json: note: if boost@1.83.0#1 is already published, update this file with a new version or port-version, commit it, then add the new version by running:
  vcpkg x-add-version boost
  git add versions
  git commit -m "Update version database"
note: if boost@1.83.0#1 is not yet published, overwrite the previous git tree by running:
  vcpkg x-add-version boost --overwrite-version
  git add versions
  git commit -m "Update version database"
C:\Dev\vcpkg\ports\cfitsio\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named curl
C:\Dev\vcpkg\ports\cocoyaxi\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named libcurl
C:\Dev\vcpkg\ports\cpr\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\cpr\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named ssl
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: this port is not in the version database
C:\Dev\vcpkg\versions\c-\curl.json: note: the version database file should be here
note: run 'vcpkg x-add-version curl' to create the version database file
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named http2
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named idn
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named idn
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named schannel
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named ssh
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named ssh
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named ssl
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named ssl
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named ssl
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named winldap
C:\Dev\vcpkg\ports\curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named winssl
C:\Dev\vcpkg\ports\curlpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\czmq\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named curl
C:\Dev\vcpkg\ports\date\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named remote-api
C:\Dev\vcpkg\ports\gdal\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named curl
C:\Dev\vcpkg\ports\google-cloud-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named rest-common
C:\Dev\vcpkg\ports\gz-fuel-tools8\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\idevicerestore\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\ignition-fuel-tools1\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\ignition-fuel-tools4\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\kinectsdk1\vcpkg.json: error: the "version>=" constraint to vcpkg-tool-lessmsi names version 1.10#1 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.C:\Dev\vcpkg\versions\v-\vcpkg-tool-lessmsi.json: note: consider removing the version constraint or choosing a value declared here
C:\Dev\vcpkg\ports\kinectsdk2\vcpkg.json: error: the "version>=" constraint to vcpkg-tool-lessmsi names version 1.10#1 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.C:\Dev\vcpkg\versions\v-\vcpkg-tool-lessmsi.json: note: consider removing the version constraint or choosing a value declared here
C:\Dev\vcpkg\ports\kubernetes\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\launch-darkly-server\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\libcurl-simple-https\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\libideviceactivation\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\libmediainfo\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\libmupdf\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\librdkafka\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named curl
C:\Dev\vcpkg\ports\libvault\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\libwandio\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\matroska\vcpkg.json: error: the "version>=" constraint to ebml names version 1.4.3 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.
C:\Dev\vcpkg\versions\e-\ebml.json: note: consider removing the version constraint or choosing a value declared here
C:\Dev\vcpkg\ports\netcdf-c\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named dap
C:\Dev\vcpkg\ports\oatpp-curl\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\openjpeg\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named tools
C:\Dev\vcpkg\ports\openscap\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\opentelemetry-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\opentelemetry-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named otlp-http
C:\Dev\vcpkg\ports\opentelemetry-fluentd\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\openvino\vcpkg.json: error: the "version>=" constraint to protobuf names version 3.20.3 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.
C:\Dev\vcpkg\versions\p-\protobuf.json: note: consider removing the version constraint or choosing a value declared here
note: the dependency is in the feature named onnx
C:\Dev\vcpkg\ports\openvino\vcpkg.json: error: the "version>=" constraint to protobuf names version 3.20.3 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.
C:\Dev\vcpkg\versions\p-\protobuf.json: note: consider removing the version constraint or choosing a value declared here
note: the dependency is in the feature named onnx
C:\Dev\vcpkg\ports\openvino\vcpkg.json: error: the "version>=" constraint to protobuf names version 3.20.3 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.
C:\Dev\vcpkg\versions\p-\protobuf.json: note: consider removing the version constraint or choosing a value declared here
note: the dependency is in the feature named paddle
C:\Dev\vcpkg\ports\openvino\vcpkg.json: error: the "version>=" constraint to protobuf names version 3.20.3 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.
C:\Dev\vcpkg\versions\p-\protobuf.json: note: consider removing the version constraint or choosing a value declared here
note: the dependency is in the feature named paddle
C:\Dev\vcpkg\ports\openvino\vcpkg.json: error: the "version>=" constraint to protobuf names version 3.20.3 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.
C:\Dev\vcpkg\versions\p-\protobuf.json: note: consider removing the version constraint or choosing a value declared here
note: the dependency is in the feature named tensorflow
C:\Dev\vcpkg\ports\openvino\vcpkg.json: error: the "version>=" constraint to protobuf names version 3.20.3 which does not exist in the version database. All versions must exist in the version database to be interpreted by vcpkg.
C:\Dev\vcpkg\versions\p-\protobuf.json: note: consider removing the version constraint or choosing a value declared here
note: the dependency is in the feature named tensorflow
C:\Dev\vcpkg\ports\osg\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named plugins
C:\Dev\vcpkg\ports\osg\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named tools
C:\Dev\vcpkg\ports\pdal\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\poppler\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named curl
C:\Dev\vcpkg\ports\ppconsul\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\proj\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named net
C:\Dev\vcpkg\ports\prometheus-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named push
C:\Dev\vcpkg\ports\qtvirtualkeyboard\vcpkg.json: error: the dependency t9write does not exist in the version database; does that port exist?
note: the dependency is in the feature named t9write
C:\Dev\vcpkg\ports\restclient-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\sentry-native\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named transport
C:\Dev\vcpkg\ports\shogun\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\tesseract\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\tgbot-cpp\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\ports\vsgxchange\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
note: the dependency is in the feature named curl
C:\Dev\vcpkg\ports\wxwidgets\vcpkg.json: error: the dependency curl does not exist in the version database; does that port exist?
C:\Dev\vcpkg\versions\z-\zlib.json: error: zlib@1.3#2 git tree 5ac18c6e6e3e2bf2a9e3d0bc8a845f198e4c4e05 does not match the port directory
C:\Dev\vcpkg\ports\zlib: note: the port directory has git tree a4344974697f4860565e4c5ba39d6b3e8086dbef
C:\Dev\vcpkg\ports\zlib\vcpkg.json: note: if zlib@1.3#2 is already published, update this file with a new version or port-version, commit it, then add the new version by running:
  vcpkg x-add-version zlib
  git add versions
  git commit -m "Update version database"
note: if zlib@1.3#2 is not yet published, overwrite the previous git tree by running:
  vcpkg x-add-version zlib --overwrite-version
  git add versions
  git commit -m "Update version database"
C:\Dev\vcpkg\versions\baseline.json: error: zlib is assigned 1.3, but the local port is 1.3#2
C:\Dev\vcpkg\ports\zlib\vcpkg.json: note: zlib is declared here
note: you can run the following commands to add the current version of zlib automatically:
  vcpkg x-add-version zlib
  git add versions
  git commit -m "Update version database"

@BillyONeal
Make registries distinguish between 'baseline is broken' and 'baselin…
e7a6683 
…e missing'.
@BillyONeal
Fix move on constant variable.
09e135c
@BillyONeal
Fix merge conflict error.
4d8ca9e
@BillyONeal
Merge remote-tracking branch 'origin/main' into clarify-no-baseline-v…
ac43653 
…s-error

# Conflicts:
#	src/vcpkg/commands.ci-verify-versions.cpp
#	src/vcpkg/registries.cpp
@BillyONeal
Substantially overhaul how x-ci-verify-versions works:
21bb633 
* Print error messages naming the file we're complaining about so that IDEs etc. can go to the file, (Using the standard path: kind: message format)
* Record the location in try_load_port and friends instead of requiring callers recover that themselves.
* Make load_git_versions_file return the expected path of the versions file so that callers need not recover that themselves.
* Delete registry_location from SourceControlFileAndLocation because it is not ever set. (I don't mind this member existing if there's data to go there but right now, there is not).
* Deduplicate 'try vcpkg.json, also try CONTROL' behavior from x-add-version and x-ci-verify-versions.
* Don't stop validating versions information at the first error.

The remaining places that are still thinking about adding / "vcpkg.json" are now only:

* format-manifest when deciding the new path after parsing a CONTROL
* new when deciding the new path
* inside try_load_port and friends themselves
* the horrible mess that is how vcpkgpaths loads the consumer-manifest
@BillyONeal
Teach vcpkg to find bogus version constraints and overrides that are …
a741b84 
…meaningless because they aren't in the version database.
@BillyONeal BillyONeal added the depends:different-pr This PR depends on a different PR which has been filed label Sep 23, 2023
@BillyONeal
Copy link
Member Author

BillyONeal commented Sep 25, 2023 •
edited

C:\Dev\vcpkg\ports\azure-storage-cpp\vcpkg.json: error: the dependency cpprestsdk does not exist in the version database; does that port exist?

This is wrong, it is in the version database but the local port is damaged. Will fix...

Fixed

@BillyONeal
Clarify that source_location is path to CONTROL, not port_directory.
d9dcaae
@BillyONeal
Fix e2e test failures by also making JSON parse errors print the file…
1e9673c 
…name first
@BillyONeal
Fix output claiming that a port isn't in the versions database when i…
40bdceb 
…t is really that the local port is toast or vice versa. Make validating git trees of removed-from-baseline ports possible.
@BillyONeal
oops
7b3682f
@BillyONeal
oops2
13a30de
@BillyONeal
Try to workaround macos compile failure by just not providing copy op…
8dd6ad8 
…s not used anyway
@BillyONeal
Merge remote-tracking branch 'origin/main' into version-verification
9ad36ac
@BillyONeal
Get rid of one more CONTROL/vcpkg.json tester by using git_checkout_p…
b094ab1 
…ort instead of git_show. (This also makes repeatedly running much faster)
@BillyONeal
Print success and failure messages at the same time and deduplicate g…
58452c1 
…it checkout failure output.
BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Sep 27, 2023
@BillyONeal
Delete broken version database entries.
1b71c90 
Under normal circumstances, things can't be removed from the version database, because that breaks outstanding references to those versions. However, these entries can't be parsed in the first place which means nobody can be depending on them being there. (Usually this comes from accidentally merging PRs that add multiple versions, such as microsoft@8184c5e microsoft#31859 )

This was detected using new output from microsoft/vcpkg-tool#1210

C:\Dev\vcpkg\versions\a-\abseil.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\abseil\2209360b556a40cf034551f6f9063456eac63986_83008.tmp" -c core.autocrlf=false read-tree -m -u 2209360b556a40cf034551f6f9063456eac63986
error: git failed with exit code: (128).
fatal: failed to unpack tree object 2209360b556a40cf034551f6f9063456eac63986
note: while checking out port abseil with git tree 2209360b556a40cf034551f6f9063456eac63986
note: while validating version: 20230125.3#2
C:\Dev\vcpkg\versions\a-\abseil.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\abseil\28fa609b06eec70bb06e61891e94b94f35f7d06e\vcpkg.json: error: $.features: mismatched type: expected a set of features
note: while validating version: 2020-03-03#7
C:\Dev\vcpkg\versions\a-\async-mqtt.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\async-mqtt\61071a18dc0dc629c374fa016b81473e04a28ff1_83008.tmp" -c core.autocrlf=false read-tree -m -u 61071a18dc0dc629c374fa016b81473e04a28ff1
error: git failed with exit code: (128).
fatal: failed to unpack tree object 61071a18dc0dc629c374fa016b81473e04a28ff1
note: while checking out port async-mqtt with git tree 61071a18dc0dc629c374fa016b81473e04a28ff1
note: while validating version: 1.0.8
C:\Dev\vcpkg\versions\b-\blend2d.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\blend2d\ffce764b880d8cc24e3b00328aa3861f15bae91d\vcpkg.json: error: $.features: mismatched type: expected a set of features
note: while validating version: beta_2020-07-31
C:\Dev\vcpkg\versions\b-\blend2d.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\blend2d\03a43f03eb0cab95aac27a77b71273fc4aa2e94d\vcpkg.json: error: $.features: mismatched type: expected a set of features
note: while validating version: beta_2020-07-09
C:\Dev\vcpkg\versions\e-\elfio.json: error: The version database declares 3.8 as version, but 19659f0b36d05c1643f7ecb4b553341a942b1fd5 declares it as version-string. Versions must be unique, even if they are declared with different schemes.
note: run:
vcpkg x-add-version elfio --overwrite-version
to overwrite the scheme declared in the version database with that declared in the port.
C:\Dev\vcpkg\versions\f-\flashlight-text.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\flashlight-text\6386901fa48bce946fdc5775a1c1b784e0a97175_83008.tmp" -c core.autocrlf=false read-tree -m -u 6386901fa48bce946fdc5775a1c1b784e0a97175
error: git failed with exit code: (128).
fatal: failed to unpack tree object 6386901fa48bce946fdc5775a1c1b784e0a97175
note: while checking out port flashlight-text with git tree 6386901fa48bce946fdc5775a1c1b784e0a97175
note: while validating version: 0.0.3
C:\Dev\vcpkg\versions\l-\libwebp.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\a05e0de81085231df86f6902aba1e0a364e2ca7b\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl] (!osx), libwebp[extras] (!osx)
                                                                                                              ^
note: while validating version: 1.1.0
C:\Dev\vcpkg\versions\l-\libwebp.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\5066566c98bc1913b678347c4cbae0a6aff4cf2d\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl] (!osx), libwebp[extras] (!osx)
                                                                                                              ^
note: while validating version: 1.0.3-1
C:\Dev\vcpkg\versions\l-\libwebp.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\6ff3a23b154fad821db2d8236bf9d0382f0229cf\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl, extras] (!osx)
                                                                                                              ^
note: while validating version: 1.0.3
C:\Dev\vcpkg\versions\o-\opencolorio.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\opencolorio\9569944b76966b78bec5ef83672899acd7e4febe_83008.tmp" -c core.autocrlf=false read-tree -m -u 9569944b76966b78bec5ef83672899acd7e4febe
error: git failed with exit code: (128).
fatal: failed to unpack tree object 9569944b76966b78bec5ef83672899acd7e4febe
note: while checking out port opencolorio with git tree 9569944b76966b78bec5ef83672899acd7e4febe
note: while validating version: 2.1.2
C:\Dev\vcpkg\versions\q-\qscintilla.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\qscintilla\b5942c0b0a6d9131bc4ad9a1dde662f809a6d965_83008.tmp" -c core.autocrlf=false read-tree -m -u b5942c0b0a6d9131bc4ad9a1dde662f809a6d965
error: git failed with exit code: (128).
fatal: failed to unpack tree object b5942c0b0a6d9131bc4ad9a1dde662f809a6d965
note: while checking out port qscintilla with git tree b5942c0b0a6d9131bc4ad9a1dde662f809a6d965
note: while validating version: 2.13.4
C:\Dev\vcpkg\versions\r-\robin-map.json: error: The version database declares 0.6.3 as version-semver, but 84f1433234bb4813feee71e4042174ec9e8d5a7a declares it as version-string. Versions must be unique, even if they are declared with different schemes.
note: run:
vcpkg x-add-version robin-map --overwrite-version
to overwrite the scheme declared in the version database with that declared in the port.
C:\Dev\vcpkg\versions\v-\vcpkg-cmake-get-vars.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\vcpkg-cmake-get-vars\c6eb09f11e34173a4bfc31252d02d6aea6c25d8f_83008.tmp" -c core.autocrlf=false read-tree -m -u c6eb09f11e34173a4bfc31252d02d6aea6c25d8f
error: git failed with exit code: (128).
fatal: failed to unpack tree object c6eb09f11e34173a4bfc31252d02d6aea6c25d8f
note: while checking out port vcpkg-cmake-get-vars with git tree c6eb09f11e34173a4bfc31252d02d6aea6c25d8f
note: while validating version: 2023-04-13
@BillyONeal BillyONeal mentioned this pull request Sep 27, 2023
Merged
@BillyONeal
Add comment explaining funny output.
dac383c
@BillyONeal
Deduplicate 'failed to load port' message.
8786bab 
Before:
```
PS C:\Dev\test> ..\vcpkg\vcpkg.exe install
warning: In the September 2023 release, the default triplet for vcpkg libraries changed from x86-windows to the detected host triplet (x64-windows). For the old behavior, add --triplet x86-windows . To suppress this message, add --triplet x64-windows .
error: while loading C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\a05e0de81085231df86f6902aba1e0a364e2ca7b\CONTROL:
C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\a05e0de81085231df86f6902aba1e0a364e2ca7b\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl] (!osx), libwebp[extras] (!osx)
                                                                                                              ^Failed to load port libwebp from C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\a05e0de81085231df86f6902aba1e0a364e2ca7b
note: updating vcpkg by rerunning bootstrap-vcpkg may resolve this failure.
```

After:
```
PS C:\Dev\test> C:\Dev\vcpkg-tool\out\build\Win-x64-Release-Official\vcpkg.exe install --vcpkg-root=C:\Dev\vcpkg
warning: In the September 2023 release, the default triplet for vcpkg libraries changed from x86-windows to the detected host triplet (x64-windows). For the old behavior, add --triplet x86-windows . To suppress this message, add --triplet x64-windows .
C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\a05e0de81085231df86f6902aba1e0a364e2ca7b\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl] (!osx), libwebp[extras] (!osx)
                                                                                                              ^

note: updating vcpkg by rerunning bootstrap-vcpkg may resolve this failure.
PS C:\Dev\test>
```
@BillyONeal
Add a notice of which version we are loading back.
de7428c
@BillyONeal
Fix crash while parsing malformed manifest files.
68122ce 
Alternative to microsoft#1211

Fixes microsoft/vcpkg#33973

I'm not entirely happy with this because it emits extra 'mismatched type' warnings like $.dependencies[0].features[0]: mismatched type: expected a feature in a dependency .
vicroms pushed a commit to microsoft/vcpkg that referenced this pull request Sep 28, 2023
@BillyONeal
Delete broken version database entries. (#34078)
768619a 
Under normal circumstances, things can't be removed from the version database, because that breaks outstanding references to those versions. However, these entries can't be parsed in the first place which means nobody can be depending on them being there. (Usually this comes from accidentally merging PRs that add multiple versions, such as 8184c5e #31859 )

This was detected using new output from microsoft/vcpkg-tool#1210

C:\Dev\vcpkg\versions\a-\abseil.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\abseil\2209360b556a40cf034551f6f9063456eac63986_83008.tmp" -c core.autocrlf=false read-tree -m -u 2209360b556a40cf034551f6f9063456eac63986
error: git failed with exit code: (128).
fatal: failed to unpack tree object 2209360b556a40cf034551f6f9063456eac63986
note: while checking out port abseil with git tree 2209360b556a40cf034551f6f9063456eac63986
note: while validating version: 20230125.3#2
C:\Dev\vcpkg\versions\a-\abseil.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\abseil\28fa609b06eec70bb06e61891e94b94f35f7d06e\vcpkg.json: error: $.features: mismatched type: expected a set of features
note: while validating version: 2020-03-03#7
C:\Dev\vcpkg\versions\a-\async-mqtt.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\async-mqtt\61071a18dc0dc629c374fa016b81473e04a28ff1_83008.tmp" -c core.autocrlf=false read-tree -m -u 61071a18dc0dc629c374fa016b81473e04a28ff1
error: git failed with exit code: (128).
fatal: failed to unpack tree object 61071a18dc0dc629c374fa016b81473e04a28ff1
note: while checking out port async-mqtt with git tree 61071a18dc0dc629c374fa016b81473e04a28ff1
note: while validating version: 1.0.8
C:\Dev\vcpkg\versions\b-\blend2d.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\blend2d\ffce764b880d8cc24e3b00328aa3861f15bae91d\vcpkg.json: error: $.features: mismatched type: expected a set of features
note: while validating version: beta_2020-07-31
C:\Dev\vcpkg\versions\b-\blend2d.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\blend2d\03a43f03eb0cab95aac27a77b71273fc4aa2e94d\vcpkg.json: error: $.features: mismatched type: expected a set of features
note: while validating version: beta_2020-07-09
C:\Dev\vcpkg\versions\e-\elfio.json: error: The version database declares 3.8 as version, but 19659f0b36d05c1643f7ecb4b553341a942b1fd5 declares it as version-string. Versions must be unique, even if they are declared with different schemes.
note: run:
vcpkg x-add-version elfio --overwrite-version
to overwrite the scheme declared in the version database with that declared in the port.
C:\Dev\vcpkg\versions\f-\flashlight-text.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\flashlight-text\6386901fa48bce946fdc5775a1c1b784e0a97175_83008.tmp" -c core.autocrlf=false read-tree -m -u 6386901fa48bce946fdc5775a1c1b784e0a97175
error: git failed with exit code: (128).
fatal: failed to unpack tree object 6386901fa48bce946fdc5775a1c1b784e0a97175
note: while checking out port flashlight-text with git tree 6386901fa48bce946fdc5775a1c1b784e0a97175
note: while validating version: 0.0.3
C:\Dev\vcpkg\versions\l-\libwebp.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\a05e0de81085231df86f6902aba1e0a364e2ca7b\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl] (!osx), libwebp[extras] (!osx)
                                                                                                              ^
note: while validating version: 1.1.0
C:\Dev\vcpkg\versions\l-\libwebp.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\5066566c98bc1913b678347c4cbae0a6aff4cf2d\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl] (!osx), libwebp[extras] (!osx)
                                                                                                              ^
note: while validating version: 1.0.3-1
C:\Dev\vcpkg\versions\l-\libwebp.json: C:\Dev\vcpkg\buildtrees\versioning_\versions\libwebp\6ff3a23b154fad821db2d8236bf9d0382f0229cf\CONTROL:1:94: error: invalid character in feature name (must be lowercase, digits, '-', or '*')
  on expression: libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, simd, cwebp, dwebp], libwebp[vwebp_sdl, extras] (!osx)
                                                                                                              ^
note: while validating version: 1.0.3
C:\Dev\vcpkg\versions\o-\opencolorio.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\opencolorio\9569944b76966b78bec5ef83672899acd7e4febe_83008.tmp" -c core.autocrlf=false read-tree -m -u 9569944b76966b78bec5ef83672899acd7e4febe
error: git failed with exit code: (128).
fatal: failed to unpack tree object 9569944b76966b78bec5ef83672899acd7e4febe
note: while checking out port opencolorio with git tree 9569944b76966b78bec5ef83672899acd7e4febe
note: while validating version: 2.1.2
C:\Dev\vcpkg\versions\q-\qscintilla.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\qscintilla\b5942c0b0a6d9131bc4ad9a1dde662f809a6d965_83008.tmp" -c core.autocrlf=false read-tree -m -u b5942c0b0a6d9131bc4ad9a1dde662f809a6d965
error: git failed with exit code: (128).
fatal: failed to unpack tree object b5942c0b0a6d9131bc4ad9a1dde662f809a6d965
note: while checking out port qscintilla with git tree b5942c0b0a6d9131bc4ad9a1dde662f809a6d965
note: while validating version: 2.13.4
C:\Dev\vcpkg\versions\r-\robin-map.json: error: The version database declares 0.6.3 as version-semver, but 84f1433234bb4813feee71e4042174ec9e8d5a7a declares it as version-string. Versions must be unique, even if they are declared with different schemes.
note: run:
vcpkg x-add-version robin-map --overwrite-version
to overwrite the scheme declared in the version database with that declared in the port.
C:\Dev\vcpkg\versions\v-\vcpkg-cmake-get-vars.json: error: failed to execute: "C:\Program Files\Git\cmd\git.exe" "--git-dir=C:\Dev\vcpkg\.git" "--work-tree=C:\Dev\vcpkg\buildtrees\versioning_\versions\vcpkg-cmake-get-vars\c6eb09f11e34173a4bfc31252d02d6aea6c25d8f_83008.tmp" -c core.autocrlf=false read-tree -m -u c6eb09f11e34173a4bfc31252d02d6aea6c25d8f
error: git failed with exit code: (128).
fatal: failed to unpack tree object c6eb09f11e34173a4bfc31252d02d6aea6c25d8f
note: while checking out port vcpkg-cmake-get-vars with git tree c6eb09f11e34173a4bfc31252d02d6aea6c25d8f
note: while validating version: 2023-04-13
BillyONeal
BillyONeal commented Sep 28, 2023
View reviewed changes
src/vcpkg/paragraphs.cpp Outdated Show resolved Hide resolved
@BillyONeal BillyONeal marked this pull request as draft September 28, 2023 22:40
@BillyONeal BillyONeal mentioned this pull request Sep 28, 2023
Merged
@BillyONeal
Merge remote-tracking branch 'BillyONeal/use-schemed-version' into ve…
36b33a2 
…rsion-verification

# Conflicts:
#	include/vcpkg/sourceparagraph.h
#	src/vcpkg-test/dependencies.cpp
#	src/vcpkg-test/manifests.cpp
#	src/vcpkg/dependencies.cpp
#	src/vcpkg/sourceparagraph.cpp
@BillyONeal
Merge remote-tracking branch 'origin/main' into version-verification
2291b0a 
# Conflicts:
#	src/vcpkg/commands.ci-verify-versions.cpp
#	src/vcpkg/registries.cpp
@BillyONeal
Merge remote-tracking branch 'origin/main' into spdx_location
dc0110e
@BillyONeal
Persist the correct path.
a18997b
@BillyONeal
Code review feedback from 2023-10-26.
6e794ec
@BillyONeal
Copy link
Member Author 

 version database to be interpreted by vcpkg.C:\Dev\vcpkg\versions\v-\vcpkg-tool-lessmsi.json

Check that this missing newline is fixed

@BillyONeal
Merge remote-tracking branch 'origin/main' into spdx_location
a922e76
@BillyONeal
Merge remote-tracking branch 'origin/main' into spdx_location
4434e81 
# Conflicts:
#	src/vcpkg/portfileprovider.cpp
#	src/vcpkg/spdx.cpp
@BillyONeal
Merge branch 'spdx_location' into version-verification
b70ddbe 
# Conflicts:
#	include/vcpkg/base/expected.h
#	include/vcpkg/base/message-args.inc.h
#	include/vcpkg/base/message-data.inc.h
#	include/vcpkg/paragraphs.h
#	include/vcpkg/sourceparagraph.h
#	locales/messages.json
#	src/vcpkg-test/manifests.cpp
#	src/vcpkg/commands.add-version.cpp
#	src/vcpkg/commands.autocomplete.cpp
#	src/vcpkg/commands.build.cpp
#	src/vcpkg/commands.ci-verify-versions.cpp
#	src/vcpkg/dependencies.cpp
#	src/vcpkg/paragraphs.cpp
#	src/vcpkg/portfileprovider.cpp
#	src/vcpkg/registries.cpp
#	src/vcpkg/sourceparagraph.cpp
#	src/vcpkg/spdx.cpp
#	src/vcpkg/vcpkgpaths.cpp
@BillyONeal
Merge gore. Not sure if I want to keep format-manifest changes lookin…
570d2f3 
…g like this.
@BillyONeal BillyONeal marked this pull request as draft November 7, 2023 21:36
@BillyONeal
Copy link
Member Author

I marked this as draft for now while I'm still sucking review parts out

@BillyONeal
Merge remote-tracking branch 'origin/main' into version-verification
b195ca2 
# Conflicts:
#	include/vcpkg/paragraphs.h
#	include/vcpkg/sourceparagraph.h
#	src/vcpkg/commands.add-version.cpp
#	src/vcpkg/commands.autocomplete.cpp
#	src/vcpkg/commands.build.cpp
#	src/vcpkg/commands.ci-verify-versions.cpp
#	src/vcpkg/commands.format-manifest.cpp
#	src/vcpkg/dependencies.cpp
#	src/vcpkg/paragraphs.cpp
#	src/vcpkg/portfileprovider.cpp
#	src/vcpkg/registries.cpp
#	src/vcpkg/sourceparagraph.cpp
@BillyONeal BillyONeal dismissed JavierMatosD’s stale review November 10, 2023 23:24

Contents have changed a lot

@BillyONeal BillyONeal mentioned this pull request Nov 16, 2023
Merged
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Dec 21, 2023
@BillyONeal
In several PRs, such as microsoft#908 and microsoft#1210 , it has bec…
b6c97dc 
…ome clear that we need a thread safe channel through which errors and warnings can both be reasonably reported. Now that microsoft#1279 is landed and functionally everything in the codebase uses ExpectedL, we can look at what the new thing that fixes issues is.

Consider the following:

```c++
    ExpectedL<T> example_api(int a);

    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_manifest_text(StringView text,
                                                                              StringView control_path,
                                                                              MessageSink& warning_sink);
```

The reason this can't return the warnings through the ExpectedL channel is that we don't want the 'error' state to be engaged when there are merely warnings. Moreover, that these channels are different channels means that situations that might want to return errors and warnings together, as happens when parsing files, means that order relationships between errors and warnings is lost. It is probably a good idea in general to put warnings and errors about the same location next to each other in the output, but that's hard to do with this interface.

Rather than multiplexing everything through the return value, this proposal is to multiplex only the success or failure through the return value, and report any specific error information through an out parameter.

1. Distinguish whether an overall operation succeeded or failed in the return value, but
2. record any errors or warnings via an out parameter.

Applying this to the above gives:

```c++
    Optional<T> example_api(MessageContext& context, int a);

    // unique_ptr is already 'optional'
    std::unique_ptr<SourceControlFile> try_load_port_manifest_text(MessageContext& context,
                                                                   StringView text,
                                                                   StringView control_path);
```

Issues this new mechanism fixes:

* Errors and warnings can share the same channel and thus be printed together
* The interface between code wanting to report events and the code wanting to consume them is a natural thread synchronization boundary. Other attempts to fix this have been incorrect by synchronizing individual print calls ( microsoft#1290 ) or complex enough that we are not sure they are correct by trying to recover boundaries by reparsing our own error output ( microsoft#908 )
* This shuts down the "error: error:" and similar bugs where it isn't clear who is formatting the overall error message vs. talking about individual components

Known issues that are not fixed by this change:

* This still doesn't make it easy for callers to programmatically handle specific types of errors. Currently, we have some APIs that still use explicit `std::error_code` because they want to do different things for 'file does not exist' vs. 'there was an I/O error'. Given that this condition isn't well served by the ExpectedL mechanism I don't want to wait until we have a better solution to it to proceed.
* Because we aren't making the context parameter the 'success carrier' it's more complex to implement 'warnings as errors' or similar functionality where the caller decides how 'important' something is. I would be in favor of moving all success tests to the context parameter but I'm not proposing that because the other vcpkg maintainers do not like it.
* Contextual information / stack problems aren't solved. However, the context parameter might be extended in the future to help with this.
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Dec 22, 2023
@BillyONeal
In several PRs, such as microsoft#908 and microsoft#1210 , it has bec…
1738898 
…ome clear that we need a thread safe channel through which errors and warnings can both be reasonably reported. Now that microsoft#1279 is landed and functionally everything in the codebase uses ExpectedL, we can look at what the new thing that fixes issues is.

Consider the following:

```c++
    ExpectedL<T> example_api(int a);

    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_manifest_text(StringView text,
                                                                              StringView control_path,
                                                                              MessageSink& warning_sink);
```

The reason this can't return the warnings through the ExpectedL channel is that we don't want the 'error' state to be engaged when there are merely warnings. Moreover, that these channels are different channels means that situations that might want to return errors and warnings together, as happens when parsing files, means that order relationships between errors and warnings is lost. It is probably a good idea in general to put warnings and errors about the same location next to each other in the output, but that's hard to do with this interface.

Rather than multiplexing everything through the return value, this proposal is to multiplex only the success or failure through the return value, and report any specific error information through an out parameter.

1. Distinguish whether an overall operation succeeded or failed in the return value, but
2. record any errors or warnings via an out parameter.

Applying this to the above gives:

```c++
    Optional<T> example_api(MessageContext& context, int a);

    // unique_ptr is already 'optional'
    std::unique_ptr<SourceControlFile> try_load_port_manifest_text(MessageContext& context,
                                                                   StringView text,
                                                                   StringView control_path);
```

Issues this new mechanism fixes:

* Errors and warnings can share the same channel and thus be printed together
* The interface between code wanting to report events and the code wanting to consume them is a natural thread synchronization boundary. Other attempts to fix this have been incorrect by synchronizing individual print calls ( microsoft#1290 ) or complex enough that we are not sure they are correct by trying to recover boundaries by reparsing our own error output ( microsoft#908 )
* This shuts down the "error: error:" and similar bugs where it isn't clear who is formatting the overall error message vs. talking about individual components

Known issues that are not fixed by this change:

* This still doesn't make it easy for callers to programmatically handle specific types of errors. Currently, we have some APIs that still use explicit `std::error_code` because they want to do different things for 'file does not exist' vs. 'there was an I/O error'. Given that this condition isn't well served by the ExpectedL mechanism I don't want to wait until we have a better solution to it to proceed.
* Because we aren't making the context parameter the 'success carrier' it's more complex to implement 'warnings as errors' or similar functionality where the caller decides how 'important' something is. I would be in favor of moving all success tests to the context parameter but I'm not proposing that because the other vcpkg maintainers do not like it.
* Contextual information / stack problems aren't solved. However, the context parameter might be extended in the future to help with this.
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Dec 29, 2023
@BillyONeal
In several PRs, such as microsoft#908 and microsoft#1210 , it has bec…
9835c31 
…ome clear that we need a thread safe channel through which errors and warnings can both be reasonably reported. Now that microsoft#1279 is landed and functionally everything in the codebase uses ExpectedL, we can look at what the new thing that fixes issues is.

Consider the following:

```c++
    ExpectedL<T> example_api(int a);

    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_manifest_text(StringView text,
                                                                              StringView control_path,
                                                                              MessageSink& warning_sink);
```

The reason this can't return the warnings through the ExpectedL channel is that we don't want the 'error' state to be engaged when there are merely warnings. Moreover, that these channels are different channels means that situations that might want to return errors and warnings together, as happens when parsing files, means that order relationships between errors and warnings is lost. It is probably a good idea in general to put warnings and errors about the same location next to each other in the output, but that's hard to do with this interface.

Rather than multiplexing everything through the return value, this proposal is to multiplex only the success or failure through the return value, and report any specific error information through an out parameter.

1. Distinguish whether an overall operation succeeded or failed in the return value, but
2. record any errors or warnings via an out parameter.

Applying this to the above gives:

```c++
    Optional<T> example_api(MessageContext& context, int a);

    // unique_ptr is already 'optional'
    std::unique_ptr<SourceControlFile> try_load_port_manifest_text(MessageContext& context,
                                                                   StringView text,
                                                                   StringView control_path);
```

Issues this new mechanism fixes:

* Errors and warnings can share the same channel and thus be printed together
* The interface between code wanting to report events and the code wanting to consume them is a natural thread synchronization boundary. Other attempts to fix this have been incorrect by synchronizing individual print calls ( microsoft#1290 ) or complex enough that we are not sure they are correct by trying to recover boundaries by reparsing our own error output ( microsoft#908 )
* This shuts down the "error: error:" and similar bugs where it isn't clear who is formatting the overall error message vs. talking about individual components

Known issues that are not fixed by this change:

* This still doesn't make it easy for callers to programmatically handle specific types of errors. Currently, we have some APIs that still use explicit `std::error_code` because they want to do different things for 'file does not exist' vs. 'there was an I/O error'. Given that this condition isn't well served by the ExpectedL mechanism I don't want to wait until we have a better solution to it to proceed.
* Because we aren't making the context parameter the 'success carrier' it's more complex to implement 'warnings as errors' or similar functionality where the caller decides how 'important' something is. I would be in favor of moving all success tests to the context parameter but I'm not proposing that because the other vcpkg maintainers do not like it.
* Contextual information / stack problems aren't solved. However, the context parameter might be extended in the future to help with this.
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Jan 2, 2024
@BillyONeal
Eliminate ParseError.
6ae7db8 
Another extraction from microsoft#1210 ; this means that ExpectedL is the only ExpectedT that exists.
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Jan 3, 2024
@BillyONeal
Fix Utf8Decoder operator== handling of the last code point in the input
97082da 
While working on diagnostics for microsoft#1210 I observed that we were printing the caret ^ in the wrong place when it goes after the input.

The way this works is we form the line of text to print, then decode the unicode encoding units, and when we hit the target, we stop and print ^:

https://github.com/microsoft/vcpkg-tool/blob/5b8f9c40dd9d6a07ec636590e78c2f49d28624b9/src/vcpkg/base/parse.cpp#L51-L68

however, if the intended location for the ^ is the "end" of the line, we hit this bug:

https://github.com/microsoft/vcpkg-tool/blob/5b8f9c40dd9d6a07ec636590e78c2f49d28624b9/src/vcpkg/base/unicode.cpp#L273

The iterator only compares the last_ pointers, but both the "points at the last code point in the input" and "points to the end of the input" state set `next_ == last_`. See:

https://github.com/microsoft/vcpkg-tool/blob/5b8f9c40dd9d6a07ec636590e78c2f49d28624b9/src/vcpkg/base/unicode.cpp#L222-L226

This means that the points to the end and points one past the end iterator compare equal, so the loop in parse.cpp stops one position too early.

Also adds a bunch of testing for this specific case, for other parts of Utf8Decoder, adds a way to parse the first code point without failing, makes all the operators 'hidden friends', and removes localized strings for bugs-in-vcpkg-itself.
@BillyONeal BillyONeal mentioned this pull request Jan 3, 2024
Merged
BillyONeal added a commit that referenced this pull request Jan 5, 2024
@BillyONeal
Fix Utf8Decoder operator== handling of the last code point in the inp…
4e75e1c 
…ut (#1316)

* Fix Utf8Decoder operator== handling of the last code point in the input

While working on diagnostics for #1210 I observed that we were printing the caret ^ in the wrong place when it goes after the input.

The way this works is we form the line of text to print, then decode the unicode encoding units, and when we hit the target, we stop and print ^:

https://github.com/microsoft/vcpkg-tool/blob/5b8f9c40dd9d6a07ec636590e78c2f49d28624b9/src/vcpkg/base/parse.cpp#L51-L68

however, if the intended location for the ^ is the "end" of the line, we hit this bug:

https://github.com/microsoft/vcpkg-tool/blob/5b8f9c40dd9d6a07ec636590e78c2f49d28624b9/src/vcpkg/base/unicode.cpp#L273

The iterator only compares the last_ pointers, but both the "points at the last code point in the input" and "points to the end of the input" state set `next_ == last_`. See:

https://github.com/microsoft/vcpkg-tool/blob/5b8f9c40dd9d6a07ec636590e78c2f49d28624b9/src/vcpkg/base/unicode.cpp#L222-L226

This means that the points to the end and points one past the end iterator compare equal, so the loop in parse.cpp stops one position too early.

Also adds a bunch of testing for this specific case, for other parts of Utf8Decoder, adds a way to parse the first code point without failing, makes all the operators 'hidden friends', and removes localized strings for bugs-in-vcpkg-itself.

* Add noexcepts as requested by @Thomas1664
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Jan 9, 2024
@BillyONeal
Eliminate ParseError.
6e0f46c 
Another pseudo-extraction from microsoft#1210 ; this means that ExpectedL is the only ExpectedT that exists.
This was referenced Jan 9, 2024
Merged
Open
BillyONeal added a commit that referenced this pull request Jan 26, 2024
@BillyONeal
Eliminate ParseError. (#1322)
910db8b 
* Eliminate ParseError.

Another pseudo-extraction from #1210 ; this means that ExpectedL is the only ExpectedT that exists.

* MessageMap
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Jan 26, 2024
@BillyONeal
In several PRs, such as microsoft#908 and microsoft#1210 , it has bec…
3138143 
…ome clear that we need a thread safe channel through which errors and warnings can both be reasonably reported. Now that microsoft#1279 is landed and functionally everything in the codebase uses ExpectedL, we can look at what the new thing that fixes issues is.

Consider the following:

```c++
    ExpectedL<T> example_api(int a);

    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_manifest_text(StringView text,
                                                                              StringView control_path,
                                                                              MessageSink& warning_sink);
```

The reason this can't return the warnings through the ExpectedL channel is that we don't want the 'error' state to be engaged when there are merely warnings. Moreover, that these channels are different channels means that situations that might want to return errors and warnings together, as happens when parsing files, means that order relationships between errors and warnings is lost. It is probably a good idea in general to put warnings and errors about the same location next to each other in the output, but that's hard to do with this interface.

Rather than multiplexing everything through the return value, this proposal is to multiplex only the success or failure through the return value, and report any specific error information through an out parameter.

1. Distinguish whether an overall operation succeeded or failed in the return value, but
2. record any errors or warnings via an out parameter.

Applying this to the above gives:

```c++
    Optional<T> example_api(MessageContext& context, int a);

    // unique_ptr is already 'optional'
    std::unique_ptr<SourceControlFile> try_load_port_manifest_text(MessageContext& context,
                                                                   StringView text,
                                                                   StringView control_path);
```

Issues this new mechanism fixes:

* Errors and warnings can share the same channel and thus be printed together
* The interface between code wanting to report events and the code wanting to consume them is a natural thread synchronization boundary. Other attempts to fix this have been incorrect by synchronizing individual print calls ( microsoft#1290 ) or complex enough that we are not sure they are correct by trying to recover boundaries by reparsing our own error output ( microsoft#908 )
* This shuts down the "error: error:" and similar bugs where it isn't clear who is formatting the overall error message vs. talking about individual components

Known issues that are not fixed by this change:

* This still doesn't make it easy for callers to programmatically handle specific types of errors. Currently, we have some APIs that still use explicit `std::error_code` because they want to do different things for 'file does not exist' vs. 'there was an I/O error'. Given that this condition isn't well served by the ExpectedL mechanism I don't want to wait until we have a better solution to it to proceed.
* Because we aren't making the context parameter the 'success carrier' it's more complex to implement 'warnings as errors' or similar functionality where the caller decides how 'important' something is. I would be in favor of moving all success tests to the context parameter but I'm not proposing that because the other vcpkg maintainers do not like it.
* Contextual information / stack problems aren't solved. However, the context parameter might be extended in the future to help with this.
@WangWeiLin-MV WangWeiLin-MV mentioned this pull request Mar 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ras0219-msft ras0219-msft ras0219-msft left review comments

@JavierMatosD JavierMatosD JavierMatosD left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@BillyONeal @ras0219-msft @JavierMatosD