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

[libzim]add package #30758

Merged
merged 8 commits into from May 9, 2023
Merged

[libzim]add package #30758

merged 8 commits into from May 9, 2023

Conversation

xiaoyifang
Copy link
Contributor

@xiaoyifang xiaoyifang commented Apr 10, 2023

If this PR adds a new port, please uncomment and fill out this checklist:

  • 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.

@xiaoyifang xiaoyifang force-pushed the feature/libzim branch 2 times, most recently from b0b92d1 to 12dfa91 Compare April 10, 2023 16:33
@xiaoyifang xiaoyifang marked this pull request as draft April 10, 2023 23:46
@xiaoyifang xiaoyifang force-pushed the feature/libzim branch 2 times, most recently from 4347d63 to 4f6c2ff Compare April 11, 2023 00:29
@xiaoyifang xiaoyifang marked this pull request as ready for review April 11, 2023 00:29
@xiaoyifang xiaoyifang marked this pull request as draft April 11, 2023 00:45
@Adela0814 Adela0814 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Apr 11, 2023
@Adela0814 Adela0814 changed the title [new port]libzim: add package [libzim]add package Apr 11, 2023
@Adela0814
Copy link
Contributor

Is work still being done for this PR? Please ping us if this PR is ready for review.

@xiaoyifang
Copy link
Contributor Author

should hold for a while as libzim's windows release has some problem.

@xiaoyifang xiaoyifang force-pushed the feature/libzim branch 7 times, most recently from f12a1de to c10eddc Compare April 29, 2023 17:42
@xiaoyifang xiaoyifang marked this pull request as ready for review April 29, 2023 17:42
@xiaoyifang xiaoyifang marked this pull request as draft April 29, 2023 17:46
@xiaoyifang
Copy link
Contributor Author

I can build success on my local machine .
I do not know why it failed here. need some help.

@xiaoyifang xiaoyifang force-pushed the feature/libzim branch 3 times, most recently from e5b5028 to 8606aa2 Compare April 29, 2023 17:58
update libzim versions

libzim:meson ignore xapian
versions/l-/libzim.json Outdated Show resolved Hide resolved
ports/libzim/portfile.cmake Outdated Show resolved Hide resolved
ports/libzim/portfile.cmake Outdated Show resolved Hide resolved
ports/libzim/portfile.cmake Outdated Show resolved Hide resolved
ports/libzim/portfile.cmake Outdated Show resolved Hide resolved
ports/libzim/portfile.cmake Outdated Show resolved Hide resolved
ports/libzim/portfile.cmake Outdated Show resolved Hide resolved
@xiaoyifang xiaoyifang force-pushed the feature/libzim branch 5 times, most recently from 9e8b984 to 5f0bd44 Compare May 5, 2023 12:59
Update ports/libzim/portfile.cmake

Co-authored-by: Mengna Li <95600143+Adela0814@users.noreply.github.com>

add version
@xiaoyifang
Copy link
Contributor Author

vcpkg seems have encounter some build issue problem. the check task has not been finished in hours.

@xiaoyifang xiaoyifang marked this pull request as ready for review May 6, 2023 07:44
@Adela0814
Copy link
Contributor

@xiaoyifang Feature xapian test failed:
Repro step: vcpkg install libzim[xapian]:x64-windows
FAILED: src/zim-8.dll.p/version.cpp.obj "C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.34.31933/bin/Hostx64/x64/cl.exe" "-Isrc\zim-8.dll.p" "-Isrc" "-I..\src\8.2.0-10b574337e.clean\src" "-Iinclude" "-I..\src\8.2.0-10b574337e.clean\include" "-Istatic" "-IE:/2/vcpkg/installed/x64-windows/debug/../include" "-IE:/2/vcpkg/installed/x64-windows/include" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/EHsc" "/std:c++14" "/permissive-" "/Od" "-DNOMINMAX" "-nologo" "-DWIN32" "-D_WINDOWS" "-W3" "-utf-8" "-GR" "-EHsc" "-MP" "-D_DEBUG" "-MDd" "-Z7" "-Ob0" "-Od" "-RTC1" "-DSORTPP_PASS" "-DLIBZIM_EXPORT_DLL" "/Fdsrc\zim-8.dll.p\version.cpp.pdb" /Fosrc/zim-8.dll.p/version.cpp.obj "/c" ../src/8.2.0-10b574337e.clean/src/version.cpp cl : Command line warning D9025 : overriding '/W2' with '/W3' E:\2\vcpkg\installed\x64-windows\include\xapian/version.h(25): fatal error C1189: #error: You are compiling with _DEBUG defined, but the library
package-x64-windows-dbg-out.log

@Adela0814 Adela0814 marked this pull request as draft May 6, 2023 10:25
@xiaoyifang
Copy link
Contributor Author

xiaoyifang commented May 6, 2023

@xiaoyifang Feature xapian test failed: Repro step: vcpkg install libzim[xapian]:x64-windows FAILED: src/zim-8.dll.p/version.cpp.obj "C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.34.31933/bin/Hostx64/x64/cl.exe" "-Isrc\zim-8.dll.p" "-Isrc" "-I..\src\8.2.0-10b574337e.clean\src" "-Iinclude" "-I..\src\8.2.0-10b574337e.clean\include" "-Istatic" "-IE:/2/vcpkg/installed/x64-windows/debug/../include" "-IE:/2/vcpkg/installed/x64-windows/include" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/EHsc" "/std:c++14" "/permissive-" "/Od" "-DNOMINMAX" "-nologo" "-DWIN32" "-D_WINDOWS" "-W3" "-utf-8" "-GR" "-EHsc" "-MP" "-D_DEBUG" "-MDd" "-Z7" "-Ob0" "-Od" "-RTC1" "-DSORTPP_PASS" "-DLIBZIM_EXPORT_DLL" "/Fdsrc\zim-8.dll.p\version.cpp.pdb" /Fosrc/zim-8.dll.p/version.cpp.obj "/c" ../src/8.2.0-10b574337e.clean/src/version.cpp cl : Command line warning D9025 : overriding '/W2' with '/W3' E:\2\vcpkg\installed\x64-windows\include\xapian/version.h(25): fatal error C1189: #error: You are compiling with _DEBUG defined, but the library package-x64-windows-dbg-out.log

xapian only support release library. How can I disable debug library?
if I add the following line,
set(VCPKG_BUILD_TYPE release)?

the check will not pass with error

Debug binaries were not found.
Found 1 release binaries:

    D:\packages\libzim_arm64-windows\lib\zim.lib

warning: Mismatching number of debug and release binaries.
Debug binaries were not found.
Found 1 release binaries:

    D:\packages\libzim_arm64-windows\bin\zim-8.dll

error: Found 2 post-build check problem(s). 

@dg0yt
Copy link
Contributor

dg0yt commented May 7, 2023

xapian only support release library.

I don't think this is right. vcpkg builds both configurations.
However, vcpkg only keeps the release variant of the includes. In most cases it is no problem, but if the headers contain information about the build type, there will be trouble. This needs fixup in port xapian.

There is currently no other port which consumes xapian, and the problem might be specific to MSVC. I guess that's why it wasn't noticed before.
Instead of disabling debug (for all possible feature sets), try marking feature xapian as unsupported on window & !mingw. Fixing can be done in a separate PR.
("Unsupported" is not really true with regard to upstream, but IMO it is still the cleanest solution in vcpkg for now.)

@xiaoyifang
Copy link
Contributor Author

However, vcpkg only keeps the release variant of the includes. In most cases it is no problem, but if the headers contain information about the build type, there will be trouble. This needs fixup in port xapian.

This is in the xapian.h


#ifdef _MSC_VER
#ifdef _DEBUG
#error You are compiling with _DEBUG defined, but the library
#error was not compiled with this flag. The settings must match or your
#error program will not work correctly.
#endif
#endif

@xiaoyifang
Copy link
Contributor Author

xiaoyifang commented May 8, 2023

Build log #L22298
REGRESSIONS:
REGRESSION: libass:x64-android failed with BUILD_FAILED. If expected, add libass:x64-android=fail to /vcpkg/scripts/azure-pipelines/../ci.baseline.txt.
REGRESSION: azure-c-shared-utility:x64-android failed with BUILD_FAILED. If expected, add azure-c-shared-utility:x64-android=fail to /vcpkg/scripts/azure-pipelines/../ci.baseline.txt.
REGRESSION: libressl:x64-android failed with POST_BUILD_CHECKS_FAILED. If expected, add libressl:x64-android=fail to /vcpkg/scripts/azure-pipelines/..

weird , I do not think I changed libass?

I think I need to disable android support for now.

@xiaoyifang xiaoyifang marked this pull request as ready for review May 8, 2023 02:06
@xiaoyifang xiaoyifang requested a review from Adela0814 May 8, 2023 02:06
@dg0yt
Copy link
Contributor

dg0yt commented May 8, 2023

This is in the xapian.h

Yes, this was the base for my comment. It might even be possible to remove that code, but it needs more investigation. This doesn't need to be done in this PR.

Build log #L22298
...
weird , I do not think I changed libass?

Cannot check the logs because you didn't let that CI run finish.

@Adela0814 Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label May 8, 2023
@xiaoyifang
Copy link
Contributor Author

Yes, this was the base for my comment. It might even be possible to remove that code, but it needs more investigation. This doesn't need to be done in this PR.

#31338

@vicroms vicroms dismissed Adela0814’s stale review May 9, 2023 23:56

comments resolved

@vicroms vicroms merged commit e7250fe into microsoft:master May 9, 2023
15 checks passed
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.

None yet

4 participants