After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 789118 - Port to meson build system
Port to meson build system
Status: RESOLVED OBSOLETE
Product: ghex
Classification: Applications
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: GHex maintainers
GHex maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-17 21:21 UTC by Martin Blanchard
Modified: 2021-05-25 17:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
All-in-one migration path (15.25 KB, patch)
2017-10-17 21:25 UTC, Martin Blanchard
committed Details | Review
build: Change header installation directory back to gtkhex-3 (790 bytes, patch)
2018-04-22 10:47 UTC, Ting-Wei Lan
committed Details | Review
Try to use consistent naming (4.16 KB, patch)
2018-05-12 13:49 UTC, Martin Blanchard
none Details | Review
Clean up the build process (2.66 KB, patch)
2018-05-12 13:50 UTC, Martin Blanchard
none Details | Review
Do not rely on template for pkg-config file (1.51 KB, patch)
2018-05-12 13:50 UTC, Martin Blanchard
none Details | Review
Try to use consistent naming (4.20 KB, patch)
2018-05-13 19:23 UTC, Martin Blanchard
none Details | Review
Add an option for yelp manual (1.48 KB, patch)
2018-05-13 19:25 UTC, Martin Blanchard
none Details | Review
Use smbolic links for yelp manual's media (618 bytes, patch)
2018-05-13 19:26 UTC, Martin Blanchard
none Details | Review

Description Martin Blanchard 2017-10-17 21:21:39 UTC
From the Meson official website [1]: 'Meson is an open source build system meant to be both extremely fast, and, even more importantly, as user friendly as possible'.

Porting modules to Meson is a GNOME Goal [2]. GHex should probably follow that trend.

[1] http://mesonbuild.com
[2] https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Comment 1 Martin Blanchard 2017-10-17 21:25:33 UTC
Created attachment 361769 [details] [review]
All-in-one migration path

Port should be complete.
Comment 2 Salamandar 2018-03-07 10:54:40 UTC
Hi,
I'm used to port projects to Meson (and I did port GHex yesterday, didn't see you already did all the work :'( ), so I may have some remarks, even though that's a good port you did.

# data/
You are using configure_file() to generate the pkgconfig file, why don't you use pkgconfig.generate() ? You won't need to use a configuration_data(), Meson handles everything.

# help/
The 'linguas' argument of gnome.yelp() is deprecated. You should dump the list of languages to a LINGUAS file (just as in the po/ dir).
Why 'symlink_media: false' ? symlinks would reduce the disk usage.
That's a real question, i don't know if there's a real reason.

# icons/
That doesn't change much, but you could just install_subdir() the whole 'hicolor' directory ;)

# src/
You don't need to list headers inside sources. Actually, they will be ignored and that's just more work to maintain the list.

libghex_dep = declare_dependency -> Is that really necessary ? Also, 'sources: libghex_headers' is *really* not what you want to do, I think.


# overall

* I personnally declared a variable containing 'gtkhex-3' so that the version number changes everything at the same time.
You sometimes wrote "'gtkhex-@0@.0'.format(libghex_version_major)", but not everywhere.

* Why check for all those headers/fns ? The project actually needs *none* of the 'HAVE_*' macros, so that's just noise in the meson code :/
* Same for 'ENABLE_NLS'

* The rest of my remarks are basically codestyle, so i'll keep my personnal tastes for myself ;)

BTW, just for reference if you want, my own port is here: https://github.com/Salamandar/ghex

Hope you'll agree on some on my points ;)

Salamandar
Comment 3 Kalev Lember 2018-03-07 10:59:59 UTC
Totally makes sense what you say!

How would this plan work for you both: First, we push Martin's initial port to master, and then Salamandar can do followup patches and fix up all the issues? :)
Comment 4 Salamandar 2018-03-07 11:05:32 UTC
That's perfectly okay with me :D
Obviously Martin should be okay with my future patches.
Comment 5 Kalev Lember 2018-03-07 11:14:27 UTC
Comment on attachment 361769 [details] [review]
All-in-one migration path

OK, here we go -- pushed Martin's patch to master! Thanks, Martin! Let's keep this bug for the followup fixes as well, as all the relevant people are already CC'd here.
Comment 6 Ting-Wei Lan 2018-04-22 10:44:27 UTC
There is a minor issue in the current meson build files. The port to meson changes the subdirectory for installing headers from gtkhex-3 to gtkhex-3.0. It makes it inconsistent with Cflags specified in gtkhex-3.pc and causes nemiver to fail to build.
Comment 7 Ting-Wei Lan 2018-04-22 10:47:39 UTC
Created attachment 371223 [details] [review]
build: Change header installation directory back to gtkhex-3

This is what autotools build used to use, and it makes it consistent
with the .pc file.
Comment 8 Kalev Lember 2018-04-22 11:01:15 UTC
Review of attachment 371223 [details] [review]:

+1 from me, thanks.
Comment 9 Kalev Lember 2018-04-22 11:02:44 UTC
Salamandar: did you have any further patches? You listed a bunch of issues above.
Comment 10 Ting-Wei Lan 2018-04-22 11:59:38 UTC
Comment on attachment 371223 [details] [review]
build: Change header installation directory back to gtkhex-3

Attachment 371223 [details] pushed as 71a64b0 - build: Change header installation directory back to gtkhex-3
Comment 11 Martin Blanchard 2018-05-12 10:55:16 UTC
(In reply to Salamandar from comment #2)

That was my first meson port, so yes, it was far from being perfect, sorry... Thanks for the review.

> # data/
> You are using configure_file() to generate the pkgconfig file, why don't you
> use pkgconfig.generate() ? You won't need to use a configuration_data(),
> Meson handles everything.

That was only keep because autotools were still in place and using it. We can probably remove it now, as autotools have been dropped.

> # help/
> The 'linguas' argument of gnome.yelp() is deprecated. You should dump the
> list of languages to a LINGUAS file (just as in the po/ dir).
> Why 'symlink_media: false' ? symlinks would reduce the disk usage.
> That's a real question, i don't know if there's a real reason.

Yep, time to move to LINGUAS file. I can't remember why I decided not to use symlinks at the time. Can find any reasons now...

> # icons/
> That doesn't change much, but you could just install_subdir() the whole
> 'hicolor' directory ;)

Sure, think I just wanted the name of the target directory to be a build system variable rather than the fs source directory name.
 
> # src/
> You don't need to list headers inside sources. Actually, they will be
> ignored and that's just more work to maintain the list.

Agreed.

> libghex_dep = declare_dependency -> Is that really necessary ? Also,
> 'sources: libghex_headers' is *really* not what you want to do, I think.

The dependency is only for convenience. You're right about the 'sources' though. 

> # overall
 >
> * I personnally declared a variable containing 'gtkhex-3' so that the
> version number changes everything at the same time.
> You sometimes wrote "'gtkhex-@0@.0'.format(libghex_version_major)", but not
> everywhere.

Agreed, we need consistency here.
 
> * Why check for all those headers/fns ? The project actually needs *none* of
> the 'HAVE_*' macros, so that's just noise in the meson code :/
> * Same for 'ENABLE_NLS'

Also agreed.
Comment 12 Martin Blanchard 2018-05-12 13:49:13 UTC
Created attachment 371958 [details] [review]
Try to use consistent naming
Comment 13 Martin Blanchard 2018-05-12 13:50:01 UTC
Created attachment 371959 [details] [review]
Clean up the build process
Comment 14 Martin Blanchard 2018-05-12 13:50:38 UTC
Created attachment 371960 [details] [review]
Do not rely on template for pkg-config file
Comment 15 Ting-Wei Lan 2018-05-13 14:51:08 UTC
Review of attachment 371958 [details] [review]:

This breaks translations. You move gettext files to ghex_string, but GETTEXT_PACKAGE is still PACKAGE_NAME, which is meson.project_name().
Comment 16 Martin Blanchard 2018-05-13 19:23:43 UTC
Created attachment 371979 [details] [review]
Try to use consistent naming

(In reply to Ting-Wei Lan from comment #15)
> Review of attachment 371958 [details] [review] [review]:
> 
> This breaks translations. You move gettext files to ghex_string, but
> GETTEXT_PACKAGE is still PACKAGE_NAME, which is meson.project_name().

Fixed, thank you.
Comment 17 Martin Blanchard 2018-05-13 19:25:12 UTC
Created attachment 371980 [details] [review]
Add an option for yelp manual
Comment 18 Martin Blanchard 2018-05-13 19:26:35 UTC
Created attachment 371981 [details] [review]
Use smbolic links for yelp manual's media
Comment 19 Jan Tojnar 2018-10-21 21:46:09 UTC
Review of attachment 371959 [details] [review]:

::: src/meson.build
@@ +2,2 @@
   'accessiblegtkhex.c',
   'accessiblegtkhex.h',

This can be removed as well
Comment 20 Jan Tojnar 2018-10-21 21:47:26 UTC
Review of attachment 371960 [details] [review]:

::: data/meson.build
@@ -35,3 @@
-
-configure_file(
-  input: 'gtkhex-3.pc.in',

You can delete the file now.
Comment 21 Jan Tojnar 2018-10-21 21:55:57 UTC
Review of attachment 371979 [details] [review]:

::: meson.build
@@ +14,3 @@
 
+ghex_name = meson.project_name()
+ghex_string = '@0@-@1@'.format(ghex_name, ghex_version_major)

Maybe use something more descriptive like ghex_pc_name

@@ +19,3 @@
+
+libghex_name = 'gtkhex'
+libghex_string = '@0@-@1@'.format(libghex_name, libghex_version_major)

Maybe use something more descriptive like libgtkhex_pc_name
Comment 22 André Klapper 2021-05-25 17:46:49 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new enhancement request ticket at
  https://gitlab.gnome.org/GNOME/ghex/-/issues/

Thank you for your understanding and your help.