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

gnome.compile_resources builds wrong paths since 0.44 #2860

Closed
aclemons opened this issue Jan 2, 2018 · 5 comments
Closed

gnome.compile_resources builds wrong paths since 0.44 #2860

aclemons opened this issue Jan 2, 2018 · 5 comments

Comments

@aclemons
Copy link

aclemons commented Jan 2, 2018

Since upgrading to meson 0.44, the paths generated from a call to gnome.compile_resources include the subdirectory twice, breaking the build.

This is on Slackware Linux 14.2 x86_64 using meson 0.44. Example projects breaking are appstream-glib and fwupd.

appstream-glib includes this in its meson.build:

asresources = gnome.compile_resources(
  'as-resources', 'appstream-glib.gresource.xml',
  c_name : 'as'
)

That results in this in the build.ninja file:

build libappstream-glib/as-resources.c: CUSTOM_COMMAND ../libappstream-glib/appstream-glib.gresource.xml | ../libappstream-glib/libappstream-glib/as-stock-icons.txt ../libappstream-glib/libappstream-glib/as-license-ids.txt ../libappstream-glib/libappstream-glib/as-category-ids.txt ../libappstream-glib/libappstream-glib/as-environment-ids.txt
 COMMAND = glib-compile-resources ../libappstream-glib/appstream-glib.gresource.xml --sourcedir ../libappstream-glib --c-name as --internal --generate --target libappstream-glib/as-resources.c
 description = Generating$ as-resources_c$ with$ a$ custom$ command.

note the double libappstream-glib directory.

The same happens with fwupd:

resources_src = gnome.compile_resources(
  'fwupd-resources',
  'fwupd.gresource.xml',
  source_dir : '.',
  c_name : 'fu'
)
build src/fwupd-resources.c: CUSTOM_COMMAND ../src/fwupd.gresource.xml | ../src/src/org.freedesktop.fwupd.xml
 COMMAND = glib-compile-resources ../src/fwupd.gresource.xml --sourcedir ../src/. --sourcedir ../src --c-name fu --internal --generate --target src/fwupd-resources.c
 description = Generating$ fwupd-resources_c$ with$ a$ custom$ command.

note the duplicated src directory.

The builds were fine with meson 0.43.

Reverting the commit 5219333 unbreaks the build, but that commit was added to fix #2633.

I initially reported this as an appstream-glib issue here hughsie/appstream-glib#216, but it seems to be a meson regression.

Slackbuilds for meson, appstream-glib, and fwupd are here:
https://slackbuilds.org/slackbuilds/14.2/libraries/appstream-glib/appstream-glib.SlackBuild
https://slackbuilds.org/repository/14.2/libraries/appstream-glib/
https://slackbuilds.org/slackbuilds/14.2/development/meson/meson.SlackBuild
https://slackbuilds.org/repository/14.2/development/meson/
http://slackbuilds.org/slackbuilds/14.2/system/fwupd/fwupd.SlackBuild
http://slackbuilds.org/repository/14.2/system/fwupd/

The appstream-glib build includes a work-around for now to remove the duplicated paths from the generated ninja files.

@hughsie
Copy link
Contributor

hughsie commented Jan 2, 2018

FYI: I'm getting quite a few reports about the fwupd build being "broken" so any help welcome. I'm going to suggest that Fedora reverts to an older meson version or to backport any fix if it comes quickly.

@jibsen
Copy link
Contributor

jibsen commented Jan 2, 2018

From a cursory look, _get_gresource_dependencies() runs glib-compile-resources from the source dir, which means it returns a list of files with the subdir prepended. These are returned to compile_resources(), which uses them directly as depend_files for the custom command, if glib-compile-resources is older than 2.51.1.

I don't know much about the gnome module, but it seems to me it depends on the old (wrong) behavior of custom targets not adding the subdir to str dependencies.

@nirbheek
Copy link
Member

nirbheek commented Jan 2, 2018

I'm going to suggest that Fedora reverts to an older meson version or to backport any fix if it comes quickly.

Once this and other regressions are fixed, we will release 0.44.1.

This regression was probably caused by me, so I'll take a look at it.

@nirbheek nirbheek self-assigned this Jan 2, 2018
@jibsen
Copy link
Contributor

jibsen commented Jan 5, 2018

I know you're handling this @nirbheek, so I am just thinking out loud here. The simple fix would be to strip the subdir from the str dependencies in the gnome module, so the backend can add them again. Perhaps something like:

diff --git a/mesonbuild/modules/gnome.py b/mesonbuild/modules/gnome.py
index ad99c14d..a8bc90d5 100644
--- a/mesonbuild/modules/gnome.py
+++ b/mesonbuild/modules/gnome.py
@@ -19,6 +19,7 @@ from .. import build
 import os
 import copy
 import subprocess
+from pathlib import PurePath
 from . import ModuleReturnValue
 from ..mesonlib import MesonException, OrderedSet, Popen_safe, extract_as_list  from ..dependencies import Dependency, PkgConfigDependency, InternalDependency @@ -244,6 +245,15 @@ class GnomeModule(ExtensionModule):
         def exists_in_srcdir(f):
             return os.path.exists(os.path.join(state.environment.get_source_dir(), f))

+        def strip_subdir(f):
+            if os.path.isabs(f):
+                return f
+            pure_f = str(PurePath(f))
+            prefix = os.path.commonpath([state.subdir, pure_f])
+            if pure_f.startswith(prefix):
+                return pure_f[len(prefix) + 1:]
+            return f
+
         depends = []
         subdirs = []
         for resfile in dep_files[:]:
@@ -282,6 +292,7 @@ class GnomeModule(ExtensionModule):
                         'generated file, pass the target that generates it to '
                         'gnome.compile_resources() using the "dependencies" '
                         'keyword argument.' % (resfile, input_file))
+                dep_files[dep_files.index(resfile)] = strip_subdir(resfile)
         return dep_files, depends, subdirs

     def _get_link_args(self, state, lib, depends=None, include_rpath=False,

The question is if this is actually the right level to fix it at. Why are subdirs added by the backend?

Also, the comments in the gnome module talk about some magic regarding generated files, I don't know if this would affect that.

@wberrier
Copy link
Contributor

I'm also seeing this same issue with qt5 resource generation... (and reverting commit 5219333 also fixes it).

Should I open up a new bug report for that? I can also cook up a patch once the recommended solution to this bug is decided.

nirbheek added a commit that referenced this issue Feb 19, 2018
Also add a unit test that will test all codepaths for old Glib tools
versions.

Closes #2860
nirbheek added a commit that referenced this issue Feb 19, 2018
Also add a unit test that will test all codepaths for old Glib tools
versions.

Closes #2860
jpakkane pushed a commit that referenced this issue Feb 19, 2018
Also add a unit test that will test all codepaths for old Glib tools
versions.

Closes #2860
nirbheek added a commit that referenced this issue Feb 19, 2018
Also add a unit test that will test all codepaths for old Glib tools
versions.

Closes #2860
jpakkane pushed a commit that referenced this issue Feb 20, 2018
Also add a unit test that will test all codepaths for old Glib tools
versions.

Closes #2860
nirbheek added a commit that referenced this issue Feb 20, 2018
Also add a unit test that will test all codepaths for old Glib tools
versions.

Closes #2860
@nirbheek nirbheek removed this from the 0.44.1 milestone Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants