GNOME Bugzilla – Bug 787013
Port to meson build system
Last modified: 2018-03-15 22:27:15 UTC
Port to meson build system.
Created attachment 358750 [details] [review] Updated autotools to use libgd module at subproject directory This patch updates libgd module location from source root to subprojects directory, which helps porting to meson.
Created attachment 358752 [details] [review] Port to meson build system Patch that adds meson build system along autotools. I would like to highlight the following points: - 'Makefile-js.am' is used to 'build' config.js and path.js, but these files don't seem to be used anymore. 'path.js' also does not exist anymore. - 'gd-main-view.h' and 'gd-main-view-generic.h' are used to generate 'org.gnome.Documents.enums.xml', but due to how libgd includes those headers, there is no clean way to include them. - A post install script is used to create the links for gnome-books and gnome-documents, but meson is not able to track the files created with an install script.
Created attachment 358753 [details] [review] Remove autotools Some applications are removing autotools to avoid the burden of maintaining two build systems. This patch removes autotools in case it is considered.
Finally, I have created a wip branch[0] to ease it's testing. Any suggestion would be very much appreciated. https://git.gnome.org/browse/gnome-documents/log/?h=wip/inigomartinez/meson
Thanks for the patches. Since we are in the freezes for GNOME 3.26, it is probably best to merge these early in the next cycle.
Created attachment 359797 [details] [review] Move libgd module Updated patch with proper subject and commit message.
Created attachment 359799 [details] [review] Port to meson build system An updated patch that bumps version number to 3.26.0. It also has some minor fixes to post install script.
Created attachment 359800 [details] [review] Remove autotools A follow up update to the remove autotools patch.
Finally, the wip branch[0] has also been updated to ease it's testing. [0] https://git.gnome.org/browse/gnome-documents/log/?h=wip/inigomartinez/meson
Review of attachment 359797 [details] [review]: Looks good. ::: configure.ac @@ +110,3 @@ tagged-entry gir +][subprojects/libgd]) I don't understand this construct, but it's the same one used in totem, so I guess that's fine.
Review of attachment 359799 [details] [review]: Looks good otherwise. I think you can commit after branching. Please make sure to catch somebody on IRC before this, so that we can make changes to continuous/jhbuild/Flatpak builds when that happens. ::: data/meson.build @@ +56,3 @@ + +info_names = [ + ['org.gnome.Books.appdata.xml.in', 'org.gnome.Books.metainfo.xml'], Could you rename those appdata.xml.in to metainfo.xml.in in a preparatory patch? ::: meson.build @@ +82,3 @@ +] + +if host_machine.system().contains('darwin') What's this for? ::: meson_options.txt @@ +1,1 @@ +option('enable-documentation', type: 'boolean', value: false, description: 'build documentation') I think we said that we don't want enable or disable to appear, as there's already a boolean. The default in autotools is "true". ::: meson_post_install.py @@ +8,3 @@ + +if not os.environ.get('DESTDIR'): + # FIXME: meson will not track the creation of these files Please add a link to the meson RFE for this.
Review of attachment 359800 [details] [review]: That's fine to do after we've committed and migrated to meson. Let's leave autotools around until we're sure we didn't break anything.
*** Bug 787684 has been marked as a duplicate of this bug. ***
Thank you very much for reviewing these patches! I've already updated the patches with your comments, and I'm trying to contact Debarshi Ray on IRC for he to tell me how to proceed. Some comments below. (In reply to Bastien Nocera from comment #10) > Review of attachment 359797 [details] [review] [review]: > > Looks good. > > ::: configure.ac > @@ +110,3 @@ > tagged-entry > gir > +][subprojects/libgd]) > > I don't understand this construct, but it's the same one used in totem, so I > guess that's fine. This is because of this: https://github.com/jessevdk/libgd/blob/master/libgd.m4#L33 (In reply to Bastien Nocera from comment #11) > Review of attachment 359799 [details] [review] [review]: > > Looks good otherwise. I think you can commit after branching. Please make > sure to catch somebody on IRC before this, so that we can make changes to > continuous/jhbuild/Flatpak builds when that happens. > > ::: data/meson.build > @@ +56,3 @@ > + > +info_names = [ > + ['org.gnome.Books.appdata.xml.in', 'org.gnome.Books.metainfo.xml'], > > Could you rename those appdata.xml.in to metainfo.xml.in in a preparatory > patch? Sure! It's done now. > ::: meson.build > @@ +82,3 @@ > +] > + > +if host_machine.system().contains('darwin') > > What's this for? Gettext autotools support does check if CFLocaleCopyCurrent, CFPreferencesCopyAppValue exist in the CoreFoundation framework which seems to be used on Mac OS X. Although I don't think this is useful in gnome-documents, I don't have an OS X to test this, so I've implemented the same autotools behaviour in meson just in case it's needed. Probably standard header checking is also not useful, which makes meson slightly slower: > +# headers > +check_headers = [ > + ['HAVE_DLFCN_H', 'dlfcn.h'], > + ['HAVE_INTTYPES_H', 'inttypes.h'], > + ['HAVE_MEMORY_H', 'memory.h'], > + ['HAVE_STDINT_H', 'stdint.h'], > + ['HAVE_STDLIB_H', 'stdlib.h'], > + ['HAVE_STRINGS_H', 'strings.h'], > + ['HAVE_STRING_H', 'string.h'], > + ['HAVE_SYS_STAT_H', 'sys/stat.h'], > + ['HAVE_SYS_TYPES_H', 'sys/types.h'], > + ['HAVE_UNISTD_H', 'unistd.h'] > +] > + > +foreach header: check_headers > + config_h.set(header[0], cc.has_header(header[1])) > +endforeach If you think it's convenient to remove these checks, I will be happy to do so. > ::: meson_options.txt > @@ +1,1 @@ > +option('enable-documentation', type: 'boolean', value: false, description: > 'build documentation') > > I think we said that we don't want enable or disable to appear, as there's > already a boolean. > The default in autotools is "true". Although I'm aware of the discussion about meson options naming[0], there does not seem to be any agreement yet. Until then I'm keeping the naming of autotools options so they don't confuse package developers/maintainers. I would happy to rename the options once such agreement happens. > ::: meson_post_install.py > @@ +8,3 @@ > + > +if not os.environ.get('DESTDIR'): > + # FIXME: meson will not track the creation of these files > > Please add a link to the meson RFE for this. I don't know if there is any public reference about this, at least I'm not aware of any. However you can find this in meson's source code: https://github.com/mesonbuild/meson/blob/master/mesonbuild/scripts/uninstall.py#L39 I've extended the comment. However, I would like to fix it someday. [0] https://mail.gnome.org/archives/desktop-devel-list/2017-July/msg00000.html
(In reply to Iñigo Martínez from comment #14) > I've already updated the patches with your comments, and I'm trying to > contact Debarshi Ray on IRC for he to tell me how to proceed. A gnome-3-26 branch is now in place. I see that Bastien has ACKed your patches, so feel free to push them, except the Autotools removal. Let's keep Autotools until we have shaken out any bugs or fallout from the Meson port.
Patches are now in master: - 9711254: build: Move libgd module to subprojects - 71403a6: build: Rename AppStream XML files - 9af110f: build: Port to meson build system Please test it as much as you can, and let me know any problems you may have.
Thanks for your work!
Comment on attachment 359797 [details] [review] Move libgd module Pushed as 9711254 - build: Move libgd module to subprojects
Comment on attachment 359799 [details] [review] Port to meson build system Pushed as 9af110f - build: Port to meson build system
Created attachment 360073 [details] [review] Remove autotools Slightly modified patch updated witch changes on master.
Created attachment 360074 [details] [review] Rename the names of the icons files This patch comes from 787684[0]. The names of icons' files, besides containing the final names, they also contained the path in which they were going to be installed. This implies that they have to be renamed on the installation process. Nautilus is an example of the same tree structure[1] which has also been ported to meson[2] and faced similar problems[3]. Given the fact that autotools could be removed, this patch depends on meson. [0] https://bugzilla.gnome.org/show_bug.cgi?id=787684 [1] https://git.gnome.org/browse/nautilus/tree/data/icons [2] https://bugzilla.gnome.org/show_bug.cgi?id=778167 [3] https://github.com/mesonbuild/meson/issues/1487
(In reply to Debarshi Ray from comment #17) > Thanks for your work! You are welcome! Let's see how this evolves.
(In reply to Iñigo Martínez from comment #24) > Comment on attachment 359799 [details] [review] [review] > Port to meson build system > > Pushed as 9af110f - build: Port to meson build system diff --git a/src/org.gnome.Books.in b/src/org.gnome.Books.in old mode 100644 new mode 100755 diff --git a/src/org.gnome.Documents.in b/src/org.gnome.Documents.in old mode 100644 new mode 100755 Is that on purpose?
(In reply to Piotr Drąg from comment #25) > (In reply to Iñigo Martínez from comment #24) > > Comment on attachment 359799 [details] [review] [review] [review] > > Port to meson build system > > > > Pushed as 9af110f - build: Port to meson build system > > diff --git a/src/org.gnome.Books.in b/src/org.gnome.Books.in > old mode 100644 > new mode 100755 > diff --git a/src/org.gnome.Documents.in b/src/org.gnome.Documents.in > old mode 100644 > new mode 100755 > > > Is that on purpose? Yes, although input files are actually different files, meson applies the same permissions from the original file in the new generated file. Those input files are used to build $datadir/gnome-documents/org.gnome.[Books|Documents] files, which must actually be executable files. They have the `shebang` pointing to the `gjs-console`, and the books and documents "binaries" in the $bindir directory are in fact links to these files.
Now it’s clear, thanks!
Created attachment 361310 [details] [review] Fix AppStream metainfo file name This patchs fixes the mistake done in commit `71403a6: build: Rename AppStream XML files`, where AppStream metainfo files were renamed by using the `%{id}.metainfo.xml` format, which is incorrect because applications are a special case[0] and they have to use the `%{id}.appdata.xml` format. This patch modifies the metainfo file names, and updates both autotools and meson. [0] https://www.freedesktop.org/software/appstream/docs/sect-Metadata-Application.html
Regarding "Remove autotools" patch[0], if it's finally merged there are steps before/after to be done. One of the steps is related to `gnome-continuous`[1]. Actually the `configure` script is not strictly necessary, as `gnome-continuous` may patch it on the fly[2]. It's up to the maintainer to include it. Another step is related to `jhbuild`. It must be updated to set the build system to meson. This also implies a release with meson build system merged. There is more information in the GNOME wiki[3]. These are also notes for me. I know how to proceed with these changes, so I can take responsibility for making the appropriate changes, but I have to know when the patch is going to be merged. BTW, I've done a quick search regarding issues with meson, but I haven't found any. Have you had any issue with it? [0] https://bugzilla.gnome.org/attachment.cgi?id=361624 [1] https://mail.gnome.org/archives/desktop-devel-list/2017-April/msg00080.html [2] https://git.gnome.org/browse/gnome-continuous/tree/patches [3] https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Created attachment 363936 [details] [review] build: Remove autotools I've revisited the meson port due to the new meson's porting guidelines, and there are new patches to be applied. There is a wip branch[0] to ease the review and testing of the whole changes. I've noticed that I left some comments in the meson build files. I would like to highlight the comment in `src/meson.build` regarding `config.js` and `path.js`, because they don't seem to be used anymore. This is the first patch which belongs to the `Remove autotools` patch that has been slightly modified, so here goes an update. [0] https://git.gnome.org/browse/gnome-documents/log/?h=wip/inigomartinez/meson
Created attachment 363937 [details] [review] build: Rename build options Following the new meson porting guidelines, this patch renames the build options as follows: - Remove the 'enable' prefix from boolean options. - The character separator in multi-word options has been changed to underscore.
Created attachment 363938 [details] [review] build: Remove default warning level gnome-documents uses 1 as the project's default warning level. However, meson already uses this level as the default warning level. This patch removes the warning level from project's default options.
Created attachment 363939 [details] [review] build: Remove unused defines meson generates the config.h file with multiple defines to be used as compile-time options, in the same way as autotools does. However, some of them are not used. This patch removes those unused defines.
Created attachment 363940 [details] [review] build: Rename build options I've forgotten to include the bug url in the commit description. It's fixed now.
Created attachment 363941 [details] [review] build: Remove default warning level I've forgotten to include the bug url in the commit description. It's fixed now.
Created attachment 363942 [details] [review] build: Remove unused defines I've forgotten to include the bug url in the commit description. It's fixed now.
Created attachment 363943 [details] [review] build: Use LINGUAS file for help generation The linguas parameter is deprecated in gnome's yelp function for the future meson's versions. The patch uses the LINGUAS file instead of the linguas parameter.
Review of attachment 361310 [details] [review]: ++ This was failing the build for me, so I pushed it to master.
Review of attachment 363941 [details] [review]: ++
Review of attachment 363940 [details] [review]: ++
Review of attachment 360074 [details] [review]: This will break the installation of icons using Autotools, and sprinkling Makefile.am in the sub-directories will interfere with Meson. Is there a way to avoid breaking Autotools? Otherwise, I think this should be squashed with the "remove autotools" commit, and pushed once we have decided to jump ship.
Created attachment 365248 [details] [review] build: Rename the names of the icons' files (In reply to Debarshi Ray from comment #41) > Review of attachment 360074 [details] [review] [review]: > > This will break the installation of icons using Autotools, and sprinkling > Makefile.am in the sub-directories will interfere with Meson. Is there a way > to avoid breaking Autotools? > > Otherwise, I think this should be squashed with the "remove autotools" > commit, and pushed once we have decided to jump ship. I'm sorry, the patch wasn't meant to be applied with autotools still in place. I had this applied in my branch just after the "remove autotools" patch. I've updated the patch by extending it also to autotools.
Created attachment 365274 [details] [review] build: Rename the names of the icons' files A minor update which changes `uninstall-local` rule for `uninstall-hook` in autotools.
I found two problems when building gnome-documents with meson: 1. There is no executable in bin. It seems that symlinks in bin are only created when running 'ninja install' without settings DESTDIR, but I think most people set DESTDIR when installing gnome-documents because it is required to build packages. 2. If the system has libgd, a graphics library available from https://libgd.github.io/, installed, g-ir-scanner will fail with undefined reference error because it finds the wrong libgd.so. I think it is similar to https://bugzilla.gnome.org/show_bug.cgi?id=787578. Will gnome-documents apply a workaround like what gnome-builder did, or it will be kept broken until meson fixes the problem? [1/2] Generating GdPrivate-1.0.gir with a custom command. FAILED: src/GdPrivate-1.0.gir /home/lantw44/gnome/devinstall/bin/g-ir-scanner -I/home/lantw44/gnome/devinstall/include/gobject-introspection-1.0 -I/home/lantw44/gnome/devinstall/include/glib-2.0 -I/home/lantw44/gnome/devinstall/lib/glib-2.0/include -pthread -I/usr/local/lib/libffi-3.2.1/include --no-libtool --namespace=GdPrivate --nsversion=1.0 --warn-all --output src/GdPrivate-1.0.gir --warn-all -I/home/lantw44/gnome/source/gnome-documents/src -I/home/lantw44/gnome/build/gnome-documents/src -I./. -I../../source/gnome-documents/. -I./src/lib -I../../source/gnome-documents/src/lib -I./subprojects/libgd/. -I../../source/gnome-documents/subprojects/libgd/. --filelist=/home/lantw44/gnome/build/gnome-documents/src/gdprivate-1.0@sha/GdPrivate_1.0_gir_filelist --include=GData-0.0 --include=GnomeDesktop-3.0 --include=Goa-1.0 --include=Gtk-3.0 --include=EvinceDocument-3.0 --include=EvinceView-3.0 --include=Zpj-0.0 --symbol-prefix=gd --identifier-prefix=Gd --cflags-begin -DHAVE_CONFIG_H -I./. -I../../source/gnome-documents/. -I./src/lib -I../../source/gnome-documents/src/lib -I./subprojects/libgd/. -I../../source/gnome-documents/subprojects/libgd/. -I/home/lantw44/gnome/devinstall/include/gjs-1.0 -I/home/lantw44/gnome/devinstall/include/glib-2.0 -I/home/lantw44/gnome/devinstall/lib/glib-2.0/include -I/home/lantw44/gnome/devinstall/include/gobject-introspection-1.0 -I/usr/local/lib/libffi-3.2.1/include -I/home/lantw44/gnome/devinstall/include/mozjs-52 -I/usr/local/include/cairo -I/usr/local/include/pixman-1 -I/usr/local/include/freetype2 -I/usr/local/include/libdrm -I/usr/local/include/libpng16 -include /home/lantw44/gnome/devinstall/include/mozjs-52/js/RequiredDefines.h -D_THREAD_SAFE -I/home/lantw44/gnome/devinstall/include/gtk-3.0 -I/home/lantw44/gnome/devinstall/include/pango-1.0 -I/home/lantw44/gnome/devinstall/include/harfbuzz -I/home/lantw44/gnome/devinstall/include/gdk-pixbuf-2.0 -I/home/lantw44/gnome/devinstall/include/gio-unix-2.0/ -I/home/lantw44/gnome/devinstall/include/atk-1.0 -I/home/lantw44/gnome/devinstall/include/at-spi2-atk/2.0 -I/home/lantw44/gnome/devinstall/include/at-spi-2.0 -I/usr/local/include/dbus-1.0 -I/usr/local/lib/dbus-1.0/include -pthread -I/home/lantw44/gnome/devinstall/include/evince/3.0 -I/home/lantw44/gnome/devinstall/include/gnome-desktop-3.0 -I/home/lantw44/gnome/devinstall/include/gsettings-desktop-schemas -I/home/lantw44/gnome/devinstall/include/libsoup-2.4 -I/usr/local/include/libxml2 -I/home/lantw44/gnome/devinstall/include/tracker-2.0 -I/home/lantw44/gnome/devinstall/include/tracker-2.0/libtracker-sparql -I/home/lantw44/gnome/devinstall/include/webkitgtk-4.0 -march=corei7 -B/home/lantw44/.local/bin -pipe -g3 -O0 --cflags-end -L/home/lantw44/gnome/build/gnome-documents/src -L/home/lantw44/gnome/devinstall/lib --extra-library=gjs --extra-library=gobject-2.0 --extra-library=glib-2.0 --extra-library=intl --extra-library=evdocument3 --extra-library=gtk-3 --extra-library=gdk-3 --extra-library=pangocairo-1.0 --extra-library=pango-1.0 -L/usr/local/lib --extra-library=atk-1.0 --extra-library=cairo-gobject --extra-library=cairo --extra-library=pthread --extra-library=gdk_pixbuf-2.0 --extra-library=gio-2.0 --extra-library=evview3 --extra-library=gthread-2.0 -pthread --extra-library=gnome-desktop-3 --extra-library=girepository-1.0 --extra-library=soup-2.4 --extra-library=tracker-control-2.0 --extra-library=tracker-sparql-2.0 --extra-library=gmodule-2.0 --extra-library=webkit2gtk-4.0 --extra-library=javascriptcoregtk-4.0 --extra-library=m --add-include-path=/home/lantw44/gnome/devinstall/share/gir-1.0 -L/home/lantw44/gnome/build/gnome-documents/src -L/home/lantw44/gnome/build/gnome-documents/subprojects/libgd/libgd --library gdprivate-1.0 -L/home/lantw44/gnome/devinstall/lib -L/usr/local/lib g-ir-scanner: link: clang -o /home/lantw44/gnome/build/gnome-documents/tmp-introspectd4b5k8tk/GdPrivate-1.0 -march=corei7 -B/home/lantw44/.local/bin -pipe -g3 -O0 /home/lantw44/gnome/build/gnome-documents/tmp-introspectd4b5k8tk/GdPrivate-1.0.o -L. -Wl,-rpath,. -Wl,--no-as-needed -lgdprivate-1.0 -lgjs -lgobject-2.0 -lglib-2.0 -lintl -levdocument3 -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lpthread -lgdk_pixbuf-2.0 -lgio-2.0 -levview3 -lgthread-2.0 -lgnome-desktop-3 -lgirepository-1.0 -lsoup-2.4 -ltracker-control-2.0 -ltracker-sparql-2.0 -lgmodule-2.0 -lwebkit2gtk-4.0 -ljavascriptcoregtk-4.0 -lm -L/home/lantw44/gnome/build/gnome-documents/src -Wl,-rpath,/home/lantw44/gnome/build/gnome-documents/src -L/home/lantw44/gnome/devinstall/lib -Wl,-rpath,/home/lantw44/gnome/devinstall/lib -L/usr/local/lib -Wl,-rpath,/usr/local/lib -L/home/lantw44/gnome/build/gnome-documents/src -Wl,-rpath,/home/lantw44/gnome/build/gnome-documents/src -L/home/lantw44/gnome/build/gnome-documents/subprojects/libgd/libgd -Wl,-rpath,/home/lantw44/gnome/build/gnome-documents/subprojects/libgd/libgd -L/home/lantw44/gnome/devinstall/lib -Wl,-rpath,/home/lantw44/gnome/devinstall/lib -L/usr/local/lib -Wl,-rpath,/usr/local/lib -L/home/lantw44/gnome/devinstall/lib -lgio-2.0 -lgobject-2.0 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lglib-2.0 -lintl -L/home/lantw44/gnome/devinstall/lib -L/usr/local/lib /home/lantw44/gnome/build/gnome-documents/src/libgdprivate-1.0.so: undefined reference to `gd_styled_text_renderer_add_class' /home/lantw44/gnome/build/gnome-documents/src/libgdprivate-1.0.so: undefined reference to `gd_styled_text_renderer_get_type' /home/lantw44/gnome/build/gnome-documents/src/libgdprivate-1.0.so: undefined reference to `gd_styled_text_renderer_new' clang: error: linker command failed with exit code 1 (use -v to see invocation) linking of temporary binary failed: Command '['clang', '-o', '/home/lantw44/gnome/build/gnome-documents/tmp-introspectd4b5k8tk/GdPrivate-1.0', '-march=corei7', '-B/home/lantw44/.local/bin', '-pipe', '-g3', '-O0', '/home/lantw44/gnome/build/gnome-documents/tmp-introspectd4b5k8tk/GdPrivate-1.0.o', '-L.', '-Wl,-rpath,.', '-Wl,--no-as-needed', '-lgdprivate-1.0', '-lgjs', '-lgobject-2.0', '-lglib-2.0', '-lintl', '-levdocument3', '-lgtk-3', '-lgdk-3', '-lpangocairo-1.0', '-lpango-1.0', '-latk-1.0', '-lcairo-gobject', '-lcairo', '-lpthread', '-lgdk_pixbuf-2.0', '-lgio-2.0', '-levview3', '-lgthread-2.0', '-lgnome-desktop-3', '-lgirepository-1.0', '-lsoup-2.4', '-ltracker-control-2.0', '-ltracker-sparql-2.0', '-lgmodule-2.0', '-lwebkit2gtk-4.0', '-ljavascriptcoregtk-4.0', '-lm', '-L/home/lantw44/gnome/build/gnome-documents/src', '-Wl,-rpath,/home/lantw44/gnome/build/gnome-documents/src', '-L/home/lantw44/gnome/devinstall/lib', '-Wl,-rpath,/home/lantw44/gnome/devinstall/lib', '-L/usr/local/lib', '-Wl,-rpath,/usr/local/lib', '-L/home/lantw44/gnome/build/gnome-documents/src', '-Wl,-rpath,/home/lantw44/gnome/build/gnome-documents/src', '-L/home/lantw44/gnome/build/gnome-documents/subprojects/libgd/libgd', '-Wl,-rpath,/home/lantw44/gnome/build/gnome-documents/subprojects/libgd/libgd', '-L/home/lantw44/gnome/devinstall/lib', '-Wl,-rpath,/home/lantw44/gnome/devinstall/lib', '-L/usr/local/lib', '-Wl,-rpath,/usr/local/lib', '-L/home/lantw44/gnome/devinstall/lib', '-lgio-2.0', '-lgobject-2.0', '-Wl,--export-dynamic', '-lgmodule-2.0', '-pthread', '-lglib-2.0', '-lintl', '-L/home/lantw44/gnome/devinstall/lib', '-L/usr/local/lib']' returned non-zero exit status 1. ninja: build stopped: subcommand failed. I added --verbose to the command line of ld, and it printed the following messages: GNU ld (GNU Binutils) 2.28 ================================================== ... attempt to open ./libgdprivate-1.0.so failed attempt to open ./libgdprivate-1.0.a failed attempt to open /home/lantw44/gnome/build/gnome-documents/src/libgdprivate-1.0.so succeeded -lgdprivate-1.0 (/home/lantw44/gnome/build/gnome-documents/src/libgdprivate-1.0.so) ... libgd.so needed by /home/lantw44/gnome/build/gnome-documents/src/libgdprivate-1.0.so attempt to open ./libgd.so failed attempt to open /home/lantw44/gnome/build/gnome-documents/src/libgd.so failed attempt to open /home/lantw44/gnome/devinstall/lib/libgd.so failed found libgd.so at /usr/local/lib/libgd.so The libgd.so it should use is located in /home/lantw44/gnome/build/gnome-documents/subprojects/libgd/libgd, but it found /usr/local/lib/libgd.so, which is an unrelated library with the same name.
Created attachment 365897 [details] [review] build: Create always links in bindir > 1. There is no executable in bin. It seems that symlinks in bin are only created when running 'ninja install' without settings DESTDIR, but I think most people set DESTDIR when installing gnome-documents because it is required to build packages. This patch fixes this. I suppose that it makes sense to create them always. > 2. If the system has libgd, a graphics library available from https://libgd.github.io/, installed, g-ir-scanner will fail with undefined reference error because it finds the wrong libgd.so. I think it is similar to https://bugzilla.gnome.org/show_bug.cgi?id=787578. Will gnome-documents apply a workaround like what gnome-builder did, or it will be kept broken until meson fixes the problem? This doesn't have an easy solution. gnome-builder had easier to workaround the problem because it wasn't almost using gd. I don't know what is the status of this, but by chance have you tried with one of the latest meson's versions? (0.44 - 0.45.0-dev?)
(In reply to Iñigo Martínez from comment #45) > Created attachment 365897 [details] [review] [review] > build: Create always links in bindir > > > 1. There is no executable in bin. It seems that symlinks in bin are only created when running 'ninja install' without settings DESTDIR, but I think most people set DESTDIR when installing gnome-documents because it is required to build packages. > > This patch fixes this. I suppose that it makes sense to create them always. > This patch doesn't respect the value of DESTDIR, so these two symlinks become untracked files in jhbuild. I think it is also not going to work for distributions because users used to build packages usually don't have permission to write to the real bindir on the system. > > 2. If the system has libgd, a graphics library available from https://libgd.github.io/, installed, g-ir-scanner will fail with undefined reference error because it finds the wrong libgd.so. I think it is similar to https://bugzilla.gnome.org/show_bug.cgi?id=787578. Will gnome-documents apply a workaround like what gnome-builder did, or it will be kept broken until meson fixes the problem? > > This doesn't have an easy solution. gnome-builder had easier to workaround > the problem because it wasn't almost using gd. > > I don't know what is the status of this, but by chance have you tried with > one of the latest meson's versions? (0.44 - 0.45.0-dev?) I tested the build with meson from git master branch again. It still failed with the same error.
Created attachment 365977 [details] [review] build: DESTDIR support for link creation (In reply to Ting-Wei Lan from comment #46) > This patch doesn't respect the value of DESTDIR, so these two symlinks > become untracked files in jhbuild. I think it is also not going to work for > distributions because users used to build packages usually don't have > permission to write to the real bindir on the system. I'm sorry. I've never done any packaging for distributions so I don't know how DESTDIR works. Here goes an update, which preprends DESTDIR in links creation. I hope this is correct now. However, I'm in doubt regarding DESTDIR and yelp generated help. Aren't the image links going to break when DESTDIR is set? > I tested the build with meson from git master branch again. It still failed > with the same error. Thank you for testing it. I've been looking to the meson's issues and I've found number of them which are related[0][1][2], and almost all of them have a message from you :) [1] seems that is directly related to this problem. Do you think that it may help this issue? [0] https://github.com/mesonbuild/meson/pull/1928 [1] https://github.com/mesonbuild/meson/pull/1932 [2] https://github.com/mesonbuild/meson/pull/2800
Created attachment 365987 [details] [review] build: DESTDIR support for link creation Ting-Wei has helped me with this so it should be working properly now. The links should not use the DESTDIR prefix, even if the links are broken.
(In reply to Iñigo Martínez from comment #48) > Created attachment 365987 [details] [review] [review] > build: DESTDIR support for link creation > > Ting-Wei has helped me with this so it should be working properly now. The > links should not use the DESTDIR prefix, even if the links are broken. Yes, it works now. Links are not broken. They will work when files are installed into the system by a package manager. (In reply to Iñigo Martínez from comment #47) > > I tested the build with meson from git master branch again. It still failed > > with the same error. > > Thank you for testing it. > > I've been looking to the meson's issues and I've found number of them which > are related[0][1][2], and almost all of them have a message from you :) > > [1] seems that is directly related to this problem. Do you think that it may > help this issue? > > [0] https://github.com/mesonbuild/meson/pull/1928 > [1] https://github.com/mesonbuild/meson/pull/1932 > [2] https://github.com/mesonbuild/meson/pull/2800 Yes, these are required fixes for meson to work in JHBuild on FreeBSD.
That just leaves: https://github.com/mesonbuild/meson/pull/2800 which I get the FreeBSD meson port can include to allow us to remove autotools before 3.28, right?
Review of attachment 363942 [details] [review]: Looks good.
Review of attachment 363943 [details] [review]: Looks good as well.
Review of attachment 365274 [details] [review]: Looks a bit messy, but fine overall.
Review of attachment 365987 [details] [review]: And this as well.
The DESTDIR patch needs to be updated for the icons patch. Iñigo, could you also update the "remove autotools" patch? This one should be a bit easier ;)
(In reply to Bastien Nocera from comment #50) > That just leaves: > https://github.com/mesonbuild/meson/pull/2800 > which I get the FreeBSD meson port can include to allow us to remove > autotools before 3.28, right? This pull request doesn't fix the libgd problem and I still have to manually delete libgd graphics library when building gnome-documents. I think it can also affect other distributions, especially source-based distributions where users compile packages on their own machines.
Comment on attachment 363942 [details] [review] build: Remove unused defines Pushed as bf131fd - build: Remove unused defines
Comment on attachment 363943 [details] [review] build: Use LINGUAS file for help generation Pushed as 2b4796b - build: Use LINGUAS file for help generation
Comment on attachment 365274 [details] [review] build: Rename the names of the icons' files Pushed as 759f193 - build: Rename the names of the icons' files
Created attachment 366760 [details] [review] build: DESTDIR support for link creation I'm sorry, it seems that I have forgotten to update the patch. Here goes the proper patch.
Comment on attachment 366760 [details] [review] build: DESTDIR support for link creation Pushed as a5862c2 - build: DESTDIR support for link creation
Created attachment 366761 [details] [review] build: Remove autotools Here goes an updated patch to remove autotools. (In reply to Ting-Wei Lan from comment #56) > This pull request doesn't fix the libgd problem and I still have to manually > delete libgd graphics library when building gnome-documents. I think it can > also affect other distributions, especially source-based distributions where > users compile packages on their own machines. I also though that the PR fixed the issue. If it doesn't, it's something that should be addressed at some point. I'm also worried about meson not able to pack the generated documentation in the distributable file[0]. [0] https://github.com/mesonbuild/meson/issues/2166
Created attachment 369574 [details] [review] build: Unbreak 'make distcheck'
I have now released gnome-documents-3.27.92 with Meson and "ninja dist". While doing so, I noticed that it's not possible to build a Autotools-generated tarball with Meson, but the opposite is possible, which is fine, I guess.
Review of attachment 366761 [details] [review]: There's a gnome-3-28 branch now. I see that Continuous, the nightly Flatpak and jhbuild are using Meson already. So, I vote for removing the Autotools build in master for the GNOME 3.29.x cycle.
Thanks for the Meson port, Iñigo, and everybody else who contributed!
Iñigo, could you add a dependency check for gepub? It's not technically a build dependency (although autotools does check for it during build) but it is a runtime dependency via the libgepub gir. gnome-documents 3.27.92 was released this week depending on libgepub 0.6 which hasn't been released yet. I'm guessing that the GNOME Release Team didn't notice the issue because it builds fine with meson. https://gitlab.gnome.org/GNOME/libgepub/issues/5
Comment on attachment 366761 [details] [review] build: Remove autotools Pushed as 673b0a6 - build: Remove autotools
Created attachment 369704 [details] [review] build: Add dependency over libgepub-0.6 (In reply to Jeremy Bicha from comment #67) > Iñigo, > > could you add a dependency check for gepub? It's not technically a build > dependency (although autotools does check for it during build) but it is a > runtime dependency via the libgepub gir. This patch adds dependency over libgepub-0.6. It just checks if libgepub-0.6 is available. However, I've not been able to check it properly here due to some issues with jhbuild. Could you please check it?
Created attachment 369705 [details] [review] build: Add dependency over libgepub-0.6 Updated to include the bug number in the commit description.
Created attachment 369706 [details] [review] build: Remove unnecessary libgd with-tagged-entry option libgd does not use the `with-tagged-entry` anymore so there is no need to set the option.
Created attachment 369707 [details] [review] build: Change file centric approach meson uses a file centric approach, which means that for every file available it loops all the available applications. However, every file related to applications are duplicated so actually the application approach is the right one. Following this approach all the files are handled by every available application.
Created attachment 369708 [details] [review] build: Remove meson's desktop directory variable There is a variable available for the directory where desktop files are installed. However, this variable is used only once. The variable has been removed and the path is used directly.
Created attachment 369709 [details] [review] build: Improve introspection generation Introspection generation build commands have been improved by creating variables for gnome-documents namespace, which are reused, and replacing GIR and Typelib paths variables by their values as they are used only once.
(In reply to Iñigo Martínez from comment #69) > This patch adds dependency over libgepub-0.6. It just checks if libgepub-0.6 > is available. However, I've not been able to check it properly here due to > some issues with jhbuild. Could you please check it? I have fixed my issues with pkg-config (actually the problem wasn't located in jhbuild) and tested the patch. AFAIK, it's correct and it works. However, as always, I would appreciate very much a review and/or any comment.
(In reply to Iñigo Martínez from comment #69) > Created attachment 369704 [details] [review] [review] > build: Add dependency over libgepub-0.6 > > (In reply to Jeremy Bicha from comment #67) > > Iñigo, > > > > could you add a dependency check for gepub? It's not technically a build > > dependency (although autotools does check for it during build) but it is a > > runtime dependency via the libgepub gir. > > This patch adds dependency over libgepub-0.6. It just checks if libgepub-0.6 > is available. However, I've not been able to check it properly here due to > some issues with jhbuild. Could you please check it? Yes, the libgepub0.6 check patch works here and is helpful. :)
Review of attachment 369705 [details] [review]: LGTM
Review of attachment 369706 [details] [review]: This commit does not do what the commit message says -- it removes the with-view-common option.
Review of attachment 369707 [details] [review]: Nice cleanup
Comment on attachment 369705 [details] [review] build: Add dependency over libgepub-0.6 Pushed as 238043d - build: Add dependency over libgepub-0.6
Review of attachment 369708 [details] [review]: LGTM
Review of attachment 369709 [details] [review]: LGTM
(In reply to Cosimo Cecchi from comment #78) > Review of attachment 369706 [details] [review] [review]: > > This commit does not do what the commit message says -- it removes the > with-view-common option. Sorry, my mistake, I have written it wrong. And I've also made a bigger mistake because I had this change in the master branch when I pushed the last commit. I'm very sorry. Is there any way to fix this?
Comment on attachment 369707 [details] [review] build: Change file centric approach Pushed as 6149912 - build: Change file centric approach
Comment on attachment 369708 [details] [review] build: Remove meson's desktop directory variable Pushed as 97dc10b - build: Remove meson's desktop directory variable
Comment on attachment 369709 [details] [review] build: Improve introspection generation Pushed as 0144d8c - build: Improve introspection generation
attachment 369706 [details] [review] has been pushed as 921b89a. I can think of a way to fix it in the master branch. Please, let me know if you think about any solution for this. At least it does what it should, even if the description of the commit is wrong.
(In reply to Iñigo Martínez from comment #83) > (In reply to Cosimo Cecchi from comment #78) > > Review of attachment 369706 [details] [review] [review] [review]: > > > > This commit does not do what the commit message says -- it removes the > > with-view-common option. > > Sorry, my mistake, I have written it wrong. And I've also made a bigger > mistake because I had this change in the master branch when I pushed the > last commit. I'm very sorry. > > Is there any way to fix this? We can't change the commit message, but you can add a note mentioning that there's a typo in the commit message using "git notes": http://web.archive.org/web/20131105081943/http://git-scm.com/blog/2010/08/25/notes.html It's long, but it's really just a couple of commands. You should be able to see the note in cgit afterwards: https://git.gnome.org//browse/gnome-documents/log/
(In reply to Bastien Nocera from comment #88) > We can't change the commit message, but you can add a note mentioning that > there's a typo in the commit message using "git notes": > http://web.archive.org/web/20131105081943/http://git-scm.com/blog/2010/08/25/ > notes.html > > It's long, but it's really just a couple of commands. You should be able to > see the note in cgit afterwards: > https://git.gnome.org//browse/gnome-documents/log/ It worked! I've just added a note explaining my mistake. Thank you very much for your suport!
Closing the bug as all the changes are in.