Created attachment 369734 [details] [review]
add option to meson.build to allow overriding gio_module_dir

Review of attachment 369734 [details] [review]:

I'll probably accept this, but first please answer this question: why would you want to do this? The modules won't work at all if installed somewhere that GIO does not know to look for them. Are you setting the GIO_MODULE_DIR environment variable? How?

This would simplify the Homebrew formula for glib-networking that I maintain slightly easier as I know have to hack the meson.build file (previously we did something similar with configure).

Homebrew installs all packages into its own unique prefix, which is then symlinked into /usr/local. We cannot have the gio_module_dir set to the prefix of glib, as we don't allow formulas to install into other formulas' prefixes.

Doesn't make a huge difference since the hack I currently use is a one-liner, but this would be prettier :-)

OK. This also makes sense from the perspective that installing files outside of the install prefix, without any way to override, is not very nice.

But how do the modules actually work in Homebrew? GIO won't be able to load the module, right?

No it works just fine, because GIO is configured to look for modules in /usr/local/lib/gio/modules, not within glib's prefix.

(In reply to Tom Schoonjans from comment #5)
> No it works just fine, because GIO is configured to look for modules in
> /usr/local/lib/gio/modules, not within glib's prefix.

This is a GLib bug then, it should print the right path (/usr/local/lib/gio/modules) in its pkg-config file!

(In reply to Tom Schoonjans from comment #1)
> Created attachment 369734 [details] [review] [review]
> add option to meson.build to allow overriding gio_module_dir

Pushed with a couple corrections:

 - Fix the variable name gio -> gio_dep so the build doesn't fail by default ;)
 - Tweaked capitalization in the option description

Emmanuele is adamant that we should not be using pkg-config at all. I'll probably back this out and attach a different patch that hardcodes the module dir under the install prefix.

I'll just copypaste:

mcatanzaro	inigomartinez, ebassi: Do you remember, several months ago, we had a discussion on how to handle modules that want to install files outside the normal install prefix, using pkg-config to find installation directories? We specifically discussed what the best-practice would be for handling these situations. Either of you remembered what our consensus was?
Was it: defer to pkg-config by default, potentially installing outside the install prefix, but provide a meson option to allow overriding it?
Or was it: ignore pkg-config by default, and ensure the files get installed inside the prefix by default?

ebassi	mcatanzaro: Always use the prefix that was provided to you by your own build system, never use the variable inside a pkg-config file
mcatanzaro: If you're installing in a different prefix, modify your environment variables to look in that prefix as well
e.g. if you're installing introspection data, *always* install them under the prefix you were configured with, never in the location provided by the gobject-introspection pkg-config file variables
And if you're using a different prefix that the system one, it's up to you to modify your environment so that things work

mcatanzaro	Hmm OK
So you would say https://git.gnome.org/browse/glib-networking/tree/meson.build#n49 is wrong

ebassi	mcatanzaro: Yes, it's wrong

mcatanzaro	I just pushed https://bugzilla.gnome.org/show_bug.cgi?id=794358 which adds a meson option to allow configuring it... would you say that's unnecessary?
bugbot	Bug 794358: glib, normal, Normal, ---, gtkdev, RESOLVED FIXED, glib-networking: add option for overriding gio_module_dir to meson script

ebassi	Unless you define the variable as `{prefix}/some/place` and then you replace {prefix} with your *own* prefix

mcatanzaro	BTW we have lots and lots of packages that use pkg-config in this way that you don't like (i.e. that use pkg-config for its intended purpose ;)

ebassi	mcatanzaro: I doubt we have that many modules doing that, since that horribly breaks things like `make distcheck`

mcatanzaro	ebassi: I guess installing outside of prefix is surely fine if the user explicitly configures it to do so, right? We just don't want to do that by default.

ebassi	Unless you build *everything* into a separate prefix that you own
i.e. if you have a mixed environment with system dependencies and custom built ones, you're going to have a bad time

mcatanzaro	ebassi: Have you ever looked at the _broken directories that jhbuild creates whenever it detects a package installing outside of the install prefix? We must have 10-20 packages at least doing that.

ebassi	mcatanzaro: No, installing outside of the defined prefix is 99% of the time wrong
mcatanzaro: Then we have 10-20 packages (out of ~200) that are wrong

mcatanzaro	Someone who uses jhbuild still should be able to check by running a 'jhbuild build' to build everything.

ebassi	And need to be fixed
jhbuild does not run `make distcheck`
Say, for instance, you're building glib-networking against the system gio, instead of the one under a separate prefix; now glib-networking will try to install modules under /usr, which won't be able to do because root permissions
But, worse, with distcheck you're going to run `uninstall`, and that will also fail miserably because you cannot remove the file
The problem is that you *can* use pkg-config variables, but: the variables must use something like `${prefix}` or `${libdir}` or `${datadir}`
And then you have to provide your own prefix, libdir, or datadir when retrieving the value

mcatanzaro	Aaaah

ebassi	i.e. you cannot just expand the variables with the prefix used inside the pkg-config file
Otherwise all of this will break horribly

mcatanzaro	gio-2.0 includes giomoduledir=${libdir}/gio/modules
So we just have to pass our own libdir to pkg-config

ebassi	mcatanzaro: Yep: pkg-config --define-variable=libdir=/your/prefix/lib --variable=giomoduledir gio-2.0
mcatanzaro: The issue is that nobody does that, and thus: breakage
mcatanzaro: With meson: zdep_prefix = gio_dep.get_pkgconfig_variable('giomoduledir', define_variable: ['libdir', get_option('libdir')])
mcatanzaro: So, you either do that, or you simply install your GIO module under your own libdir/gio/modules

mcatanzaro	Okidokie ebassi. I think that was what we had agreed when we discussed it last time.

ebassi	I'm going to write a quick blog post about it

(In reply to Michael Catanzaro from comment #6)
> This is a GLib bug then, it should print the right path
> (/usr/local/lib/gio/modules) in its pkg-config file!

Emmanuele would probably disagree.

Created attachment 369742 [details] [review]
Ensure giomoduledir is always set under prefix

We have to override the libdir defined in gio-2.0.pc, or we could wind
up installing outside our install prefix.

Now, the problem here is that Homebrew really *does* want to install the giomodules outside the install prefix.

Emmanuele, any thoughts on this? Should we keep the (brand new, just pushed 20 minutes ago) meson build option to configure this directory, in addition to changing how we invoke pkg-config? Or get rid of it and tell Homebrew that trying to use separate install prefixes for each module is insane?

Created attachment 369743 [details] [review]
Revert "meson: improve GIO modules directory handling"

This reverts commit bb6f8fc39d019e975b0f9ef272b99f8ffee464ce.

Let's try a different approach...

Created attachment 369744 [details] [review]
Ensure giomoduledir is always set under prefix

We have to override the libdir defined in gio-2.0.pc, or we could wind
up installing outside our install prefix.

ebassi says:

   mcatanzaro: That looks good to me; AFAICS, homebrew installs everything into its own prefix, and then symlinks everything into /usr/local as a flat namespace; so this shouldn't change anything

Tom, can you let me know if this approach works out for you?

Yes, think it looks good. As long as the modules themselves remain within the prefix then everything is fine, and I can remove the hack we have used so far.

We will still need to disable the meson_post_install.py script by setting DESTDIR because we need gio-querymodules to work on /usr/local/lib/gio/modules (containing the symlinks of the modules), not the glib-networking installation prefix. This is not a problem that needs fixing at your end however.

Thanks!

Oh, and you may want to do something similar in glib-openssl.

(In reply to Tom Schoonjans from comment #16)
> Oh, and you may want to do something similar in glib-openssl.

CCing Nacho

Attachment 369743 [details] pushed as c9d9319 - Revert "meson: improve GIO modules directory handling"
Attachment 369744 [details] pushed as a76aaad - Ensure giomoduledir is always set under prefix

Here's the rest of our IRC discussion:

inigomartinez	ebassi, mcatanzaro: ouch! I have been following the glib-networking approach (using pkg-confing variables) in several packages
however, i always tried to use a variable that allowed to set a different directory by the user

mcatanzaro	inigomartinez: https://bugzilla.gnome.org/attachment.cgi?id=369744 <-- this is what ebassi is suggesting, just be sure to check the pkg-config file in question and define what you need to define to ensure it gets installed under our own prefix

inigomartinez	btw, also remember that meson has now support to override pkg-config variables
mcatanzaro: i was thinking on that, thx. however, not all pkg-config files use variables to set some paths

mcatanzaro	inigomartinez: Yes, in those cases, we should just ignore pkg-config.

inigomartinez	meson for example resolves the absolute path instead of using relative paths, so that won't work on a meson generated file

mcatanzaro	Oh, lovely
ebassi: ^^^^^^^^^^^^^
This will break when dependencies switch from autotools to meson :S

ebassi	Yes, that's a bug in Meson
You can still generate your own pkg-config files using configure_file() instead of the pkgconfig module
And you can still define variables like `giomoduledir=${libdir}/gio/modules` with the `variables` arguments
Meson shouldn't replace those
If it does, it's a Meson bug

mcatanzaro	I assume my glib-networking patch is probably broken if glib was built with meson though, right?

ebassi	mcatanzaro: Glib built with Meson is broken in other ways

mcatanzaro	And defining giomoduledir when calling pkg-config to get the giomoduledir kinda defeats the point of using pkgconfig at all, right?

ebassi	And it does not generate the pkgconfig files with the pkgconfig module
So don't worry about that

mcatanzaro	OK then
This meson problem is worth mentioning in your blog post

ebassi	We have many thing to fix in GLib and Meson before we can switch from Autotools

inigomartinez	ok, so or meson is fixed so the default directories (`libdir` and `includedir`) are generated as relatives to `prefix` or a custom `configure_file` generated `.pc` files is created, isn't it?

ebassi	inigomartinez: This is not a problem for bindir, libdir, or includedir — though, of course, it's better if those are relative paths
inigomartinez: The problem is that, when you define a new path, like `giomoduledir`, using the pkgconfig.generate() function, you need to do this:
`variables: [ 'giomoduledir=${libdir}/gio/modules' ]`
And Meson must not attempt to replace `${libdir}` with its own `libdir`
If it doesn't, then we're all good

inigomartinez	iirc, it generates sets those variables properly

ebassi	Because you're not replacing `prefix` when asking for the giomoduledir; you're replacing `libdir`
Now, if `libdir` was also specified in terms of `prefix`, then replacing the prefix would work and we could all be celebrating

inigomartinez	iirc, it sets those variables properly
ok
ok, i'll check some of the meson ports then

Thanks for letting me know: 
https://git.gnome.org/browse/glib-openssl/commit/?id=793e1b85b71a8832918e1511fa8d45a865cfe8d4