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

[litehtml] new port version 0.6 #29673

Merged
merged 12 commits into from
Feb 21, 2023
Merged

Conversation

rioki
Copy link
Contributor

@rioki rioki commented Feb 15, 2023

  • 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 docs/examples/adding-an-explicit-usage.md 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.

github-actions[bot]
github-actions bot previously approved these changes Feb 15, 2023
@rioki rioki changed the title Litehtml 0.6 [litehtml] new port version 0.6 Feb 15, 2023
github-actions[bot]
github-actions bot previously approved these changes Feb 15, 2023
@Neumann-A
Copy link
Contributor

Note: litehtml is vendored by qttools currently. I don't know if it is visible from the outside or just a part of qtassistent.

@jimwang118 jimwang118 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Feb 16, 2023
@rioki
Copy link
Contributor Author

rioki commented Feb 16, 2023

Note: litehtml is vendored by qttools currently. I don't know if it is visible from the outside or just a part of qtassistent.

As far as I can see the QT version runs as qlitehtml and has some changes to the code. It's API and ABI incompatible as it uses QT specific types and should be considered a sperate fork.

If I see it correctly qtassistent consumes qlitehtml as a static library and never installs anything.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 16, 2023

qlitehtml is a wrapper and needs litehtml. It will use an installed version if it is found:
https://code.qt.io/cgit/playground/qlitehtml.git/tree/src/CMakeLists.txt#n18

So AFAICT qttools must use the vcpkg version of litehtml if we don't want to have two providers of the same symbols.
Another questions is if we also want/need a qlitehtml port...

@Neumann-A
Copy link
Contributor

Another questions is if we also want/need a qlitehtml port...

I don't think that is currently needed.

@jimwang118
Copy link
Contributor

All test passed with following triplets:
x86-windows
x64-windows
x64-windows-static

JonLiu1993
JonLiu1993 previously approved these changes Feb 17, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Feb 17, 2023

IIUC litehtml still needs to be devendored from qttools.

@jimwang118 jimwang118 added the info:reviewed Pull Request changes follow basic guidelines label Feb 17, 2023
@rioki
Copy link
Contributor Author

rioki commented Feb 18, 2023

I will see if I can devendor litehtml this weekend form qttools.

@rioki rioki dismissed stale reviews from JonLiu1993 and github-actions[bot] via 2192ad3 February 19, 2023 11:12
@rioki
Copy link
Contributor Author

rioki commented Feb 19, 2023

I fixed the CMakeLists.txt in qlitehtml/src so that is used the vcpkg litehtml. git still pulls the submodule, but I have no idea how to fix that, since I can patch the .gitmodules but that happens only after the submodule was pulled.

@rioki
Copy link
Contributor Author

rioki commented Feb 19, 2023

Looking into why it fails on the CI (works on my machine™)

@rioki
Copy link
Contributor Author

rioki commented Feb 19, 2023

I fixed litehtml exposing the vendored gumbo for CMake to handle it's dependencies cleanly. The project is defunct, thus I don't think making a unique port is needed.

@JavierMatosD JavierMatosD merged commit 2f0f813 into microsoft:master Feb 21, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Feb 23, 2023

I fixed litehtml exposing the vendored gumbo for CMake to handle it's dependencies cleanly. The project is defunct, thus I don't think making a unique port is needed.

There is a gumbo port, and we have a file conflict in CI now.
https://dev.azure.com/vcpkg/public/_build/results?buildId=85761&view=logs&j=84da4f4a-968a-5b5c-16e9-f444c845396d&t=8cdf6620-a129-5b70-3f3e-822630a64446

@rioki
Copy link
Contributor Author

rioki commented Feb 23, 2023

Sorry for missing that. I will look into fixing this.

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.

6 participants