[document] Doc fixes for utils in vcpkg-cmake port #21763
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!
PRs must add only one version and must not modify any published versions
When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.
Error: Local changes detected for vcpkg-cmake but no changes to version or port version.
-- Version: 2021-09-13
-- Old SHA: fc4d9fcc5b8d2b97c083c6b70dd06df5174bd97b
-- New SHA: 413b19357425489d510d4bf76c65e6f50e49b1ad
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
JackBoosY
added
the
category:documentation
|
Please download this patch, apply it in your branch and commit changes. |
JackBoosY
changed the title
|
@JackBoosY I can't actually download that, but I can update the port version properly -- I kinda forgot that since I am doing the change in a file in a port, I have to update port versions. |
|
Please note that the documentation of vcpkg is automatically synchronized through the comments in the cmake script. |
|
Depends on #21825. |
JackBoosY
added
the
depends:different-pr
* `vcpkg_cmake_configure`: links to `opencv4` as example instead of `opencv` which is an empty port * `vcpkg_cmake_install`: fix link to `vcpkg_cmake_build`'s docs * `vcpkg_cmake_build`: fix link to `vcpkg_cmake_install`'s docs
horenmar
changed the title
|
Since this is still open, I added some more doc fixes for the vcpkg-cmake port. |
JackBoosY
removed
the
depends:different-pr
| "version-date": "2021-09-13", | ||
| "port-version": 1 |
There was a problem hiding this comment.
We shouldn't add the vcpkg-cmake port version for update doc.
There was a problem hiding this comment.
We can either bump the main version, increment port version, or silently change what an already published version means.
There was a problem hiding this comment.
Updating the version of vcpkg-cmake will cause if users use vcpkg upgrade to upgrade all installed ports, all ports that depend on the cmake build system will be updated unnecessarily.
There was a problem hiding this comment.
The CI requires either a version change or a port-version changes when port content changes. That's the downside of docs in scripts.
There was a problem hiding this comment.
all ports that depend on the cmake build system will be updated unnecessarily.
This might be mitigated by bundling with the necessary uwp toolchain fix, #21857.
There was a problem hiding this comment.
I think we should just not put the docs in the sources any more.
There was a problem hiding this comment.
For enforcing consistency, the dependency might be turned around:
Let the document cross-reference the cmake script's sha512 sum. Check in a CI task.
There was a problem hiding this comment.
How about removing comments and trailing whitespaces from a script before taking its hash?
There was a problem hiding this comment.
How about removing comments and trailing whitespaces from a script before taking it's hash?
I would prefer to remove the redundancy instead of adding a cmake parser.
OTOH we might just move the documented scripts into a subdirectory of the respective port directory - IIRC subdirectory contents are not hashed. (?)
There was a problem hiding this comment.
@dg0yt I still think it could be a positive change. Right now I have a custom triplet with a typo which I'm afraid to correct because it would require rebuilding of 170+ libraries.
|
Conclusion: We should wait for the next update of vcpkg-cmake PR and incorporate this content. |
JackBoosY
added
the
depends:different-pr
Fun fact, there have been two of those between the time this PR was open and now. 🤷 |
And I explicitly proposed to do that in one of them, but it was ignored. |
Ah that's because that PR doesn't contain changes about |
ras0219-msft
added
info:world-rebuild
and removed
depends:different-pr
|
Thanks you very much for the PR and sorry for the long merge time! These changes were rolled into #22626 to minimize user impact. |
Describe the pull request
I changed the example link in documentation from opencv (empty portfile, alias of opencv4) to opencv4 proper.
What does your PR fix?
Fixes #21762
Which triplets are supported/not supported? Have you updated the [CI baseline]
N/A doc change
Does your PR follow the maintainer guide?
As much as it applies, yes.