FindGObjectIntrospection.cmake: prefix variables obtained from pkg-config with PKG_CONFIG_SYSROOT_DIR

• Component: WebKitGTK
• Status: RESOLVED INVALID

[Alexander Kanavin]

FindGObjectIntrospection.cmake: prefix variables obtained from pkg-config with PKG_CONFIG_SYSROOT_DIR

[Alexander Kanavin]

Created attachment 443789 [details]
Patch

[Michael Catanzaro]

Comment on attachment 443789 [details]
Patch

It took me a little while to convince myself that this was correct, but I suppose there's no way for cross-compilation to work otherwise, so OK.

That said, not all pkg-config variables are paths, but you're prepending this to all of them, e.g. version numbers are now going to contain filesystem paths. Also, it's generally not appropriate to prepend this variable even to most filesystem paths: it only makes sense in this particular case because you're executing those binaries during the build itself. Now, because the macro is only used immediately below, we know that's not a problem with how it is currently used, but it is still a semantic problem that we need to solve before landing. You can fix it in either of two ways: you could rename the macro to indicate it is only supposed to be used to get paths to binaries for use during compilation (this might result in a weird name, _GIR_GET_ PKGCONFIG_VAR_BUT_ONLY_IF_ITS_A_PATH_TO_A_BINARY_THAT_WE_ARE_ABOUT_TO_EXECUTE does not sound very swell), or maybe it would be better to do the append later after calling the macro so that we can leave it as a generic macro for getting any variable. Either way would work.

[Alexander Kanavin]

A change has landed that removes the code altogether, so I need to check if this patch is still relevant, and rework accordingly.

[Alexander Kanavin]

Created attachment 452238 [details]
Patch

[Alexander Kanavin]

Ping :)

[Michael Catanzaro]

Comment on attachment 452238 [details]
Patch

This looks fine to me, but Adrian mentioned something about not liking this. Adrian?

[Alexander Kanavin]

Ping!

[Adrian Perez]

(In reply to Michael Catanzaro from comment #6)
> Comment on attachment 452238 [details]
> Patch
> 
> This looks fine to me, but Adrian mentioned something about not liking this.
> Adrian?

So what's suspicious about this is that when cross-compiling, the sysroot
refers to the location of the *target platform* files (libraries, headers,
etc.) but in the case of programs to execute during the build, we cannot
(typically, at least) run them on the *build host*. Prefixing the sysroot
is the wrong thing to do in the general case, what one does is passing
-DCMAKE_PROGRAM_PATH= to tell if where to look for programs used during
compilation and that will run in the *build host*.

Now, I do know that GI is cross-compilation trouble, and at least Yocto
and Buildroot use qemu's userspace emulation to run the tools; but how to
treat the particularities of GI tooling is the choice of each particular
build system. I would rather not have a workaround for some compilation
system in the WebKit source tree—such a think should not be needed, and
proof of that is Buildroot can builds WebKitGTK with introspection enabled
without a patch like this:

  https://git.busybox.net/buildroot/tree/package/webkitgtk/webkitgtk.mk?h=2022.02.x#n68

[Adrian Perez]

(ooops, s/think/thing/ in my previous comment)

[Alexander Kanavin]

Right, this actually makes sense. I can withdraw this patch and mark it as yocto-specific in our tree to prevent further submissions.

[Adrian Perez]

(In reply to Alexander Kanavin from comment #10)
> Right, this actually makes sense. I can withdraw this patch and mark it as
> yocto-specific in our tree to prevent further submissions.

👍️

Of course if you run into any other issues, please do ask and we can try
to help out --

[Adrian Perez]

(In reply to Alexander Kanavin from comment #10)
> Right, this actually makes sense. I can withdraw this patch and mark it as
> yocto-specific in our tree to prevent further submissions.

👍️

Of course if you run into any other issues, please do ask and we can try
to help out -- we do want people to be able to package and cross build
WebKit after all :)

[Michael Catanzaro]

(In reply to Alexander Kanavin from comment #10)
> Right, this actually makes sense. I can withdraw this patch and mark it as
> yocto-specific in our tree to prevent further submissions.

And add a link to this bug too, if you don't mind, so people can find this discussion in the future.