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

[tomsolver] Add new port #42096

Merged
merged 2 commits into from
Nov 14, 2024
Merged

[tomsolver] Add new port #42096

merged 2 commits into from
Nov 14, 2024

Conversation

tomwillow
Copy link
Contributor

  • Changes comply with the maintainer guide.
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@tomwillow tomwillow marked this pull request as ready for review November 11, 2024 01:22
@Mengna-Li Mengna-Li added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Nov 11, 2024
@Mengna-Li Mengna-Li changed the title Tomsolver [tomsolver] Add new port Nov 11, 2024
ports/tomsolver/portfile.cmake Outdated Show resolved Hide resolved
The package tomsolver provides CMake targets:

find_package(tomsolver CONFIG REQUIRED)
target_link_libraries(main PRIVATE tomsolver::tomsolver)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a new line at the end of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I have modified it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got this information at this page:

Merging is blocked
Merging can be performed automatically once the requested changes are addressed.

I have added the new line at those two files, but this PR has not been merged automatically.
What should I do next?
Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Merging is not automatic. There official reviewer, and there people who merge what was flagged as reviewed.

@tomwillow tomwillow force-pushed the tomsolver branch 2 times, most recently from 9392760 to 65c2de4 Compare November 11, 2024 07:24
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

If the heuristical usage output is good enough, an explicit usage file is not needed.

ports/tomsolver/portfile.cmake Outdated Show resolved Hide resolved
ports/tomsolver/portfile.cmake Outdated Show resolved Hide resolved
ports/tomsolver/portfile.cmake Outdated Show resolved Hide resolved
ports/tomsolver/usage Outdated Show resolved Hide resolved
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

Sorry.

ports/tomsolver/portfile.cmake Outdated Show resolved Hide resolved
@tomwillow
Copy link
Contributor Author

If the heuristical usage output is good enough, an explicit usage file is not needed.

Sorry I am a newer and not know well with vcpkg's cmake machinism. I just follow this tutorial, so I don't know how to implement the "heuristical usage output".
Should I implement it? Or could you give me some help?
Thank you.

@dg0yt
Copy link
Contributor

dg0yt commented Nov 12, 2024

If a port doesn't have a usage file, vcpkg generates usage output using heuristics if it finds CMake config files or pkgconfig files. For simple projects (a single CMake target) this is usually good enough. And it removes the need to discuss wording and formatting ;-)

ports/tomsolver/portfile.cmake Outdated Show resolved Hide resolved
ports/tomsolver/portfile.cmake Outdated Show resolved Hide resolved
@tomwillow
Copy link
Contributor Author

If a port doesn't have a usage file, vcpkg generates usage output using heuristics if it finds CMake config files or pkgconfig files. For simple projects (a single CMake target) this is usually good enough. And it removes the need to discuss wording and formatting ;-)

I learnt it and removed the usage file.
Thank you for your patient explain.

@Mengna-Li
Copy link
Contributor

@tomwillow Usage test failed on x64-windows. could not find any header file.

@tomwillow
Copy link
Contributor Author

@tomwillow Usage test failed on x64-windows. could not find any header file.

I'm a bit confused because on this page, there is a message stating:

All checks have passed
16 successful checks

Additionally, I have tested on my local machine (Windows 10 x64) with the following steps:

  • use this manifest file(vcpkg.json):
{
  "dependencies": [
    "tomsolver"
  ]
}
  • run $ vcpkg install
  • After installation, I was able to find some C++ header files in the directory "vcpkg_installed\x64-windows\include\tomsolver".

Could you give me more details? Thanks.

@Mengna-Li
Copy link
Contributor

@tomwillow Usage test failed on x64-windows. could not find any header file.

I'm a bit confused because on this page, there is a message stating:

All checks have passed
16 successful checks

Additionally, I have tested on my local machine (Windows 10 x64) with the following steps:

  • use this manifest file(vcpkg.json):
{
  "dependencies": [
    "tomsolver"
  ]
}
  • run $ vcpkg install
  • After installation, I was able to find some C++ header files in the directory "vcpkg_installed\x64-windows\include\tomsolver".

Could you give me more details? Thanks.

Create a CMake project, add usage to CMakeLists.txt and include your header file in .cpp file. Please refer to https://learn.microsoft.com/en-us/vcpkg/get_started/get-started-vs?pivots=shell-powershell.

The reason for not being able to find the header file is that your CMakeList file is missing target_include-directories, and you need to modify the source file.

@tomwillow
Copy link
Contributor Author

@Mengna-Li Sorry for my mistake. I had previously tested on my machine, but my environment was not entirely clean.
I have since updated the tomsolver source repository and ensured that the example code compiles successfully using vcpkg on both Windows 10 x64 and Ubuntu 22.04.

@Mengna-Li
Copy link
Contributor

Usage tested pass on x64-windws.

@Mengna-Li Mengna-Li added the info:reviewed Pull Request changes follow basic guidelines label Nov 13, 2024
@tomwillow tomwillow requested a review from dg0yt November 13, 2024 05:54
@vicroms vicroms merged commit 03d773c into microsoft:master Nov 14, 2024
16 checks passed
@tomwillow tomwillow deleted the tomsolver branch November 14, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants