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 754041 - llvm (on all systems) and clang (on some systems) sysdeps don't work
llvm (on all systems) and clang (on some systems) sysdeps don't work
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: module sets
unspecified
Other FreeBSD
: Normal normal
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
Depends on:
Blocks:
 
 
Reported: 2015-08-24 20:12 UTC by Ting-Wei Lan
Modified: 2015-12-15 16:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use a real header file to detect llvm (869 bytes, patch)
2015-08-26 13:54 UTC, Michael Catanzaro
committed Details | Review
moduleset.dtd: Allow using <cmakeargs> in <if> (963 bytes, patch)
2015-09-21 17:30 UTC, Ting-Wei Lan
committed Details | Review
moduleset.rnc: The order of <dep> and <if> is not significant (1.25 KB, patch)
2015-09-21 17:32 UTC, Ting-Wei Lan
committed Details | Review
sysdeps: Support alternative dependencies (8.80 KB, patch)
2015-09-21 17:36 UTC, Ting-Wei Lan
committed Details | Review
sysdeps-3.18: Add alternative c_include for llvm and clang (3.17 KB, patch)
2015-09-21 17:39 UTC, Ting-Wei Lan
none Details | Review
sysdeps-3.18: Add alternative c_include for llvm and clang (2.82 KB, patch)
2015-09-23 12:44 UTC, Ting-Wei Lan
none Details | Review
sysdeps-3.18/3.20: Add alternative c_include for llvm and clang (5.31 KB, patch)
2015-10-29 17:19 UTC, Ting-Wei Lan
committed Details | Review
sysdeps: Include alternative dependencies in --dump (1.86 KB, patch)
2015-10-29 17:19 UTC, Ting-Wei Lan
committed Details | Review
docs: Document <altdep> tag (2.33 KB, patch)
2015-12-15 12:16 UTC, Ting-Wei Lan
committed Details | Review

Description Ting-Wei Lan 2015-08-24 20:12:42 UTC
The recently added llvm sysdep checks for llvm-c directory, but it doesn't work because jhbuild only accepts regular file in c_include.

The same commit also added clang sysdep. It works on systems that install clang headers in /usr/include/clang-c or /usr/local/include/clang-c. However, llvm and clang packages on FreeBSD install into separate prefixes such as /usr/local/llvm35/include/clang-c and /usr/local/llvm36/include/clang-c. There is no clang package in the default repository that installs into the default prefix. gnome-builder builds fine because llvm-config35 is in PATH.
Comment 1 Michael Catanzaro 2015-08-24 22:59:06 UTC
(In reply to Ting-Wei Lan from comment #0)
> The recently added llvm sysdep checks for llvm-c directory, but it doesn't
> work because jhbuild only accepts regular file in c_include.

Hm, this works properly on Fedora (I did test it before committing this time :). Is it maybe an issue with your PackageKit backend (are you using PackageKit)? Feel free to change it to something that works for FreeBSD (I guess picking some arbitrary file in the directory should work).

Well, I take that back: 'jhbuild sysdeps --install' does properly install llvm-devel the first time it's used, but it thereafter fails to recognize that it is already installed and tries to install it again. This makes all further attempts to use 'jhbuild sysdeps --install' fail. For example, with:

Required packages:
  System installed packages which are too old:
    (none)
  No matching system package installed:
    rdflib 
    llvm 
Optional packages: (JHBuild will build the missing packages)
  System installed packages which are too old:
    libical (libical.pc, required=1.0.1, installed=1.0)
  No matching system package installed:
I: Installing dependencies on system: rdflib llvm
I: Installing:
  llvm-devel;3.5.0-9.fc22;x86_64;fedora
E: PackageKit: llvm-devel-3.5.0-9.fc22.x86_64 is already installed
I: Complete!

So that's bad.

> The same commit also added clang sysdep. It works on systems that install
> clang headers in /usr/include/clang-c or /usr/local/include/clang-c.
> However, llvm and clang packages on FreeBSD install into separate prefixes
> such as /usr/local/llvm35/include/clang-c and
> /usr/local/llvm36/include/clang-c. There is no clang package in the default
> repository that installs into the default prefix. gnome-builder builds fine
> because llvm-config35 is in PATH.

Hmmmm, this is a tricky problem. I guess there is no way to make a c_include sysdep work, then. This is exactly the problem that pkg-config solves, but clang is too "good" for pkg-config, apparently....

Originally I tried to add a path sysdep to look for llvm-config in PATH, but that did not work for me; I think because it is a symlink in Fedora, managed by alternatives.

Can you please try this and see if it works for you:

  <systemmodule id="clang">
    <branch repo="system"/>
    <systemdependencies>
      <dep type="c_include" name="clang-c/Index.h"/>
      <dep type="path" name="llvm-config"/>
    </systemdependencies>
  </systemmodule>

I'm a bit surprised, but that works mostly fine on Fedora: the only issue is that clang shows up twice in the list of sysdeps if it is not installed, but it installs fine and it's recognized as installed once it is. So if the path dep works for FreeBSD, then that could be our solution there.
Comment 2 Michael Catanzaro 2015-08-24 23:04:20 UTC
Well that's a terrible solution, since /usr/bin/llvm-config belongs to llvm-devel, not clang-devel. Hm... I'm out of ideas here.
Comment 3 Ting-Wei Lan 2015-08-25 04:32:33 UTC
(In reply to Michael Catanzaro from comment #1)
> (In reply to Ting-Wei Lan from comment #0)
> > The recently added llvm sysdep checks for llvm-c directory, but it doesn't
> > work because jhbuild only accepts regular file in c_include.
> 
> Hm, this works properly on Fedora (I did test it before committing this time
> :). Is it maybe an issue with your PackageKit backend (are you using
> PackageKit)? Feel free to change it to something that works for FreeBSD (I
> guess picking some arbitrary file in the directory should work).

llvm sysdep problem was found on Fedora. It uses os.path.isfile, so I think it only works with regular files when I filed this bug.

PackageKit on FreeBSD is very outdated and I never successfully install any package using it. Currently FreeBSD package manager doesn't support finding uninstalled package by filename, so I think it is possible that we still cannot use PackageKit even if we fix its FreeBSD ports backend or add a FreeBSD pkg backend.

There is a patch that adds a sysdep-to-package table for FreeBSD in JHBuild, but the patch is not reviewed for more than 6 months. https://bugzilla.gnome.org/show_bug.cgi?id=742291

> 
> Well, I take that back: 'jhbuild sysdeps --install' does properly install
> llvm-devel the first time it's used, but it thereafter fails to recognize
> that it is already installed and tries to install it again. This makes all
> further attempts to use 'jhbuild sysdeps --install' fail. For example, with:
> 
> Required packages:
>   System installed packages which are too old:
>     (none)
>   No matching system package installed:
>     rdflib 
>     llvm 
> Optional packages: (JHBuild will build the missing packages)
>   System installed packages which are too old:
>     libical (libical.pc, required=1.0.1, installed=1.0)
>   No matching system package installed:
> I: Installing dependencies on system: rdflib llvm
> I: Installing:
>   llvm-devel;3.5.0-9.fc22;x86_64;fedora
> E: PackageKit: llvm-devel-3.5.0-9.fc22.x86_64 is already installed
> I: Complete!
> 
> So that's bad.

'jhbuild build' and 'jhbuild sysdeps' (without --install) also don't work on Fedora. It seems the only working command is 'jhbuild sysdeps --install' ...


> 
> > The same commit also added clang sysdep. It works on systems that install
> > clang headers in /usr/include/clang-c or /usr/local/include/clang-c.
> > However, llvm and clang packages on FreeBSD install into separate prefixes
> > such as /usr/local/llvm35/include/clang-c and
> > /usr/local/llvm36/include/clang-c. There is no clang package in the default
> > repository that installs into the default prefix. gnome-builder builds fine
> > because llvm-config35 is in PATH.
> 
> Hmmmm, this is a tricky problem. I guess there is no way to make a c_include
> sysdep work, then. This is exactly the problem that pkg-config solves, but
> clang is too "good" for pkg-config, apparently....
> 
> Originally I tried to add a path sysdep to look for llvm-config in PATH, but
> that did not work for me; I think because it is a symlink in Fedora, managed
> by alternatives.
> 
> Can you please try this and see if it works for you:
> 
>   <systemmodule id="clang">
>     <branch repo="system"/>
>     <systemdependencies>
>       <dep type="c_include" name="clang-c/Index.h"/>
>       <dep type="path" name="llvm-config"/>
>     </systemdependencies>
>   </systemmodule>
> 
> I'm a bit surprised, but that works mostly fine on Fedora: the only issue is
> that clang shows up twice in the list of sysdeps if it is not installed, but
> it installs fine and it's recognized as installed once it is. So if the path
> dep works for FreeBSD, then that could be our solution there.

This also doesn't work because there is no unversioned llvm-config executable in PATH on FreeBSD. llvm-config35 is in PATH (/usr/local/bin/llvm-config35), but llvm-config (/usr/local/llvm35/bin/llvm-config) is not.
Comment 4 Michael Catanzaro 2015-08-26 13:53:02 UTC
OK, I don't know what to recommend for FreeBSD. :( Maybe using your list of sysdeps -> packages is the best approach?

For Fedora, I think the only needed change is to use a real header file to detect llvm. I picked llvm-c/Core.h.
Comment 5 Michael Catanzaro 2015-08-26 13:54:03 UTC
Created attachment 310032 [details] [review]
Use a real header file to detect llvm

Using a directory does not work reliably.
Comment 6 Michael Catanzaro 2015-08-26 13:56:25 UTC
Ah, you already committed an identical patch. :)  And git was smart enough to completely remove my commit when I ran 'git pull -r' before pushing. Nice, although I was a bit confused when my commit disappeared!
Comment 7 Ting-Wei Lan 2015-08-28 18:05:54 UTC
I think we are able to resolve this by adding a FreeBSD-specific sysdep that checks for llvm-config35. This was what I did for guile and gc:

<autotools id="guile">
  <dependencies>
    <if condition-unset="freebsd">
      <dep package="gc"/>
    </if>
    <if condition-set="freebsd">
      <dep package="gc-threaded"/>
    </if>
  </dependencies>
</autotools>

<systemmodule id="gc">
  <pkg-config>bdw-gc.pc</pkg-config>
  <branch repo="system"/>
</systemmodule>
                                                
<systemmodule id="gc-threaded">
  <pkg-config>bdw-gc-threaded.pc</pkg-config>
  <branch repo="system"/>
</systemmodule>

But this doesn't look good. I think a better solution may be adding condition set support for <systemmodule>.

I think we also need a way to allow a sysdep to meet without requiring all <dep> are installed because it is possible for users to install different versions of llvm and clang, and the name of llvm-config or c_include can be different.
Comment 8 Ting-Wei Lan 2015-09-21 17:30:46 UTC
Created attachment 311785 [details] [review]
moduleset.dtd: Allow using <cmakeargs> in <if>

This fixes validation problems caused by commit ae68cb8 and 8eb8008.

This change is not directly related to this bug report, but it is needed
to validate changes to modulesets made by following commits.
Comment 9 Ting-Wei Lan 2015-09-21 17:32:13 UTC
Created attachment 311786 [details] [review]
moduleset.rnc: The order of <dep> and <if> is not significant

This should fix all validation errors in modulesets because the order
of elements is significant in rnc by default.

This change is also not directly related, but it is also required to
validate modulesets changes.
Comment 10 Ting-Wei Lan 2015-09-21 17:36:55 UTC
Created attachment 311787 [details] [review]
sysdeps: Support alternative dependencies

Some system dependencies, such as LLVM, cannot be easily detected with
single c_include or path because different distributions can install the
same things in different places with different names.

To resolve this problem, we add a new type of tag <altdep>, which can be
used as a child node of <dep> in <systemdependencies> to specify the
list of alternative c_include and path. The syntax of <altdep> and <dep>
are the same, so we can reuse code used to process <dep>.

This commit only makes dependency checking work for 'jhbuild build' and
'jhbuild sysdeps'. Changes required to make 'jhbuild sysdeps --dump',
'jhbuild sysdeps --dump-all', 'jhbuild sysdeps --install' work are still
under testing, so they are not included in this commit.
Comment 11 Ting-Wei Lan 2015-09-21 17:39:51 UTC
Created attachment 311788 [details] [review]
sysdeps-3.18: Add alternative c_include for llvm and clang

I know it is ugly to have .. in c_include, but I don't know there are
better solutions if we cannot specify alternative prefixes.
Comment 12 Ting-Wei Lan 2015-09-23 12:44:49 UTC
Created attachment 311944 [details] [review]
sysdeps-3.18: Add alternative c_include for llvm and clang

Fix conflict caused by commit f2275e9 and use llvm-config to detect clang.
Comment 13 Michael Catanzaro 2015-09-23 13:20:43 UTC
Thanks Ting-Wei! Unfortunately I am not qualified to review these; I think Frederic or Ryan will need to take a look.
Comment 14 Ting-Wei Lan 2015-10-29 17:19:25 UTC
Created attachment 314422 [details] [review]
sysdeps-3.18/3.20: Add alternative c_include for llvm and clang
Comment 15 Ting-Wei Lan 2015-10-29 17:19:42 UTC
Created attachment 314423 [details] [review]
sysdeps: Include alternative dependencies in --dump
Comment 16 Frederic Peters 2015-12-15 08:50:41 UTC
Could you add a paragraph or two to the <systemmodule> section in jhbuild.docbook?

Then it will be good to go.
Comment 17 Ting-Wei Lan 2015-12-15 12:16:57 UTC
Created attachment 317413 [details] [review]
docs: Document <altdep> tag
Comment 18 Frederic Peters 2015-12-15 16:20:30 UTC
Fine; go ahead and push all the things.
Comment 19 Ting-Wei Lan 2015-12-15 16:29:01 UTC
Attachment 311785 [details] pushed as 71ce53b - moduleset.dtd: Allow using <cmakeargs> in <if>
Attachment 311786 [details] pushed as 482a504 - moduleset.rnc: The order of <dep> and <if> is not significant
Attachment 311787 [details] pushed as 1ebdf58 - sysdeps: Support alternative dependencies
Attachment 314422 [details] pushed as 1bd67fc - sysdeps-3.18/3.20: Add alternative c_include for llvm and clang
Attachment 314423 [details] pushed as 29e7e7e - sysdeps: Include alternative dependencies in --dump
Attachment 317413 [details] pushed as 2dd5073 - docs: Document <altdep> tag