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

[harfbuzz] Use meson and update to 2.7.0 #12572

Closed
ebraminio opened this issue Jul 25, 2020 · 17 comments · Fixed by #12860
Closed

[harfbuzz] Use meson and update to 2.7.0 #12572

ebraminio opened this issue Jul 25, 2020 · 17 comments · Fixed by #12860
Assignees
Labels
category:port-update The issue is with a library, which is requesting update new revision

Comments

@ebraminio
Copy link
Contributor

ebraminio commented Jul 25, 2020

Library name: harfbuzz

New version number: 2.7.0

Other information that may be useful (release notes, etc...)
Please use meson port just as fribidi, I've also tried that ebraminio@681a100 and the problem is vcpkg_check_features should give enabled/disabled instead on/off for meson and there are some issue on install parts as lack of vcpkg_fixup_cmake_targets equivalent for meson, otherwise it is working locally at least as far as I can say. cc: @vejmartin

@vejmartin
Copy link
Contributor

@ebraminio, thank you the details! I picked up your work from 681a100 and got the macOS port working. However, I'm having issues passing the freetype, icu, glib and graphite2 dependencies to meson.

I tried using dependency(..., method : 'cmake') and cpp.find_library in meson. However, I couldn't get either of them to successfully link to the libraries in vcpkg/installed. Can you think of a way to specify them without using pkgconfig?

From the meson docs, it looks like we could set the CFLAGS, CXXFLAGS, and LDFLAGS envs manually, but this might not work with debug/release and vcpkg_configure_meson, and it will still require patching meson.build.

@ebraminio
Copy link
Contributor Author

ebraminio commented Jul 26, 2020

I tried using dependency(..., method : 'cmake') and cpp.find_library in meson. However, I couldn't get either of them to successfully link to the libraries in vcpkg/installed. Can you think of a way to specify them without using pkgconfig?

It tries both pkg-config and cmake, then fallbacks to cpp.find_library('freetype', https://github.com/harfbuzz/harfbuzz/blob/master/meson.build#L78 but just for MSVC, maybe that could work work if wasn't msvc limited?

Maybe we can ping @codicodi 6ca475a#diff-2dbb69c73b42618914cba39c2e271bb6 to help us out here, a help will be very appreciated as we like to have one unified build system (or at least more bounded version of our maintained cmake) and this is one of the blockers people are complaining about, thus blocking upstream progress :/

@LilyWangL LilyWangL added the category:port-update The issue is with a library, which is requesting update new revision label Jul 27, 2020
@vejmartin
Copy link
Contributor

I tried using the env variables (CFLAGS, CXXFLAGS, LDFLAGS) and replaced dependency('freetype2', required: false) with cpp.find_library. meson was then able to find freetype. However, it added it as -lfreetype instead of using the absolute path to the library file, and none of the-I or -L flags actually made it to the resulting build.ninja file.

I might be wrong, but by the looks of it, meson doesn't support linking to an external library via an absolute path. The implementation itself doesn't seem to store the paths to the library files either. A possible workaround could be to add a configuration option to disable library lookup from meson in harfbuzz/meson.build, and instead supply the link and include compiler flags from the command line directly, i.e -I/path-to-includes -l/path-to-library.lib.

@ebraminio, with respect to the vcpkg_check_features and vcpkg_fixup_cmake_targets comments, I made some changes here, vejmartin@e0157fa.

@vejmartin
Copy link
Contributor

@ebraminio, it appears more support for pkg-config has recently been added 🎉, 458c20e. Based on that, the were only a few more changes to get harfbuzz to build with freetype, vejmartin@3c36bcf.

There are already some PRs that cover pkg-config for the rest of the dependencies, so it might be best to delay this until they are in. See #7129

@ebraminio
Copy link
Contributor Author

it appears more support for pkg-config has recently been added

That's just fantastic!

so it might be best to delay this until they are in

Oh, this makes sense. Or if that that has been delayed maybe it can makes sense to peek the needed parts from the patch?

@vejmartin
Copy link
Contributor

maybe it can makes sense to peek the needed parts from the patch?

The patch relies on some changes to vcpkg itself, so I'm not sure about copying those over.

icu should now be covered as well, vejmartin@cb30df9. I think graphite2 has support for pkg-config. @ebraminio, could you have a look at glib? It's failing to build on macOS.

@ebraminio
Copy link
Contributor Author

ebraminio commented Jul 28, 2020

@vejmartin fetched your patch and ran that locally and it apparently works on Linux? Don't have macOS nowadays unfortunately :/ What should I see if it fails? Also please use --wrap-mode=nodownload as meson argument (I should've do it myself) so that harfbuzz won't use its internal mechanism for fetching dependencies, oh nvm, is already there so feel free to drop backend ninja also

@ebraminio
Copy link
Contributor Author

Just to note, many of dependencies vcpkg tries its best to wire up for harfbuzz is just unnecessary, there was a time that harfbuzz didn't have unicode data as main part of the library and now we have that and recommend it actually, I doubt anyone use vcpkg would need these three APIs hb-glib provides, hb_script_t hb_glib_script_to_script (GUnicodeScript script), GUnicodeScript hb_glib_script_from_script (hb_script_t script), hb_unicode_funcs_t *hb_glib_get_unicode_funcs (void), enabling as much features as possible does make sense for Linux packagers but not for things like vcpkg that its sole propose is to library embedding easier.

@vejmartin
Copy link
Contributor

fetched your patch and ran that locally and it apparently works on Linux

Glad to hear it's working on Linux :)

I doubt anyone use vcpkg would need these three APIs hb-glib provides

It might be worth to double-check the pango port, i.e. harfbuzz[glib] (!(windows&static)&!osx) (from ports/pango/CONTROL).

@ebraminio
Copy link
Contributor Author

It might be worth to double-check the pango port, i.e. harfbuzz[glib] (!(windows&static)&!osx) (from ports/pango/CONTROL).

Ops, Pango uses it.

harfbuzz[glib]

Oh should I use such so? Apparently my vcpkg knowledge has become so useless, should I have ran that using

./vcpkg install harfbuzz[glib]
zsh: no matches found: harfbuzz[glib]

or something?

@vejmartin
Copy link
Contributor

@ebraminio, I get that error with zsh as well, a workaround is to wrap it in quotes, ./vcpkg install "harfbuzz[glib]"

@ebraminio
Copy link
Contributor Author

ebraminio commented Jul 29, 2020

a workaround is to wrap it in quotes, ./vcpkg install "harfbuzz[glib]"

Now I see different complains, is this the referred issue?

Ops, Pango uses it.

Will try to remove the hb-glib use from Pango however, guess then we can pick up that patch.

@ebraminio
Copy link
Contributor Author

ebraminio commented Jul 29, 2020

Will try to remove the hb-glib use from Pango however, guess then we can pick up that patch.

https://gitlab.gnome.org/GNOME/pango/-/merge_requests/213/diffs guess if you pick this up you won't need actually to compile hb-glib, this is what was we doing internally also harfbuzz/harfbuzz@44a3136 anyway.

@ebraminio
Copy link
Contributor Author

ebraminio commented Aug 3, 2020

As merge of https://gitlab.gnome.org/GNOME/pango/-/merge_requests/213/diffs since 1.45.5 release of Pango you should be able to compile pango without hb-glib, so feel free to update pango and go for harfbuzz meson update without hb-glib.

@vejmartin
Copy link
Contributor

@ebraminio, that's amazing! Thank you for covering Pango, I'll look into integrating the changes.

@vejmartin
Copy link
Contributor

@ebraminio, I looked into updating pango, however, the pango port needs to be rewritten, as the configuration is now entirely covered by meson.

@ebraminio
Copy link
Contributor Author

Ok, maybe you can pick up the patch? Hopefully that makes it work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants