Can I draft this PR until it's ready for review?

Can I draft this PR until it's ready for review?

It was meant to be a draft.

I would really like to use this now for every port I update.

PING @BillyONeal @ras0219-msft @strega-nil-ms

I really don't want to submit more PRs without tested usage so help define at least the interface.

# 3. Fully emulate the toolchain file for the just-built package (just adding it to CMAKE_PREFIX_PATH is not enough).

I touch one of the dragons here, loading vcpkg-cmake-wrapper.cmake directly from the staging directory (packages) instead of calling find_package through the vcpkg toolchain. This implementation won't work if more than one wrapper from the tested port is need to resolve a find_package call. A robust implementation needs adjustments in vcpkg.cmake to make the staged package testable. And this topic might touch all variables where vcpkg.cmake adds paths.

Last we looked at this I think we were still worried about this issue. Because this has been added and removed before we are nervous about opening this can of worms.

===================

Looking again if the interface is really "if this other port is installed first then additional checks happen" then that makes install path dependent which I don't think is OK. We can hook up an additional tool parameter akin to our prohibit-backcompat-features stuff if necessary, pretty cheaply. I would like people who understand those dragons to be happy before dropping everything to unblock you on that though.

Looking again if the interface is really "if this other port is installed first then additional checks happen" then that makes install path dependent which I don't think is OK.

In general I do agree. However, the check is not a fixup. It has no side-effect on the package content and dependencies. It just adds extra conditions for the build result to be accepted.

In particular, given expected frequent usage, I would very much like to decouple the testing facility from the package ABI hash, to not trigger mandatory world rebuilds from check improvements. This may need more work, and it could use a different interface, completely avoiding the port dependency.

Because this has been added and removed before we are nervous about opening this can of worms.

The current can of worms is that package quality is a lottery. No option to force validation of cmake or pkg-config usage. No option to run the tests provided by the packages themselves. There is vcpkg's whole-universe CI, but this isn't a sufficient substitute for valdiating the usage contract of individual packages. The wrappers may break without notice when a CMake update brings an updated Find module.

In general I do agree. However, the check is not a fixup. It has no side-effect on the package content and dependencies. It just adds extra conditions for the build result to be accepted.

Sure, neither does --prohibit-backcompat-features.

In particular, given expected frequent usage, I would very much like to decouple the testing facility from the package ABI hash, to not trigger mandatory world rebuilds from check improvements. This may need more work, and it could use a different interface, completely avoiding the port dependency.

Right, the interface I'm arguing for is a command line parameter.

The current can of worms is that package quality is a lottery. No option to force validation of cmake or pkg-config usage. No option to run the tests provided by the packages themselves. There is vcpkg's whole-universe CI, but this isn't a sufficient substitute for valdiating the usage contract of individual packages. The wrappers may break without notice when a CMake update brings an updated Find module.

Yes, I agree that the lack of testing situation is a problem.

Right, the interface I'm arguing for is a command line parameter.

The downside of a parameter, apart from requiring changes in the tool, is that I would have to add it permanently. Unless I missed a configuration file which is always read.

FTR I could also use find_package(vcpkg-cmake-validate 1) \n if(vcpkg-cmake-validate_FOUND) to discover if a particular test is enabled and a compatible version is available.

Right, the interface I'm arguing for is a command line parameter.

The downside of a parameter, apart from requiring changes in the tool, is that I would have to add it permanently. Unless I missed a configuration file which is always read.

We could give it an environment variable?

We could give it an environment variable?

Okay, I could add it to my .profile.
It would probably work well also for CI. (Even with extra parameters added to an existing AZP pipeline.)

We do need to add these test functions to ensure the correctness of the port, but the extra time cost should not be borne by the user.
My suggestion is to only enable it in our pipeline tests with --editable mode.

We do need to add these test functions to ensure the correctness of the port, but the extra time cost should not be borne by the user.

Everybody agrees, and nobody proposes something different.

My suggestion is to only enable it in our pipeline tests with --editable mode.

--editable has a different meaning. It affects only the current package. It doesn't work for vcpkg ci.

--editable has a different meaning. It affects only the current package. It doesn't work for vcpkg ci.

I mean both of them, we may need to add an extra cmake flag in vcpkg-tool to enable those tests, such as VCPKG_ENABLE_TESTS.
The reason for needing to support --editable is that we can test the port while doing port development or repair work, so we only need to target the current port.

@ras0219 @ras0219-msft indicates that if we're going to make this optional that it is likely a good idea to separate this from portfile.cmake entirely so that we can more easily run this as a post-build check, for example after a binary cache restore. (And also to create space in the future where we would probably want to be in the business of running ports' unit tests themselves where reasonable)

@vicroms Is taking point to make sure you have the feedback you need otherwise regarding our hesitancy due to history with vcpkg_test_cmake.

to make sure you have the feedback you need otherwise regarding our hesitancy due to history with vcpkg_test_cmake.

I feel like there is still a gap between rejecting this PR and defining the interface of what could be submitted as the next PR with a reasonable chance of acceptance.

So let me give you some options.

The key aspect is the entry point for port usage tests. Some ideas:

• porttest.cmake file
  To be run in script mode, similar but independent of portfile.cmake.
  May easily build multiple child projects via "maintainer functions".
  May easily use an alternative version of CMake for some tests.
  May run either on a staged or on an installed port.
• porttest\CMakelists.txt file
  To be run in with the vcpkg toolchain on an installed port. (With some extra hooks, it might also work on a staged port.)
  May need to use ExternalProject to decouple multiple tests or alternative CMake versions.
  May use support functions via include or find_package.
  Alternative CMake versions could be deduced from the cmake_minimum_required statement.
• porttest directory hosting multiple scripts or subdirs.

FTR regular post-build checks run on the staged port (in packages but not in installed). IIUC The package artifact (for caching) is created after the post-build checks, and the package is installed after that. So it is difficult to use the same check as a post-build gate and after installation from a cached artifact.

Another aspect mentioned in the comments is running ports' unit tests. This must happen while building the package, from portfile.cmake. Some ideas:

• A script mode variable, e.g. VCPKG_ENABLE_TESTING (similar to CMake's ENABLE_TESTING).
• A special feature, e.g. tests (similar to the special feature core).

I feel like there is still a gap between rejecting this PR and defining the interface of what could be submitted as the next PR with a reasonable chance of acceptance.

Right, the bits @vicroms is on the hook to verify is making sure how the actual validation works works; after that's OK we can relatively easily hook it up to any interface.

No desire to "reject" the PR.

I will merge master ASAP.

making sure how the actual validation works

Does the validation work? It verifies some working cases, but we cannot prove that it detects all broken cases.
What I can say is that I permanently detect broken ports which would have been easily detected.

Ideally, the tests would run after installation to installed. This would avoid some complications, and it would better reflect the user's scenario. However, all current checks, and the upload of the binary artifact, seem to take place before this step.

Can you please merge to master first?

Ideally, the tests would run after installation to installed. This would avoid some complications, and it would better reflect the user's scenario. However, all current checks, and the upload of the binary artifact, seem to take place before this step.

This is the same conclusion the team has reached. We would like to change CI to insert the CMake validation step after everything is laid out in installed/ and before uploading to the binary cache.

I really like this PR but I think there are a few issues that have not been discussed yet:

Feel free to correct me if any of my points is wrong.

• Do we have enough CI budget for these tests, i.e. how long do they take?
• Maintenance overhead: We have to manually keep track of installed headers/ symbols/ targets that might change during port updates
  • We have to proactively touch a port in order to find bugs which is hard given that there are ~ 2K ports. It would be easier if we had a tool that automatically scans all ports for bugs. In contrast, with this approach we only observe a bug after actively searching for them, i. e. using vcpkg_cmake_validate.
• It is inefficient to test only one symbol at a time
  • However, this might be required to make sure there is no interference between individual tests
  • It is well known that extensive check symbol executions slow down configure time
  • Ideally, we would test all symbols of a port in one go or even multiple ports in one go
• Do we really need to perform a compilation in order to test vcpkg_cmake_wrapper and find_package()?
  • Isn't the configure step enough to tell that a package is found? We could observe the variables that find_package sets if it finds a package?
  • Is it possible to tell by analysing the installed CMake config/ vcpkg_cmake_wrapper that everything is set up correctly?
• Is it possible to detect weither a package comes from somewhere else than vcpkg?

@Thomas1664 I can't correct you if you ask questions. Regarding the magic bug scanning tool: vcpkg even fails to derive quality usage hints.

I'm not worried by CI time. I'm worried about how many ports turn out to be unusable despite hundreds of CI runs, and no way to protect my voluntary work (read: my time!) against regressions.

I would prefer to have half the number of ports but twice the quality. Cross-platform.

Regarding the magic bug scanning tool: vcpkg even fails to derive quality usage hints.

I'm worried about how many ports turn out to be unusable despite hundreds of CI runs, and no way to protect my voluntary work (read: my time!) against regressions.

You could put them on CI baseline if there are too many regressions.

The problem with this approach is that it doesn't scale well because you have to explicitly add tests and I'm afraid only a few people actually would use this. Like there are many ports that use vcpkg_configure_cmake although this function is deprecated for more than a year.

I think it would be easier to implement this check inside the vcpkg-tool via C++ and automate which symbols/ headers are used for testing. The only tricky part would be to find symbols to use for testing. The tool nm (installed by default on Linux) can help with this. With nm we can get the symbol table of a .a/ .lib/.so/.dll/.dylib file: nm libbz2.a --demangle --defined-only.

The problem with this approach is that it doesn't scale well because you have to explicitly add tests

We have code review for this.

Checking up on this again because it was tagged requires vcpkg-team-review:

• We remain interested in some way for ports to test the things tested here.
• The problem that the installation happens after portfile.cmake finishes running and therefore portfile.cmake can't see the actually installed state and as a result can't truly test this remains.
• We are considering adding features like this to the tool in the future, and when we do so we will come back to this PR to make sure the use cases it intends to test are fixed.

FWIW I added CI test ports to cover critical port and feature configurations, after installation. As long as this isn't blocked, I can arrange with that state.