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.

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

[vcpkg-cmake] Remove cmake generator support restrictions #22626

Merged
merged 9 commits into from
Jan 28, 2022

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 19, 2022

Describe the pull request

  • What does your PR fix?

    Removes generator restrictions from vcpkg_cmake_build. When arriving in this function, the generator was already used successfully for build system generation, so there is no reaon for failing here. There is still a point in printing a warning if we don't how to control the build tool (verbosity, parallelism). Resolves VCPKG forcing Ninja #22368 if using the maintainer functions from port vcpkg-cmake. Fixes building packages which assume the make build tool, e.g. by using $(MAKE) for recursive invocations.
    Rectifies the configure command mentioned in that message.
    Adds Unix Makefiles support. (Tested locally with libwebsockets:x64-linux. Total port build time is equal to Ninja, app. 20 s.)
    Integrates the documentation update from [document] Doc fixes for utils in vcpkg-cmake port #21763.
    Adds Xcode support by @luncliff, cherry-picked from [vcpkg-cmake] support Xcode parallel build #22631.

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

    all, no
    (No effort is made to acquire make for windows. I assume cmake will already complain in the configure step if cannot find the build tool.)

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    yes

dg0yt and others added 3 commits January 19, 2022 07:54
CMake already completed configuration using the given generator.
* `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
@dg0yt dg0yt mentioned this pull request Jan 19, 2022
@PhoebeHui PhoebeHui assigned JackBoosY and unassigned PhoebeHui Jan 19, 2022
@PhoebeHui PhoebeHui added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Jan 19, 2022
dg0yt and others added 3 commits January 19, 2022 21:11
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/vcpkg-cmake/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

@JackBoosY
Copy link
Contributor

LGTM.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jan 22, 2022
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, though I will merge this with a bundle of other rebuild-the-world changes.

Thanks!

@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 25, 2022

I will merge this with a bundle of other rebuild-the-world changes.

It would be nicely complemented by vcpkg-cmake-config changes, such as #22546.

@ras0219-msft ras0219-msft merged commit dd42206 into microsoft:master Jan 28, 2022
@dg0yt dg0yt deleted the make branch January 28, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VCPKG forcing Ninja
6 participants