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 780373 - JHBuild is only detected when installed in ~/.local/bin/jhbuild
JHBuild is only detected when installed in ~/.local/bin/jhbuild
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: plugins
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-21 20:37 UTC by Dor Askayo
Modified: 2017-03-24 23:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch v1 (3.17 KB, patch)
2017-03-21 23:44 UTC, Dor Askayo
none Details | Review
Patch v2 (3.79 KB, patch)
2017-03-22 16:23 UTC, Dor Askayo
none Details | Review
Patch v3 (4.71 KB, patch)
2017-03-22 23:31 UTC, Dor Askayo
committed Details | Review

Description Dor Askayo 2017-03-21 20:37:07 UTC
Builder assumes that the JHBuild's executable is ~/.local/bin/jhbuild, which can be wrong. For example, in Arch Linux the most convenient and recommended way to install JHBuild is through its AUR packages [1]. In this case, JHBuild's executable would end up in /usr/bin/jhbuild.

To solve this, Builder should honor the PATH environment variable when looking for JHBuild. The effect would be that when the JHBuild executable is accessible in a terminal, it's also accessible to Builder. Keeping ~/.local/bin/jhbuild as a fallback is useful for systems where ~/.local/bin/ is not in the PATH by default. (again, in Arch Linux)

I have a simple patch in the works. I'll upload it when I finish testing it.

[1] https://wiki.archlinux.org/index.php/JHBuild#Installation
Comment 1 Patrick Griffis (tingping) 2017-03-21 20:40:20 UTC
>Install the jhbuild AUR package, which usually provides the stable version.
>
> Alternatively, you may want to install jhbuild-git AUR for latest JHBuild version in GNOME repository.

Just want to note JHBuild has no stable releases so that is a broken concept to begin with. It is designed to be ran directly out of a git repo and the codebase directly references its git repo, so I wouldn't be surprised if that package was easily broken.
Comment 2 Christian Hergert 2017-03-21 20:40:37 UTC
(In reply to Dor Askayo from comment #0)
> Builder assumes that the JHBuild's executable is ~/.local/bin/jhbuild, which
> can be wrong. For example, in Arch Linux the most convenient and recommended
> way to install JHBuild is through its AUR packages [1]. In this case,
> JHBuild's executable would end up in /usr/bin/jhbuild.

[1] is bad advice.

You should not be using jhbuild from a package manager because you're likely to get stale modulesets or use modulesets that have not been updated in lockstep with jhbuild fixes to support those modulesets.
Comment 3 Christian Hergert 2017-03-21 20:41:07 UTC
But that said, I'm still perfectly fine with accepting patches that allow it to work.
Comment 4 Dor Askayo 2017-03-21 20:48:12 UTC
I completely agree. However, Arch Linux also has a package that follows git [1]. Also, some people may prefer to install their self-built packages in a different path. We shouldn't deprive them of that ability.

[1] https://aur.archlinux.org/packages/jhbuild-git/
Comment 5 Dor Askayo 2017-03-21 23:44:05 UTC
Created attachment 348453 [details] [review]
Patch v1
Comment 6 Christian Hergert 2017-03-22 04:11:33 UTC
Review of attachment 348453 [details] [review]:

I don't like the idea of spawning a process every time we need to spawn a process. So we should cache the first successful result. Trying to recover from the user changing their jhbuild installation out from under us is not something I'm interested in protecting against.
Comment 7 Dor Askayo 2017-03-22 16:23:05 UTC
Created attachment 348515 [details] [review]
Patch v2
Comment 8 Christian Hergert 2017-03-22 23:08:40 UTC
Review of attachment 348515 [details] [review]:

LGTM, although generally I prefer to see super() used instead of directly calling the parent __init__.
Comment 9 Dor Askayo 2017-03-22 23:31:14 UTC
Created attachment 348547 [details] [review]
Patch v3

Done :)
Comment 10 Christian Hergert 2017-03-24 22:59:48 UTC
Review of attachment 348547 [details] [review]:

LGTM
Comment 11 Christian Hergert 2017-03-24 23:00:48 UTC
Thanks!