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

[libcpplocate] New port #23173

Merged
merged 9 commits into from
Mar 2, 2022
Merged

[libcpplocate] New port #23173

merged 9 commits into from
Mar 2, 2022

Conversation

ekilmer
Copy link
Contributor

@ekilmer ekilmer commented Feb 18, 2022 •
edited
Loading

New port. Also opened an issue on the project's GitHub cginternals/cpplocate#47

  • What does your PR fix?

    N/A

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

    Fails compilation on UWP. Waiting for response from maintainers to see if we want to investigate. Fixed with respective patch. Patch was reverted due to myself lacking knowledge about UWP. UWP is now marked as unsupported

  • 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

github-actions[bot]
github-actions bot approved these changes Feb 18, 2022
View reviewed changes
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/libcpplocate/vcpkg.json

Valid values for the license field can be found in the documentation

@ekilmer ekilmer mentioned this pull request Feb 18, 2022
Closed
@ekilmer
UWP unsupported
c8d5589 
Errors that I don't know how to fix and personally unfamiliar with UWP.
Maybe someone else knows how to fix/support it
@ekilmer
Add version
0fe471d
@JackBoosY JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Feb 21, 2022
@JackBoosY
Copy link
Contributor

Ready for review now?

@ekilmer ekilmer marked this pull request as ready for review February 21, 2022 14:47
@ekilmer
Copy link
Contributor Author

ekilmer commented Feb 21, 2022

Ready for review now?

@JackBoosY Yes! Thank you.

JackBoosY
JackBoosY suggested changes Feb 22, 2022
View reviewed changes
ports/libcpplocate/fix-install-paths.patch Show resolved Hide resolved
ports/libcpplocate/portfile.cmake Outdated Show resolved Hide resolved
ports/libcpplocate/portfile.cmake Outdated Show resolved Hide resolved
ports/libcpplocate/portfile.cmake Show resolved Hide resolved
ports/libcpplocate/portfile.cmake Show resolved Hide resolved
@ekilmer @JackBoosY
Apply suggestions from code review
9a29531 
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
github-actions[bot]
github-actions bot requested changes Feb 22, 2022
View reviewed changes
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.

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 libcpplocate but no changes to version or port version.
-- Version: 2.3.0
-- Old SHA: c2242946dfa4ae8c741e154747f28f6548332514
-- New SHA: db028f50cc9b597430bd2240bdd86779b2458d7c
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@ekilmer
Copy link
Contributor Author

ekilmer commented Feb 22, 2022

I was just double-checking some things and now I'm not sure what should be the correct version listing...

The most recent tag v2.2.0 is here cginternals/cpplocate@01fa9d5 but the META_VERSION_PATCH is listed as 1, making the full version 2.2.1, which doesn't seem correct. I would have expected the v2.2.0 tag to point to the parent of the referenced commit to mark it as the last commit with that version number, right?

On the other hand, their published Ubuntu packages have the latest release as 2.3.0 (like this vcpkg PR) uses the commit that I am using for vcpkg 2.3.0 (cginternals/cpplocate@23aeb69), but the branch named v2.3 is a different commit.

@scheibel (who looks to be the maintainer of cpplocate) could you help clear this up please.

@scheibel
Copy link

Thanks for pursuing an integration into vcpkg!

Regarding the two current versions in cpplocate:
The v2.2 release branch is indeed the one containing the latest official release. I think the v2.2.0 tag self-identifies as version 2.2.1 because we intended to release 2.2.0 but somehow I added an presumably unrelated commit bumping the patch version that got missed and not fixed.
The v2.3 branch, however, contains release candidates for a future v2.3.x release. We create future release branches early and sometimes miss to release them; we mainly work with deployments of the master branch in our downstream projects.
Changes in our deployment files for the Ubuntu PPA are done sporadically when I update packages in our PPA.

I suggest you use the v2.3.0 release that I created just know.

@ekilmer
Merge branch 'master' into cpplocate
6be6aa5 
* master: (57 commits)
  [vcpkg-tools] update cmake and git (windows only) (microsoft#22985)
  Update vcpkg tool to 2022-02-24. (microsoft#23162)
  [vcpkg baseline] Move cspice headers (microsoft#23272)
  Fixed inaccurate Chinese words (microsoft#23179)
  [vcpkg] Add fixed changelog generator. (microsoft#23255)
  [authentication.md] Add Jenkins section (microsoft#23226)
  [vcpkg] Meson osx sysroot (microsoft#21772)
  [pkgconf] enable search for system libs on linux (microsoft#23010)
  [yasm/yasm-tool] Incorporate yasm-tool into yasm (microsoft#23218)
  [lapack-reference] Update to 3.10 (microsoft#23228)
  [skia] Arm64 for skia on osx (microsoft#23222)
  [libfido2] Update to 1.10.0 (microsoft#23241)
  [Tracy] Fixing issue where version 0.7.8 was pulling the wrong version (microsoft#23061)
  [libgpiod] Add new port. (microsoft#23221)
  [drogon] Update to 1.7.5 (microsoft#23227)
  [tinyexif] Remove from fail list. (microsoft#23163)
  [vcpkg docs][ES] Sync with English readme (microsoft#19834) (microsoft#22618)
  [vcpkg baseline][libao] Disable dlfcn check under windows (microsoft#23235)
  [OpenCV] upgrade to v4.5.5 (microsoft#22801)
  [libcurl-simple-https] New port (microsoft#22917)
  ...
@ekilmer
Copy link
Contributor Author

ekilmer commented Feb 27, 2022

Thanks for pursuing an integration into vcpkg!

You're welcome!

I suggest you use the v2.3.0 release that I created just know.

@scheibel thank you for creating that tag and explaining the versioning for cpplocate. I appreciate your time and response(s) 😄

@JackBoosY I have pushed new commits for vcpkg to use the latest tag (v.2.3.0), so this is now ready for another review. Thank you!

I'm not sure what is making CI fail. Should I do anything to fix that?

@JackBoosY
Copy link
Contributor

Looks all passed, our pipeline test has some bugs.

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Feb 28, 2022
@ras0219-msft ras0219-msft merged commit 11be87e into microsoft:master Mar 2, 2022
@ras0219-msft
Copy link
Contributor

ras0219-msft commented Mar 2, 2022 •
edited
Loading

Thanks @ekilmer for the PR and thanks @scheibel for making such an awesome library :)

@scheibel let us know if we can help with adding vcpkg into https://cpplocate.org/#install-instructions

Jimmy-Hu added a commit to Jimmy-Hu/vcpkg that referenced this pull request Mar 2, 2022
@Jimmy-Hu
Merge pull request #956 from microsoft/master
465d16f 
[libcpplocate] New port (microsoft#23173)
ekilmer added a commit to ekilmer/vcpkg that referenced this pull request Mar 2, 2022
@ekilmer
Merge branch 'master' into more-lief
205e031 
* master: (108 commits)
  [calceph] update to version 3.5.1 (microsoft#23325)
  [SpeexDSP] Missing windows definitions (microsoft#23296)
  [triton] Fix usage (microsoft#23250)
  [vcpkg_configure_make] Report warning when unresolved arguments are passed in (microsoft#22567)
  [openmvg] no absolute paths (microsoft#22342)
  [python2] no absolute paths (microsoft#22329)
  [libcpplocate] New port (microsoft#23173)
  [Intelrdfpmathlib] Fix post build error (microsoft#23286)
  [docs] Update maintainer guide to include CMAKE_REQUIRE_FIND_PACKAGE. (microsoft#23321)
  [new-port-template] Remove deprecated cmake (microsoft#23283)
  [xtensor] Update to 0.24.0 (microsoft#23238)
  [pcl/soil2] Disable optimizations for individual function (microsoft#23252)
  [cnats] Add new port (C client for the NATS messaging system). (microsoft#23258)
  [nanobench] Update to 4.3.6 (microsoft#23262)
  [elfio] Update elfio to 3.10 (microsoft#23263)
  [tomlplusplus] Update to 3.0.1 (microsoft#23268)
  [shaderc] Fix CMake export target files (microsoft#23269)
  [PCL] Fix namespace and  VS2022 error C3052 (microsoft#23273)
  [libressl] update to 3.4.2 (microsoft#23276)
  [asio-grpc] Update to 1.4.0 (microsoft#23277)
  ...
@ekilmer ekilmer deleted the cpplocate branch March 25, 2022 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions GitHub Actions github-actions approved these changes

@JackBoosY JackBoosY JackBoosY approved these changes

Assignees

@JackBoosY JackBoosY

Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ekilmer @JackBoosY @scheibel @ras0219-msft