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 793437 - Add proper PKG_CONFIG_PATH for Debian multiarch
Add proper PKG_CONFIG_PATH for Debian multiarch
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: cerbero
git master
Other Linux
: Normal normal
: 1.13.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-02-13 23:01 UTC by Omar Akkila
Modified: 2018-02-23 00:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add PKG_CONFIG_PATH paths for Debian multiarch (1.74 KB, patch)
2018-02-13 23:02 UTC, Omar Akkila
needs-work Details | Review
Fix pkgconfig path for Debian multiarch (1.88 KB, patch)
2018-02-21 16:25 UTC, Omar Akkila
none Details | Review
Fix pkgconfig path for Debian multiarch (1.88 KB, patch)
2018-02-21 22:21 UTC, Omar Akkila
committed Details | Review

Description Omar Akkila 2018-02-13 23:01:48 UTC
Currently in `cerbero/utils/__init__.py`, it is assumed in `add_system_libs()` that one of the pkgconfig paths is always formed with a [arch]-linux-gnu triplet. In Debian, this doesn't apply to architectures such as armel (arm-linux-gnueabi) or armhf (arm-linux-gnueabihf), or to architectures where the arch name in the triplet differs such as arm64 (aarch64-linux-gnu).
Comment 1 Omar Akkila 2018-02-13 23:02:55 UTC
Created attachment 368333 [details] [review]
Add PKG_CONFIG_PATH paths for Debian multiarch
Comment 2 Olivier Crête 2018-02-20 20:06:47 UTC
Review of attachment 368333 [details] [review]:

::: cerbero/utils/__init__.py
@@ +395,3 @@
+		if Architecture.is_arm(arch) and arch != Architecture.ARM64:
+			if arch == Architecture.ARM:
+				search_paths.append(os.path.join(sysroot, 'usr/lib/arm-linux-gnueabi/pkgconfig'))

Instead of doing something debian specific like this it should be usr/lib/${host}/pkgconfig and take the host from the config.. And make sure the config has the right host, possibly by adding it to the config/linux.config file
Comment 3 Nicolas Dufresne (ndufresne) 2018-02-20 21:56:10 UTC
(In reply to Olivier Crête from comment #2)
> Review of attachment 368333 [details] [review] [review]:
> 
> ::: cerbero/utils/__init__.py
> @@ +395,3 @@
> +		if Architecture.is_arm(arch) and arch != Architecture.ARM64:
> +			if arch == Architecture.ARM:
> +				search_paths.append(os.path.join(sysroot,
> 'usr/lib/arm-linux-gnueabi/pkgconfig'))
> 
> Instead of doing something debian specific like this it should be
> usr/lib/${host}/pkgconfig and take the host from the config.. And make sure
> the config has the right host, possibly by adding it to the
> config/linux.config file

The entire point was to make it work out of the box on debian architecture.
Comment 4 Nicolas Dufresne (ndufresne) 2018-02-20 22:03:59 UTC
Review of attachment 368333 [details] [review]:

I would keep it in a way that make it works for Debian, but the code is a bit of a spaghetti. Try a construction like this instead maybe ?

  arch_path = None
  if arch == Architecture.ARM:
    arch_path = 'usr/lib/arm-linux-gnueabi/pkgconfig'
  elif arch == Architecture.ARM64:
    arch_path = 'usr/lib/aarch64-linux-gnueabi/pkgconfig'
  elif Architecture.is_arm(arch):
    arch_path = 'usr/lib/arm-linux-gnueabihf/pkgconfig'
  else:
    arch_path = 'usr/lib/%s-linux-gnu/pkgconfig' % arch
  
  search_paths.append(os.path.join(sysroot, arch_path))
Comment 5 Nicolas Dufresne (ndufresne) 2018-02-20 22:05:11 UTC
Maybe also make change arch_path into host, to reduce even more the redundant strings ?
Comment 6 Omar Akkila 2018-02-21 16:25:21 UTC
Created attachment 368720 [details] [review]
Fix pkgconfig path for Debian multiarch
Comment 7 Omar Akkila 2018-02-21 16:27:10 UTC
^ This patch worked after starting a build from scratch. Let me know if still needs more work
Comment 8 Omar Akkila 2018-02-21 22:19:55 UTC
Review of attachment 368720 [details] [review]:

Theres a typo in one of the if statements...will submit new patch with fix
Comment 9 Omar Akkila 2018-02-21 22:21:00 UTC
Created attachment 368733 [details] [review]
Fix pkgconfig path for Debian multiarch
Comment 10 Nicolas Dufresne (ndufresne) 2018-02-22 00:41:23 UTC
Review of attachment 368733 [details] [review]:

Looks good and clean now.
Comment 11 Nicolas Dufresne (ndufresne) 2018-02-22 22:16:10 UTC
Note, I had to remove some leading white spaces, careful next time ;-P

Thanks for the patch !

Attachment 368733 [details] pushed as 4fd3fe6 - Fix pkgconfig path for Debian multiarch
Comment 12 Omar Akkila 2018-02-23 00:22:48 UTC
Thanks!

Sorry about that...just configured my vim to make sure this isn't a problem in the future :p