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 564373 - packagekit - how to best exploit it in jhbuild
packagekit - how to best exploit it in jhbuild
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
: 652686 (view as bug list)
Depends on: 654582
Blocks:
 
 
Reported: 2008-12-13 12:39 UTC by John Carr
Modified: 2011-07-21 16:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
install system dependencies (22.55 KB, patch)
2011-06-17 19:37 UTC, Colin Walters
none Details | Review
systeminstall: New utility (4.35 KB, patch)
2011-06-23 21:05 UTC, Colin Walters
none Details | Review
3.2: Add pkg-config identifiers for GTK+-2 stack (2.64 KB, patch)
2011-06-23 21:06 UTC, Colin Walters
none Details | Review
Parse 'pkg-config' attribute on modules (9.29 KB, patch)
2011-06-23 21:06 UTC, Colin Walters
none Details | Review
sysdeps: New command (4.26 KB, patch)
2011-06-23 21:06 UTC, Colin Walters
none Details | Review
systeminstall: New utility (4.35 KB, patch)
2011-06-24 16:58 UTC, Colin Walters
none Details | Review
3.2: Add pkg-config identifiers for a lot of tarball modules (5.72 KB, patch)
2011-06-24 16:59 UTC, Colin Walters
none Details | Review
Parse 'pkg-config' attribute on modules (9.29 KB, patch)
2011-06-24 16:59 UTC, Colin Walters
none Details | Review
sysdeps: New command (4.37 KB, patch)
2011-06-24 16:59 UTC, Colin Walters
none Details | Review
initial experience with jhbuild in a VM (51.86 KB, text/plain)
2011-06-24 21:16 UTC, Colin Walters
  Details
Add new partial_build key (5.36 KB, patch)
2011-06-24 21:18 UTC, Colin Walters
needs-work Details | Review
3.2: Add pkg-config to a few more tarball modules (2.32 KB, patch)
2011-06-24 21:18 UTC, Colin Walters
none Details | Review
3.2 core deps: Add a lot more pkg-config identifiers (11.36 KB, patch)
2011-06-24 21:18 UTC, Colin Walters
none Details | Review
3.2: Add pkg-config identifiers for a lot of tarball modules (18.09 KB, patch)
2011-06-30 14:40 UTC, Colin Walters
reviewed Details | Review
Parse 'pkg-config' attribute on modules (9.35 KB, patch)
2011-06-30 14:40 UTC, Colin Walters
reviewed Details | Review
Add new partial_build key, "sysdeps" command (17.33 KB, patch)
2011-06-30 14:40 UTC, Colin Walters
needs-work Details | Review
Parse <pkg-config> node (3.36 KB, patch)
2011-07-15 18:10 UTC, Colin Walters
committed Details | Review
3.2: Add pkg-config identifiers for a lot of tarball modules (15.47 KB, patch)
2011-07-15 18:10 UTC, Colin Walters
committed Details | Review
Add new partial_build key, "sysdeps" command (18.79 KB, patch)
2011-07-15 18:11 UTC, Colin Walters
committed Details | Review
config.py: If partial_build is set, explicitly include system paths (3.53 KB, patch)
2011-07-15 18:11 UTC, Colin Walters
committed Details | Review

Description John Carr 2008-12-13 12:39:38 UTC
It would be *awesome* if a new developer could install jhbuild to start hacking on something and it use packagekit to install most build deps.

To start discussion, i've knocked up a module that simply works like this:

<packagekit id="foo" package="bar" />

The core idea is that we provide a moduleset for all the external dependencies, and the user has the option of including that moduleset. IIRC, the last moduleset to be loaded "wins" so we could replace all external dependencies with packages in one swoop.

Problem 1:
Package names are different. One solution is to have per distro modulesets, but this sucks. Yay, I have cross-distro pkgmgmt.. oh wait.. Option 2 is to support multiple targets in one definition:

<packagekit id="foo">
  <install type="name" param="foo-dev" />
  <install type="file" param="libfoo.pc" />
</packagekit>

Which would try and install foo-dev, and if it cant try and install a package that provides libfoo.pc.

Problem 2:
Generating these modulesets :-\ By hand i think is the only way... Ugly hacks include... For deb, we could pull the source archive and index pkgname <--> upstream (i'm thinking looking at orig.tar.gz or debian/watch here). For rpm, inspecting all the source files currently hold and adding <install type="file" name ="blah.pc" /> entries.

Problem 3:
Differing versions. Eww. What if my distro has a version thats too old. Want if i want to get more than just external deps from pkgkit. I don't need latest gtk for example. This setup starts to completely fall apart. We need to make the dependency resolver aware of versions.. The more i think about that, the more i think this whole approach is fail...

I throw this to the floor for ideas on how to proceed.
Comment 1 Frederic Peters 2008-12-18 21:12:48 UTC
I believe there has already been an enhancement request for binary packages that fell on problem 3.
Comment 2 John Carr 2009-05-27 10:08:50 UTC
Simpler option:

john@localhost: jhbuild build-deps tomboy f-spot

This command is entirely optional.

It will: Walk dependencies. For each dependency: Can we satisfy this with a system installed package? Queue it up to be installed (if it isnt already). If not, walk its dependencies and do the same.

If jhbuild is trying to make us use a newer version than needed i think thats a seperate issue.

john@localhost: jhbuild build tomboy f-spot

At dependency graph building time: For each dependency, is this already satisfied by something installed on the system? Implicitly add a skip for it.


We could use the same function for both commands (get_system_satisyfiable()). In case (A) it would be fed to the list of packages to install and in case (B) it would be an implicit 'skip'...
Comment 3 Sam Thursfield 2009-09-14 22:24:07 UTC
Here's my current take on this feature. I see jhbuild's main purpose as building a system with complex dependencies, where one or more modules needs to be of a newer version that the system provides, or be patched/a different branch. It always intalls into a non-system prefix, ie. not /usr.

With that in mind, it seems to me jhbuild should work in this manner (I realise this is far up in the blue sky):
  * jhbuildrc specifies the version of each module that is to be required as
    well as its name. Version can be 'HEAD'.
  * jhbuild, for each module:
    if the module needs building from source (ie. it has patches in the
       moduleset, or the required version is 'HEAD'):
      build module from source
    else:
      if the version requirements are satisfied by the installed version of the
         module:
        if all of the deps are also satisfield by system packages:
          do nothing.
        else:
          install the system package into the jhbuild prefix, so it will
          load the jhbuilt versions of its dependencies.
      else if the version requirements are satisfield by an uninstalled
              binary version of the package available in the distro's repo:
        if all of the deps are also satisfield by system packages:
          install the package into the system if user has given the correct
          authority (or set an option in jhbuildrc to do so, perhaps):
            otherwise install into jhbuild prefix.
        else:
          install the system package into the jhbuild prefix.
      else:
        build module from source.

Since we would then have had to implement dependency versioning, modulesets could get cleverer: instead of the current (slightly primitive IMHO :) system of making versioned modulesets, each module could be defined once ... but this is a different problem and of less interest to me :)

Anyway, I have thought up a few use cases and the above algorithm seems to cover them all while minimising bandwidth and compiling time, and without messing up builds/versions of system packages. I'm not sure how easy it is to install system packages into non-system prefixes right now, but I doubt that is rocket science to implement. There are certainly other big infrastructure problems too, but in the worst case of this code all that should really happen is jhbuild compiles more stuff from source than it needs to ... which it does anyway

I love feedback.
Comment 4 Olav Vitters 2011-05-24 08:32:09 UTC
Discussed with Colin Walters.

We should get the package names aligned and focus on just a few distributions. This to start with something small but big impact.

On Mandriva, whenever there is a pkgconfig file, the rpm will have 'pkgconfig(lcms2)' in the Providers header. We should check/get other distributions to do the same.

Then jhbuild can integrate with packagekit and check/install these.

I'm thinking of something like:
  <autotools id="colord" autogenargs="--disable-examples">
    <branch repo="colord.gitorious.org" checkoutdir="colord" module="master"/>
    <dependencies>
      <dep package="glib"/>
      <dep package="dbus"/>
[..]
      <dep pkgconfig="lcms2" />
    </dependencies>
    <suggests>
      <dep package="polkit"/>
      <dep pkgconfig="FOOBAR" />
    </suggests>
  </autotools>


So limit it to:
* pkgconfig
* packagekit
* distributions which have pkgconfig($FILENAME) in Provides: (+ dpkg equivalent)
Comment 5 Olav Vitters 2011-05-24 08:34:12 UTC
PS:

I don't care about version requirements and so on; that's for later or for autogen/configure to complain about.
Comment 6 Frederic Peters 2011-05-24 09:18:13 UTC
For package names, there is an effort started at the appinstaller meeting to match package names across distributions, <http://enricozini.org/2011/debian/distromatch/>, it should probably be reused here.

I am not fond of <dep pkgconfig="lcms2" />, I would prefer to have lcms2 as a real <autotools> module, that would be buildable with jhbuild, but at the same time its <branch/> tag would mention a package name; this would also get it next to a version number, that will have to be considered later on anyway.

(and someday we could have a <branch/> tag that references a package only, without the tarball part).
Comment 7 Olav Vitters 2011-05-24 09:52:56 UTC
The XML seems better.

This should be linked to the new external dependencies though. Tracking dependencies of dependencies (lcms2 is required by colord) will become a maintenance problem. It would be aiming for a perfect solution, while currently nothing is in place, so 'good enough' solution would have far more impact immediately.

I don't think we should wait for a perfect solution. Package names differ at this time between Fedora, Mandriva and openSuse. The pkgconfig($FOO) *works* for Mandriva, openSuse and Fedora. I'm pretty sure Ubuntu/Debian will have something similar, just terrible at figuring out what.. so I'll have to check.
Comment 8 Colin Walters 2011-05-24 12:12:04 UTC
(In reply to comment #7)

> 
> I don't think we should wait for a perfect solution. Package names differ at
> this time between Fedora, Mandriva and openSuse. The pkgconfig($FOO) *works*
> for Mandriva, openSuse and Fedora. I'm pretty sure Ubuntu/Debian will have
> something similar, just terrible at figuring out what.. so I'll have to check.

They don't client side, but we could query their web service:

http://packages.debian.org/search?searchon=contents&keywords=glib-2.0.pc&mode=path&suite=stable&arch=any
Comment 9 Olav Vitters 2011-05-24 12:26:27 UTC
Richard,

Can you advice if a provides like 'pkgconfig(lcms2)' which exists in Fedora,Mandriva,Opensuse is also somehow accessible in:
1. Debian + Ubuntu
2. PackageKit itself :)

want to avoid querying on filename (slow in Mandriva) or relying on packagenames (those differ)
Comment 10 Richard Hughes 2011-05-24 14:58:33 UTC
(In reply to comment #9)
> Richard,
> 
> Can you advice if a provides like 'pkgconfig(lcms2)' which exists in
> Fedora,Mandriva,Opensuse is also somehow accessible in:
> 1. Debian + Ubuntu

I don't think so, the PackageKit and Ubuntu story gets stranger every day. For Debian, I think an explicit provide could be added, although I thinks it's harder to automatically add like we do in rpm based distros.

> 2. PackageKit itself :)

Sure, you can use pkcon if you want to use what-provides and that kind of thing.

Richard.
Comment 11 Frederic Peters 2011-06-17 06:54:07 UTC
*** Bug 652686 has been marked as a duplicate of this bug. ***
Comment 12 Colin Walters 2011-06-17 19:37:36 UTC
Created attachment 190151 [details] [review]
install system dependencies
Comment 13 Colin Walters 2011-06-17 19:43:17 UTC
So um...packagekit.   We know it won't work in current Debian/Ubuntu releases; we have to query the web service as I said in comment #8.

Could we use packagekit as an abstraction layer over the different RPM-based wget wrappers?  Maybe, but at least it doesn't appear to work on yum for me right now:

src/jhbuild [root=/src/build/jhbuild] [git master]
$ pkcon what-provides 'pkgconfig(atk)'
Getting provides              [=========================]         
Starting                      [=========================]         
Querying                      [=========================]         
src/jhbuild [root=/src/build/jhbuild] [git master]

versus:

[root@lenny ~]# repoquery --whatprovides 'pkgconfig(atk)'
atk-devel-0:2.0.0-1.fc15.x86_64
atk-devel-0:2.0.0-1.fc15.i686
[root@lenny]#
Comment 14 Richard Hughes 2011-06-21 11:51:09 UTC
Seems to work for me:

[hughsie@hughsie-t510 ~]$ pkcon what-provides 'pkgconfig(atk)'
Getting provides              [=========================]         
Starting                      [=========================]         
Querying                      [=========================]         
Installed   	atk-devel-2.0.0-1.fc15.x86_64           	Development files for the ATK accessibility toolkit
Available   	atk-devel-2.0.0-1.fc15.i686             	Development files for the ATK accessibility toolkit

-- are you sure you're using the _system_ packagekitd instance when you're in jhbuild?
Comment 15 Colin Walters 2011-06-21 13:05:52 UTC
(In reply to comment #14)

> -- are you sure you're using the _system_ packagekitd instance when you're in
> jhbuild?

Yeah, that was the problem.  But then the next issue is that there doesn't appear to be a way to install via provides, so we'd have to get the provides (in this case both architectures), and reimplement the yum "pick my current arch" logic?
Comment 16 Richard Hughes 2011-06-21 14:34:11 UTC
(In reply to comment #15)
> Yeah, that was the problem.  But then the next issue is that there doesn't
> appear to be a way to install via provides, so we'd have to get the provides
> (in this case both architectures), and reimplement the yum "pick my current
> arch" logic?

What you need to do is:

pkcon what-provides 'pkgconfig(atk)' --filter="arch;newest"

If you only want non-installed files, do:

pkcon what-provides 'pkgconfig(atk)' --filter="arch;newest;~installed"

And then it's pretty easy to write a script in python / C / whatever to actually install the package-id it gives you.

Grab me on irc if you want a hand with the API.

Richard.
Comment 17 Colin Walters 2011-06-21 14:44:57 UTC
Ok, there are 3 separate things to do here:

1) Add the .pc identifiers to the moduleset.  My strawman proposal just looked like:
 
-  <autotools id="pango">
+  <autotools id="pango" pkgid="pango.pc">

If someone has a better idea or cares - say it *now*.  This is a lot of tedious work to add, and I really don't want to do it twice.

Frederic?  Craig?  

2) Add a "jhbuild sysdeps" type command.  The semantics/naming are open for discussion too.  At present it tries to install all system package dependencies for the named module.  

3a) Backend sysdeps with pkcon for RPM/other variants of tar
3b) Backend sysdeps with HTML scraping for .deb


I'll break up the above patch along those lines.
Comment 18 Frederic Peters 2011-06-21 15:15:13 UTC
1) is somehow addressed in an earlier comment,

> I am not fond of <dep pkgconfig="lcms2" />, I would prefer to have lcms2 as a
> real <autotools> module, that would be buildable with jhbuild, but at the same
> time its <branch/> tag would mention a package name; this would also get it
> next to a version number, that will have to be considered later on anyway.

2) I would have opted for a global prefer_sysdeps flag, with the package installed at the same time as the "jhbuild build" command. Why do you favour a new command?

3) I'd favour PackageKit only

3b) Richard, what do you think of a webservice providing support for pkgconfig() to be used by the apt backend? It could be a really simple REST API, http://api.gnome.org/pkgconfig/deb/wheezy/pango.pc would return (text/plain, text/json, text/xml, whatever) with libpango1.0-dev. (I'd volunteer for writing this)
Comment 19 Colin Walters 2011-06-21 15:47:49 UTC
(In reply to comment #18)
> 1) is somehow addressed in an earlier comment,
> 
> > I am not fond of <dep pkgconfig="lcms2" />, I would prefer to have lcms2 as a
> > real <autotools> module, that would be buildable with jhbuild, but at the same
> > time its <branch/> tag would mention a package name; this would also get it
> > next to a version number, that will have to be considered later on anyway.

Can you give an example?

> 2) I would have opted for a global prefer_sysdeps flag, with the package
> installed at the same time as the "jhbuild build" command. Why do you favour a
> new command?

That's a very different approach.  On the plus side, there's one less command to learn, and it *would* be more ideal if we had a "transparent mixing" world that someone mentioned above where basically we pull from the distro if we can, otherwise git.

However it has some downsides.  The main one I see is that up till now, jhbuild will have been noninteractive, but people will have at some point randomly in the middle of their build, a PolicyKit auth dialog appearing.  Then they type their password...more random amount of time passes, then PK dialog, repeat (a lot for a nontrivial build).

Maybe we could make it so build as a first step, pulled everything it could in from the system, then started on git?
Comment 20 Richard Hughes 2011-06-21 16:26:27 UTC
(In reply to comment #18)
> 3b) Richard, what do you think of a webservice providing support for
> pkgconfig() to be used by the apt backend? It could be a really simple REST
> API, http://api.gnome.org/pkgconfig/deb/wheezy/pango.pc would return
> (text/plain, text/json, text/xml, whatever) with libpango1.0-dev. (I'd
> volunteer for writing this)

I think it would be waaay less work just to get the required virtual provides in the metadata for Debian. Otherwise you're going to have to deal with repos like ppas and backports making the web-view of the repo different to the client-view.
Comment 21 Colin Walters 2011-06-21 20:41:08 UTC
(In reply to comment #18)
> 1) is somehow addressed in an earlier comment,
> 
> > I am not fond of <dep pkgconfig="lcms2" />, I would prefer to have lcms2 as a
> > real <autotools> module, that would be buildable with jhbuild, but at the same
> > time its <branch/> tag would mention a package name; this would also get it
> > next to a version number, that will have to be considered later on anyway.

To clarify, if you look at my patch it has both.  In other words, <dep pkgid="lcms2.pc"> doesn't mean "install from system".  In my patch jhbuild can look up what "provides" lcms2.pc from the moduleset.

In this model most <dep> changes to <dep pkgid> with no loss of generality, except for things that don't have .pc files.
Comment 22 Frederic Peters 2011-06-22 06:46:28 UTC
> Can you give an example?

Something like this (note the new pkg-config attribute):

  <autotools id="cairo" autogenargs="--enable-xlib">
    <branch module="releases/cairo-1.10.0.tar.gz" version="1.10.0"
            repo="cairo.org" pkg-config="cairo.pc"
            hash="sha256:0f2ce4cc4615594088d74eb8b5360bad7c3cc3c3da9b61af9bfd979ed1ed94b2"
            md5sum="70a2ece66cf473d976e2db0f75bf199e" size="24022822">
      <patch file="cairo-1.10.0-TrailingComma.patch" strip="1"/>
    </branch>
    …

I don't believe it's necessary to have pkgid acting as a secondary id.

> Maybe we could make it so build as a first step, pulled everything it could in
> from the system, then started on git?

Indeed, once the modules to build are known, those that can be pulled from packages can be extracted first.
Comment 23 Colin Walters 2011-06-22 20:47:54 UTC
(In reply to comment #22)
> > Can you give an example?
> 
> Something like this (note the new pkg-config attribute):
> 
>   <autotools id="cairo" autogenargs="--enable-xlib">
>     <branch module="releases/cairo-1.10.0.tar.gz" version="1.10.0"
>             repo="cairo.org" pkg-config="cairo.pc"
>            
> hash="sha256:0f2ce4cc4615594088d74eb8b5360bad7c3cc3c3da9b61af9bfd979ed1ed94b2"
>             md5sum="70a2ece66cf473d976e2db0f75bf199e" size="24022822">
>       <patch file="cairo-1.10.0-TrailingComma.patch" strip="1"/>
>     </branch>
>     …
> 
> I don't believe it's necessary to have pkgid acting as a secondary id.

I'm confused; how is it not a secondary id?  Or you're saying just because most modules don't have a <branch/>, we wouldn't need to add many pkg-config?

The problem is then that it ties sysdeps to things we have <branch/> for, so if we want to make those work, we'll have to end up adding more <branch/> es.

Concrete example: I'd like to not have to build libnl, lcms2, colord, gstreamer, gst-plugins-base, gst-plugins-good, libxml, libxslt nss, nspr, fontconfig, cairo, pango, gtk+-2 for people who just want to hack on gnome-shell.

Actually at that point we're talking about 50% of gnome-suites-core-deps-3.2.modules; the jump to just adding pkg-config to everything isn't much larger.

If you still think it should go on branch though I can try it.
Comment 24 Colin Walters 2011-06-23 15:39:49 UTC
(In reply to comment #16)
> 
> pkcon what-provides 'pkgconfig(atk)' --filter="arch;newest;~installed"
> 
> And then it's pretty easy to write a script in python / C / whatever to
> actually install the package-id it gives you.

I can't figure it out:


gjs/_build
$ pkcon -p what-provides 'pkgconfig(atk)' --filter="arch;newest"
Transaction:	Getting provides
Status: 	Starting
Status: 	Querying
Package:	atk-devel-2.0.0-1.fc15.x86_64
Results:
Installed    atk-devel-2.0.0-1.fc15.x86_64
gjs/_build
$ pkcon  -p install 'atk-devel-2.0.0-1.fc15.x86_64'
Command failed: This tool could not find any available package: could not find atk-devel-2.0.0-1.fc15.x86_64
gjs/_build
$
Comment 25 Colin Walters 2011-06-23 21:05:55 UTC
Created attachment 190535 [details] [review]
systeminstall: New utility

This takes pkg-config identifiers and uses a system-specific method
to install them.
Comment 26 Colin Walters 2011-06-23 21:06:05 UTC
Created attachment 190536 [details] [review]
3.2: Add pkg-config identifiers for GTK+-2 stack
Comment 27 Colin Walters 2011-06-23 21:06:13 UTC
Created attachment 190537 [details] [review]
Parse 'pkg-config' attribute on modules

I still called the variable "pkgid" internally because it's shorter
and descriptive enough.
Comment 28 Colin Walters 2011-06-23 21:06:32 UTC
Created attachment 190538 [details] [review]
sysdeps: New command

This command parses the moduleset and finds all .pc files that
are referenced by the given moduleset, and installs them.

A key use case here is partial jhbuilds; for example, say you know
you can avoid building gtk+-2.  Simply:

jhbuild sysdeps gtk+-2
Comment 29 Colin Walters 2011-06-23 21:12:49 UTC
(In reply to comment #23)

> If you still think it should go on branch though I can try it.

So I looked at adding pkg-config to <branch/>, but it doesn't make sense to me architecturally.  branch as all about revision control stuff, whereas the pkg-config identifier is very similar to the 'id' attribute on <module> - it *is* an alternative id.

*However* you were right there was really no point in going through and changing <dep id="foo"> to <dep pkgid="foo.pc">.  We can just as easily look up one as the other.  So I moved all of the pkg-config lookup logic into sysdeps.py.  This definitely ends up cleaner.

I kept the sysdeps command for now (but this isn't final); just doing this in build would take more thought and work.
Comment 30 John Carr 2011-06-23 21:47:32 UTC
Hey guys. Nice to see this ticket come back to life. I haven't looked at this since 2008, but i did write a prototype (the 2 year old external-deps branch on git.gnome.org) so i thought i'd wade in. Unfortunately at some point I merged master into my branch so its hard to see the changes I made :(

If I throw away the multiple backends and ubuntu/debian specific stuff in that branch it looks like there are 2 things i looked into that might still have some relevance: versions, and integration with the build command. 

Obviously ancient-branch disclaimer applies.

To integrate with the main build command I was going to extend the get_module_list function to take a callback that would decided to skip modules based on whether my apt code detected a package.

http://git.gnome.org/browse/jhbuild/commit/?h=external-deps&id=29cb0f81d8fd6761f027a385636319260a41283e

I seem to remember discussing this approach on IRC, but can't remember the outcome.

I also seem to have done a lot of work around version numbers so it could ignore the system dependency if it was too old. I don't know how much jhbuild has changed since december 2008, but it seems i was adding minimum and recommended info to the <dep/> nodes. It also looks like i was initially targetting only tarball modules, presumably because they already had version numbers.
Comment 31 Frederic Peters 2011-06-24 06:03:13 UTC
> So I looked at adding pkg-config to <branch/>, but it doesn't make sense to me
> architecturally.  branch as all about revision control stuff, whereas the
> pkg-config identifier is very similar to the 'id' attribute on <module> - it
> *is* an alternative id.

The reasoning is that the branch is about the origin of a package. For dependencies we consider stable enough, the origin is a tarball. Those stable modules could as well come from distribution packages.

I do not want to allow packages to be installed for modules defined as coming from git, as this will mean a new serie of answers, "jhbuild pulled a package for that module, but it's too old, you now have to type jhbuild build --iwantgitreally". A recent example would be the G_CONST_RETURN changes, they affected GTK+ 2.
Comment 32 Dirk Wallenstein 2011-06-24 06:09:35 UTC
(In reply to comment #28)
> Created an attachment (id=190538) [details] [review]
> sysdeps: New command
> 
> This command parses the moduleset and finds all .pc files that
> are referenced by the given moduleset, and installs them.
> 
> A key use case here is partial jhbuilds; for example, say you know
> you can avoid building gtk+-2.  Simply:
> 
> jhbuild sysdeps gtk+-2

What about a new configuration 'modules_pick' or similar to slice a moduleset. Everything that's missing could be provided by sysdeps.
Comment 33 Colin Walters 2011-06-24 13:49:16 UTC
(In reply to comment #31)
>
> I do not want to allow packages to be installed for modules defined as coming
> from git, as this will mean a new serie of answers, "jhbuild pulled a package
> for that module, but it's too old, you now have to type jhbuild build
> --iwantgitreally". A recent example would be the G_CONST_RETURN changes, they
> affected GTK+ 2.

To flesh this out a bit, this is the case where you're pulling git glib, but using system gtk+-2?  Yes, I agree that kind of thing can be an issue, but note the G_CONST_RETURN only impacts things using -DG_DISABLE_DEPRECATED.  We should be making it easy to turn that kind of stuff off.

Note that in my current patch series, "jhbuild build" (assuming you are defaulting to a moduleset) will still build everything.  So there's no --iwantgitreally.  The "sysdeps" is like I said similar to "bootstrap" in that it's something you would typically run just once.  And honeslty I think the explicit separation is good; anything else is just too magical.

I know the dream is something like:

1) Install $latest version of $distro without devel tools, headers etc.
2) Install jhbuild from git
3) Run jhbuild build gnome-shell
4) Hack
5) Goto 3

And jhbuild is smart enough to figure out exactly what you need so you can get started quickly.  This patch is only a step towards that dream, and honestly - debates about exactly what "build" does aren't the main problem.

The bigger problems are random landmines like:

* Building GNOME 3.2 nss on Fedora 15 breaks ssh, which in turn breaks jhbuild
* jhbuild has no concept of postinsts; we need to be running e.g. gdk-pixbuf-query-loaders  (I just had to help someone on IRC with this yesterday).

My goal with this bug is simply to make things like install the X.org devel headers work more seamlessly, along with being able to skip e.g. nss for the reasons above.   Given that goal, I think the separate command is going to be our best bet.

In other words, the flow would be:

1) Install $latest version of $distro without devel tools, headers etc.
2) Install jhbuild from git
3) Run jhbuild sysdeps gnome-shell
4) Run jhbuild buildone gnome-shell
5) hack
6) goto 4

If step 4) fails, the hacker can use e.g. "jhbuild buildone glib" to pull in a new glib.

Does this sound ugly and worse?  Yes, a bit - but having literally just last week installed a fresh laptop and set up jhbuild, I *know* it will be better, because I've already avoided nss/libxslt, the X development headers etc.

Basically sysdeps would be a new tool in the toolbox, and we'd learn over time what the best way to work with it would be.

One possibility I could envison would be trivial "gnome-shell-for-foo" jhbuild lists.  So for example, gnome-shell's current hacky moduleset could be:  gnome-shell-for-fedora-15.list  which would just be a list of dependencies *newer* than the ones from the distro.

Could we make that even better?  Yes, probably - if jhbuild could know *statically* before even trying to build a module what versions it depended on, we could be a lot smarter.  For example, if we took pkg-config checks out of configure.ac and put them into git in a file named DEPS or something, that would help a lot.
Comment 34 Colin Walters 2011-06-24 14:12:09 UTC
(In reply to comment #32)
> (In reply to comment #28)
> > Created an attachment (id=190538) [details] [review] [details] [review]
> > sysdeps: New command
> > 
> > This command parses the moduleset and finds all .pc files that
> > are referenced by the given moduleset, and installs them.
> > 
> > A key use case here is partial jhbuilds; for example, say you know
> > you can avoid building gtk+-2.  Simply:
> > 
> > jhbuild sysdeps gtk+-2
> 
> What about a new configuration 'modules_pick' or similar to slice a moduleset.
> Everything that's missing could be provided by sysdeps.

I'm not sure how that could work without knowing what *versions* of things are needed.  And as I said in comment 33, jhbuild doesn't have that information right now.  We could, but we don't yet, and it'd be quite a bit of work to add (I wouldn't want to maintain dependency versions in jhbuild, since the module already has those in e.g. configure.ac).
Comment 35 Frederic Peters 2011-06-24 14:15:44 UTC
Ok for sysdeps as a separate command, and we'll deal with asking "jhbuild sysdeps" to be run whenever necessary (like new modules bringing new external deps (lcms2 for example)).

I'll continue insisting on using the <branch/> tag for carrying the pkgconfig name, because, as written earlier:

> this would also get it next to a version number, that will have to be considered later on anyway.

And I'd still like to keep it only for tarballs, to avoid inventing another level of stability, between "tracking git" and "tracking a tarball", which would be "tracking git by default but an older version will maybe work".

Switching the GTK+ 2 stack to tarballs can be discussed by the release team.
Comment 36 Milan Bouchet-Valat 2011-06-24 14:24:06 UTC
Silly question: since deps are already versioned in modulesets, what prevents you from checking that distro packages are recent enough? Then, 'jhbuild build' would automatically be able to install packages instead of building for any dep that's available in the distro.
Comment 37 Frederic Peters 2011-06-24 14:32:21 UTC
Deps are only versioned for tarball modules, but sure their versions should be checked. That's why I favour carrying the pkg-config info in the tarball branch tag (which has a version attribute).
Comment 38 Colin Walters 2011-06-24 14:33:15 UTC
(In reply to comment #36)
> Silly question: since deps are already versioned in modulesets, 

I don't see any versioning of dependencies.  To pick a random example from core-deps-3.2:

 <autotools id="libchamplain">
   <branch/>
   <dependencies>
     <dep package="glib"/>
     <dep package="cairo"/>
     <dep package="clutter"/>
     <dep package="gtk+"/>
     <dep package="sqlite3"/>
   </dependencies>
   <suggests>
     <dep package="libsoup"/>
     <dep package="clutter-gtk"/>
   </suggests>
 </autotools>

And get_dependencies() simply checks for a 'package' attribute.
Comment 39 Frederic Peters 2011-06-24 14:53:42 UTC
Tarballs are versioned:

  <autotools id="libxml2">
    <branch module="libxml2/libxml2-2.7.8.tar.gz"
            repo="xmlsoft.org" version="2.7.8"
            hash="sha256:cda23bc9ebd26474ca8f3d67e7d1c4a1f1e7106364b690d822e009fdc3c417ec"
            md5sum="8127a65e8c3b08856093099b52599c86" size="4881808"/>
  </autotools>
Comment 40 Colin Walters 2011-06-24 16:04:13 UTC
(In reply to comment #39)
> Tarballs are versioned:
> 
>   <autotools id="libxml2">
>     <branch module="libxml2/libxml2-2.7.8.tar.gz"
>             repo="xmlsoft.org" version="2.7.8"
>            
> hash="sha256:cda23bc9ebd26474ca8f3d67e7d1c4a1f1e7106364b690d822e009fdc3c417ec"
>             md5sum="8127a65e8c3b08856093099b52599c86" size="4881808"/>
>   </autotools>

I agree it makes sense for sysdeps to only pull in things that are <tarball> in the moduleset, and I'll make that change.
 
However, I think the pkg-config <-> jhbuild module id mapping is generally useful for other things.  For example - in a future where module git trees contain a DEPS file like in comment 33, we no longer need to have <dep> in the jhbuild moduleset XML!  Which would be a nice win.

Then the core moduleset would just be:
<module id="gnome-shell"/>  <module id="epiphany"/>  etc.

core-deps would obviously still contain links to tarballs and such.

So...are you OK with the compromise that "jhbuild sysdeps" is just a tarball replacement, but keep pkg-config on the toplevel element?
Comment 41 Dirk Wallenstein 2011-06-24 16:23:50 UTC
(In reply to comment #34)
> (In reply to comment #32)
> > (In reply to comment #28)
> > > Created an attachment (id=190538) [details] [review] [details] [review] [details] [review]
> > > sysdeps: New command
> > > 
> > > This command parses the moduleset and finds all .pc files that
> > > are referenced by the given moduleset, and installs them.
> > > 
> > > A key use case here is partial jhbuilds; for example, say you know
> > > you can avoid building gtk+-2.  Simply:
> > > 
> > > jhbuild sysdeps gtk+-2
> > 
> > What about a new configuration 'modules_pick' or similar to slice a moduleset.
> > Everything that's missing could be provided by sysdeps.
> 
> I'm not sure how that could work without knowing what *versions* of things are
> needed.  And as I said in comment 33, jhbuild doesn't have that information
> right now.  We could, but we don't yet, and it'd be quite a bit of work to add
> (I wouldn't want to maintain dependency versions in jhbuild, since the module
> already has those in e.g. configure.ac).

And therefore just ensure the package is installed, without version info. Just that the package is installed.  configure will tell if the distro version is not sufficient.
Comment 42 Colin Walters 2011-06-24 16:58:46 UTC
Created attachment 190600 [details] [review]
systeminstall: New utility

No changes, reattaching for ordering
Comment 43 Colin Walters 2011-06-24 16:59:05 UTC
Created attachment 190601 [details] [review]
3.2: Add pkg-config identifiers for a lot of tarball modules

This version targets tarball modules, not GTK+-2
Comment 44 Colin Walters 2011-06-24 16:59:19 UTC
Created attachment 190602 [details] [review]
Parse 'pkg-config' attribute on modules

Reattaching for ordering
Comment 45 Colin Walters 2011-06-24 16:59:38 UTC
Created attachment 190603 [details] [review]
sysdeps: New command

Now targets tarballs only.
Comment 46 Dirk Wallenstein 2011-06-24 17:10:14 UTC
(In reply to comment #44)
> Created an attachment (id=190602) [details] [review]
> Parse 'pkg-config' attribute on modules

Why would you use pkgid instead of pkg_config when storing 'pkg-config'?
Comment 47 Colin Walters 2011-06-24 17:18:35 UTC
(In reply to comment #46)
> (In reply to comment #44)
> > Created an attachment (id=190602) [details] [review] [details] [review]
> > Parse 'pkg-config' attribute on modules
> 
> Why would you use pkgid instead of pkg_config when storing 'pkg-config'?

I mentioned that in the commit message!

"I still called the variable "pkgid" internally because it's shorter
and descriptive enough."

I can change it if you care...
Comment 48 Dirk Wallenstein 2011-06-24 17:24:07 UTC
(In reply to comment #47)
> (In reply to comment #46)
> > (In reply to comment #44)
> > > Created an attachment (id=190602) [details] [review] [details] [review] [details] [review]
> > > Parse 'pkg-config' attribute on modules
> > 
> > Why would you use pkgid instead of pkg_config when storing 'pkg-config'?
> 
> I mentioned that in the commit message!
> 
> "I still called the variable "pkgid" internally because it's shorter
> and descriptive enough."
> 
> I can change it if you care...

10 chars for a variable name that says what it is would be awesome, and not too long.
Comment 49 Colin Walters 2011-06-24 17:37:44 UTC
(In reply to comment #48)
>
> 10 chars for a variable name that says what it is would be awesome, and not too
> long.

Okay, done.  Any other comments?  Has anyone actually tried this other than me?

What I'm doing is I have a Fedora 15 install in virt-manager which is cloned from a zygote.  Then I'm looking at each step along the way, including things like installing gcc.
Comment 50 Colin Walters 2011-06-24 17:40:33 UTC
(In reply to comment #30)
> Hey guys. Nice to see this ticket come back to life. I haven't looked at this
> since 2008, but i did write a prototype (the 2 year old external-deps branch on
> git.gnome.org) so i thought i'd wade in. Unfortunately at some point I merged
> master into my branch so its hard to see the changes I made :(

Yes; don't do that, rebase instead!

> http://git.gnome.org/browse/jhbuild/commit/?h=external-deps&id=29cb0f81d8fd6761f027a385636319260a41283e

I think we concluded here not to do this, at least not yet.

However as far as work to do - we really need someone to look at Debian integration...
Comment 51 Colin Walters 2011-06-24 17:53:19 UTC
Ok actually trying this in my VM, we clearly need one more step, which is for "jhbuild build" to skip things installed via tarball.
Comment 52 Colin Walters 2011-06-24 21:16:36 UTC
Created attachment 190621 [details]
initial experience with jhbuild in a VM

This is a shell session log of me pretending I was following along with the jhbuild setup instructions, and I'm only semi-experienced with Linux/gcc/jhbuild etc.
Comment 53 Colin Walters 2011-06-24 21:18:25 UTC
Created attachment 190622 [details] [review]
Add new partial_build key

Basically, when the partial_build key is set (which it is by default),
when we're gathering a module list (for say "jhbuild build", or just
"jhbuild list"), we look at the installed pkg-config files.  If there
is a local module which matches the id of the tarball, we skip it.

We should still be comparing versions.

But, this allows me to skip building e.g. cairo and nspr if I have
them on the system.
Comment 54 Colin Walters 2011-06-24 21:18:31 UTC
Created attachment 190623 [details] [review]
3.2: Add pkg-config to a few more tarball modules
Comment 55 Colin Walters 2011-06-24 21:18:38 UTC
Created attachment 190624 [details] [review]
3.2 core deps: Add a lot more pkg-config identifiers
Comment 56 Colin Walters 2011-06-24 21:20:23 UTC
Review of attachment 190622 [details] [review]:

Marking this as needs-work; I think this is the right approach, but we still need to do version comparisons.  For example if we bump vala up to 0.13 in GNOME 3.2, but I used "jhbuild sysdeps" to get vala from Fedora 15 which is just 0.12, then "jhbuild build" should start ignoring the system version.
Comment 57 Colin Walters 2011-06-24 21:24:03 UTC
So this gets "jhbuild build meta-gnome-core" down from ~200 modules to 127, which is very nice!  In particular we're removing big things like webkit and landmines like nss and fontconfig. 

Some things like shared-mime-info and iso-codes lack pkg-config files; we should consider adding them.  We might also consider switching e.g. gtk+-2 to a tarball as Frederic said.

At that point, with maybe a few other tweaks we could pretty get within range of ~90 modules for "I want to hack on gnome-shell" which is starting to be quite reasonable.
Comment 58 Colin Walters 2011-06-24 21:33:39 UTC
No idea how I RESOLVED FIXED this...
Comment 59 Colin Walters 2011-06-30 14:40:16 UTC
Created attachment 191034 [details] [review]
3.2: Add pkg-config identifiers for a lot of tarball modules

Some things are still missing .pc files.
Comment 60 Colin Walters 2011-06-30 14:40:32 UTC
Created attachment 191035 [details] [review]
Parse 'pkg-config' attribute on modules
Comment 61 Colin Walters 2011-06-30 14:40:40 UTC
Created attachment 191036 [details] [review]
Add new partial_build key, "sysdeps" command

Basically, when the partial_build key is set (which it is by default),
when we're gathering a module list (for say "jhbuild build", or just
"jhbuild list"), we look at the installed pkg-config files.  If there
is a local module which matches the id of the tarball, we skip it.

We should still be comparing versions.

But, this allows me to skip building e.g. cairo and nspr if I have
them on the system.

To allow developers to fill in using the system if available, add a
new "sysdeps" command.  This command parses the moduleset and finds
all .pc files that are needed by tarballs from the given moduleset,
and installs them.
Comment 62 Colin Walters 2011-06-30 17:05:33 UTC
So, this gets us notably farther.  One issue I see now is when we're actually checking system versions, we will run into the case where the system version is old, but not *too* old.  We're taking the tarball version as a *minimum*, but in reality it may not be so.

Further work items:

* Add <dep perl="Foo::Bar"/>  and <dep python="bar"/>  These are explicit system dependencies.
* Some means to bootstrap installing gcc/automake/libtool.  A hardcoded package list maybe?  Or maybe it's not a big deal and we can just refer people to distribution pages.
Comment 63 Frederic Peters 2011-06-30 22:25:13 UTC
Updates to the tarball versions in the modulesets have been quite conservative over the years, I believe they can be considered as minimum version, even if it's not always strictly true.

Dependencies on perl and python modules (or whatever doesn't ship a .pc file), I didn't think about this yet, probably that means a way of specifying package names (or distromatch to the rescue?).

Bootstrap, automake & stuff, there is already a jhbuild bootstrap command, it should be enhanced to install from distro packages, "jhbuild bootstrap" would then be the same as "jhbuild -m bootstrap sysdeps".

(reopening the bug as it was marked as fixed by error)
Comment 64 Owen Taylor 2011-07-01 14:24:18 UTC
Review of attachment 191035 [details] [review]:

In general, seems like a correct dragging of the attribute through the boilerplate.

I think it would be more readable if docs were added as part of this patch - then the patch would "self-describe" what the attribute does

Aesthetically:

 * Though this patch doesn't add docs, and doesn't add examples, I really don't like the '.pc' in the attribute. At all. Is it 'pkg-config --modversion gtk+-3.0.pc' ? No, it's 'pkg-config --modversion gtk+-3.0'

 * It doesn't really make sense to me to have the attribute called 'pkg-config'. The 'pkg-config for the gtk3 module is gtk+-3.0' doesn't read right to me. I'd have expected 'pkg-id' or something like that. (I do feel less strongly about this then the other - I'm sure given enough time I could train myself to read pkg-config as an abbreviation for pkg-config-id.)

::: jhbuild/modtypes/tarball.py
@@ +102,3 @@
     return AutogenModule(name, branch,
             autogenargs, makeargs, makeinstallargs,
+            dependencies, after, suggests, pkg_config,

any reason not to use a named argument for the newly added one? More named arguments that are used, the less likelyhood of screwing up passing this gigantic number of arguments.
Comment 65 Owen Taylor 2011-07-01 14:50:50 UTC
Review of attachment 191034 [details] [review]:

Other then my complaint about the '.pc' in the IDs, looks plausible. Didn't verify all the module IDs, but presumably wrong module IDs would be obvious in practice if they were there

::: modulesets/gnome-suites-core-deps-3.2.modules
@@ +1293,2 @@
   <autotools id="gtk+-2">
+    <branch checkoutdir="gtk+-2" module="gtk+" revision="gtk-2-24" />

stray whitespace change
Comment 66 Owen Taylor 2011-07-01 17:52:20 UTC
Review of attachment 191036 [details] [review]:

Various comments at various levels from general interface and future ideas to nits

::: jhbuild/commands/sysdeps.py
@@ +42,3 @@
+            make_option('--install',
+                        action='store_true', default = False,
+                        help=_('Install pkg-config modules via system'))])

This either is a "subsubcommand" or a "really do it" option - neither seem very jhbuild-ish to me

 jhbuild sysdeps

Deviates from the normal jhbuild pattern of versions 'build buildone clean list..."

So, ideas are:

 jhbuild installsysdeps
 jhbuild checksysdeps

Or if you want to stick with with the sysdeps name

 jhbuild sysdeps
 jhbuild sysdeps --dry-run (or --check-only)

The help isn't that descriptive - what it does is:

 Installs as many modules as possible using system packages rather than tarballs

Because it is installing _modules_ rather than dependencies, I think that points outs a missing piece in the puzzle here (I guess the same thing fredp is mentioning in comment 63) - the components of a GNOME system can be divided into:

 A) GNOME modules
 B) External dependencies
 C) Bootstrap modules
 D) Stuff we don't even have in the bootstrap, but still might need to be installed (GCC, bison, etc.)

This only handles B) - but C) and D) are equally important, or more important. It seems that we should be able to do:

 <metamodule id="common-build-deps">
    <sysdep pkg-config="libX11"/> <!-- pkg-id !!! -->
    <sysdep system="debian" package="automake111"/>
    <sysdep system="fedora" package="automake-1.11"/>
    ....

 </metamodule>

(You might sysdep libX11 from gtk3 rather than from a common metamodule). Or you could have instead:
;
 <sysdep id=automake-1.11">
   <provider system="debian" package="automake111"/>
   <provider system="fedora" package="automake-1.11"/>
 </system>

That puts system dependencies into the common module namespace as a special type of module. Don't know if that's better or just more verbose... it would mean that that moduletype would somehow have to be skipped when doing jhbuild list, etc.

@@ +58,3 @@
+        for pkg_config,(module, req_version, installed_version, new_enough) in module_state.iteritems():
+            if (installed_version is not None) and new_enough:
+                print (_("pkg-config \"%(pkg)s\" is OK; required=%(req)s installed=%(installed)s" % {'pkg': pkg_config,

Leaking this 'pkg-config' noun out to novice people getting involved with GNOME development

  pkg-config "gtk+-3.0" is OK; required=3.0.1, installed=3.0.1

Better as:

  Installed package gtk+-3.0 is new enough; required=3.0.0, installed=3.0.1

Same below. But really, if you want to spew a whole bunch of output, you should make it compact:

 Packages already installed with new enough versions:
   gtk+-3.0 (required=3.0.0, installed=3.0.1)
   ....

 Packages already installed, but too old:
   ...

 Packages not installed:

@@ +82,3 @@
+            else:
+                logging.info(_("Installing dependencies on system: %s" % (' '.join(uninstalled), )))
+            installer.install(uninstalled)

If you are trying to provide good output here about what is going to be done, what the situation is, it's definitely better to split this into:
 
 found = installer.find_available(uninstalled)
 installer.install(found)

Rather than have the installer provide some spew about packages that can't be found - just because we know a pkg-config ID doesn't mean that it's in the repo

It's also just slightly bogus to install too old versions, but probably harmless - we have to handle that in any case.

::: jhbuild/config.py
@@ +42,2 @@
 _known_keys = [ 'moduleset', 'modules', 'skip', 'tags', 'prefix',
+                'partial_build', 'checkoutroot', 'buildroot', 'top_builddir',

partial_build is pretty non-descript as a name; wouldn't something like 'use_system_modules' or something be better?

There's also a nasty trap here:

 - I build with jhbuild, system version is too old
 - I update my system, system version is now new enough
 - jhbuild stops building the module
 - But the version in the jhbuild install directory still wins

I think we need to catch that and yell at the user to wipe the install directory and rebuild everything -c. (Or if we have file lists from your DESTDIR changes, and we don't have .la files screwing things up with absolute path references, we could just clean up the one module)

::: jhbuild/defaults.jhbuildrc
@@ +176,3 @@
 # A string displayed before JHBuild executes a command. String may contain the
 # variables %(command)s, %(cwd)s
+print_command_pattern = '%(command)s'

Can't figure out what was changed here. Trailing newline added/removed?

::: jhbuild/moduleset.py
@@ +131,3 @@
+        # process_sysdeps lets us avoid repeatedly checking system module state when
+        # handling recursive dependencies.
+        if self.config.partial_build and process_sysdeps:

Can this just be done behind the scenes - do it on the first time, keep a flag? having a getter with a flag that means "return something different for this call and subsequent calls" seems like asking for problems.

@@ +198,3 @@
                     t_ms = ModuleSet(self.config)
                     t_ms.modules = self.modules.copy()
+                    dep_modules = t_ms.get_module_list(seed=[depmod.name], process_sysdeps=False)

passing in process_sysdeps=False here seems like encapsulation failure - we're saying "don't process sysdeps" but the result could or could not have processed sysdep

@@ +243,3 @@
+    def get_system_modules(self, modules):
+        if not self.config.partial_build:
+            logging.debug(_("Partial build is not enabled; can't get system modules"))

Isn't this an error not a return-empty-list situation?

::: jhbuild/utils/systeminstall.py
@@ +37,3 @@
+
+    @classmethod
+    def get_installed_pkgconfig(cls):

Shouldn't this be "pkgconfigs" or is it like sheep/sheep ?

@@ +42,3 @@
+        if 'PKG_CONFIG_PATH' in env:
+            del env['PKG_CONFIG_PATH']
+        proc = subprocess.Popen(['pkg-config', '--list-all'], stdout=subprocess.PIPE, env=env, close_fds=True)

Hardcode /usr/bin/pkg-config? pkg-config is in bootstrap.modules though not built by default.

@@ +48,3 @@
+        for line in StringIO(stdout):
+            pkg, rest = line.split(None, 1)
+            pkgs.append(pkg + '.pc')

See, this is what doesn't make sense :-)

@@ +50,3 @@
+            pkgs.append(pkg + '.pc')
+        args = ['pkg-config', '--modversion']
+        args.extend(map(lambda x: x[:-3], pkgs))

♫ and we put the .pc in, and we take the .pc out, we put the .pc in and we shake it all about (no further comments on this subject will be found in this review :-)

@@ +72,3 @@
+        # Explicitly qualify so we don't get the one in jhbuild root
+        args = ['/usr/bin/pkexec', 'yum', 'install']
+        pkgconfig_provides = map(lambda x: 'pkgconfig(' + x[:-3] + ')', pkgconfig_ids)

I really don't like using pkexec or sudo from something hidden inside a script - the auth dialog just says that we're trying to run yum, so that's sort of scary. Certainly, the gpk experience is theoretically better, though the fact that PackageKit has no way of installing anything other than literal package names is a serious deficiency in making that theoretical advantage practical. Plus other problems, eg:

$ pkcon install cheese-libs-devel
More than one package matches:
1. cheese-libs-devel-1:3.0.1-1.fc15.i686 [fedora]
2. cheese-libs-devel-1:3.0.1-1.fc15.x86_64 [fedora]
Please choose the correct package: 2

So, this seems to be the right way to go until Richard is harassed enough to make the GPK-based experience nice.

@@ +91,3 @@
+        pkg_config = pkg_config[:-3]
+        proc = subprocess.Popen(['pkcon', '-p', 'what-provides', 'pkgconfig(%s)' % (pkg_config, ),
+                                 '--filter=arch;newest'], stdout=subprocess.PIPE, close_fds=True)

Since pkgcon what-provides seems to be completely non-functional as far as I can tell - doesn't find pkgconfig(cheese) whether cheese-libs-devel is installed or uninstalled, I'm not sure this class is close enough to be worth including the code. Workarounds are only worth including when they work. I don't know how the pkgconfig per-call overhead compares to the overhead for command line execution of yum, but even if it did a second or so of overhead, for ~100 packages, that's going to be a long time - so I think we should be asking Richard to provide something that doesn't involve a 101 pkcon invocations.

@@ +117,3 @@
+        return cmds.has_command('pkcon')
+
+_classes.append(PkconSystemInstall)

I think it's best to have:

# Ordered from best to worst
_classes = [YumInstall, PkconSystemInstall]

since you wouldn't want to have to reorder the file if you fixed the pkgcon class

@@ +121,3 @@
+if __name__ == '__main__':
+    installer = SystemInstall.find_best()
+    print "Using %r" % (installer, )

Leftover debug spew or excess noise - the installer is about to provide its own spew :-)
Comment 67 Richard Hughes 2011-07-01 18:05:50 UTC
(In reply to comment #66)
> Certainly, the gpk experience is theoretically better, though the fact
> that PackageKit has no way of installing anything other than literal package
> names is a serious deficiency in making that theoretical advantage practical.

That's false. PackageKit can install by package name, a package provide (including files) or based on any search results.

> Plus other problems, eg:
> 
> $ pkcon install cheese-libs-devel
> More than one package matches:
> 1. cheese-libs-devel-1:3.0.1-1.fc15.i686 [fedora]
> 2. cheese-libs-devel-1:3.0.1-1.fc15.x86_64 [fedora]
> Please choose the correct package: 2
> 
> So, this seems to be the right way to go until Richard is harassed enough to
> make the GPK-based experience nice.

I don't think wrapping pkcon is the best way to do this. It's pretty trivial to just do stuff like this using the org.freedesktop.PackageKit session interface. In fact, it's what it was designed for.

> # Ordered from best to worst
> _classes = [YumInstall, PkconSystemInstall]

Please don't call yum or pkcon from inside jhbuild, it's a hack. We've got a low-level system DBus API if you want all the details, or a high level DBus session interface.

Richard.
Comment 68 Owen Taylor 2011-07-01 18:26:13 UTC
(In reply to comment #67)
> (In reply to comment #66)
> > Certainly, the gpk experience is theoretically better, though the fact
> > that PackageKit has no way of installing anything other than literal package
> > names is a serious deficiency in making that theoretical advantage practical.
> 
> That's false. PackageKit can install by package name, a package provide
> (including files) or based on any search results.

OK, let me say "the command line tools being used here" - I couldn't find any way of doing it with gpk-* or pkcon. 

Is the session D-Bus interface new post-f15? I don't see any evidence of it in my installed PackageKit.

I'm sure it's possible to do writing to the daemon - but certainly there's an appeal to the lazy hacker of a yum install command that does exactly what you want in a few lines of code :-)
Comment 69 Richard Hughes 2011-07-01 18:53:08 UTC
(In reply to comment #68)
> Is the session D-Bus interface new post-f15? I don't see any evidence of it in
> my installed PackageKit.

Nah, it's been around since F13 era. It's session activated, so make sure gpk-dbus-service is running if you want to introspect without calling a method.

> I'm sure it's possible to do writing to the daemon - but certainly there's an
> appeal to the lazy hacker of a yum install command that does exactly what you
> want in a few lines of code :-)

Heh, maybe.

Richard.
Comment 70 Dirk Wallenstein 2011-07-02 06:37:57 UTC
(In reply to comment #64)
>  * It doesn't really make sense to me to have the attribute called
> 'pkg-config'. The 'pkg-config for the gtk3 module is gtk+-3.0' doesn't read
> right to me. I'd have expected 'pkg-id' or something like that. (I do feel less
> strongly about this then the other - I'm sure given enough time I could train
> myself to read pkg-config as an abbreviation for pkg-config-id.)

My point was just that it should be named the same in the moduleset and the library.
Comment 71 Colin Walters 2011-07-05 19:10:30 UTC
(In reply to comment #63)
> Updates to the tarball versions in the modulesets have been quite conservative
> over the years, I believe they can be considered as minimum version, even if
> it's not always strictly true.

The problem I see is that if we say bump nss or libxml in a devel cycle, and all of a sudden it's newer than $latest_distro_version.  

I guess maybe that problem is really more solved by the partial_build key and just explicitly skipping them.  Oh well.

> Dependencies on perl and python modules (or whatever doesn't ship a .pc file),
> I didn't think about this yet, probably that means a way of specifying package
> names (or distromatch to the rescue?).

As I was sort of alluding to, both perl and python *already* have "distro" independent identifiers - their module names.  In "import foo", "foo" must be 
globally unique.

And again in RPM land, for Perl currently, these are repressented as Provides.
$ rpm -q --requires gtk-doc|grep perl
/usr/bin/perl  
perl(Cwd)  
perl(Getopt::Long)  
perl(bytes)  
perl(strict)  
$

In Fedora, python modules don't yet have provides, but there's no reason they couldn't.  So in jhbuild, we could simply reuse this syntax - it's nice and obvious IMO.

Actually, we could go back and redo the pkg-config stuff to be:

  <autotools id="nss" provides="pkgconfig(nss)">
   ...

> Bootstrap, automake & stuff, there is already a jhbuild bootstrap command, it
> should be enhanced to install from distro packages, "jhbuild bootstrap" would
> then be the same as "jhbuild -m bootstrap sysdeps".

Ok, that makes sense to me.
Comment 72 Colin Walters 2011-07-05 19:13:52 UTC
(In reply to comment #64)

>  * Though this patch doesn't add docs, and doesn't add examples, I really don't
> like the '.pc' in the attribute. At all. Is it 'pkg-config --modversion
> gtk+-3.0.pc' ? No, it's 'pkg-config --modversion gtk+-3.0'

The problem is that in several of your comments, "package" is used to refer to a pkg-config and a distro package.  Since this patchset involves both, and jhbuild users will have to understand both, makes things very confusing.

The ".pc" on the end makes things nice and explicit.

>  * It doesn't really make sense to me to have the attribute called
> 'pkg-config'. 

It was like 15 minutes of pain to rename it from someone else' suggestion =(

> any reason not to use a named argument for the newly added one?

I will do this.
Comment 73 Colin Walters 2011-07-14 18:53:17 UTC
(In reply to comment #66)

>  jhbuild installsysdeps
>  jhbuild checksysdeps

Dunno, those don't seem particularly appealing.

>  Installs as many modules as possible using system packages rather than
> tarballs

Fixed.
 

>  C) Bootstrap modules
>  D) Stuff we don't even have in the bootstrap, but still might need to be
> installed (GCC, bison, etc.)
> 
> This only handles B) - but C) and D) are equally important, or more important.
> It seems that we should be able to do:

Honestly I see C and D as pretty much the same; i mean, bootstrap is just inconsistent right now.  In reality we depend on Perl too, but it's not in bootstrap...

> Leaking this 'pkg-config' noun out to novice people getting involved with GNOME
> development

Again though, I don't see a way to avoid this given we have to deal in this case with both pkg-config and distro packages.
 
> Same below. But really, if you want to spew a whole bunch of output, you should
> make it compact:
> 
>  Packages already installed with new enough versions:
>    gtk+-3.0 (required=3.0.0, installed=3.0.1)

Fixed.

> If you are trying to provide good output here about what is going to be done,
> what the situation is, it's definitely better to split this into:
> 
>  found = installer.find_available(uninstalled)
>  installer.install(found)

Yes, but that's hard =(  We'd need to query the package database twice, and have two interfaces for systeminstall.
 
> partial_build is pretty non-descript as a name; wouldn't something like
> 'use_system_modules' or something be better?

Maybe.  I plan to use the key not only for sysdeps, but also to determine whether or not to e.g. include /usr/share in the XDG_DATA_DIRS.
 
> There's also a nasty trap here:
> 
>  - I build with jhbuild, system version is too old
>  - I update my system, system version is now new enough
>  - jhbuild stops building the module
>  - But the version in the jhbuild install directory still wins
> 
> I think we need to catch that and yell at the user to wipe the install
> directory and rebuild everything -c.

Or rather, just keep building the module; we know if we ever built it, since it's in the packagedb.  This turns out to be annoying to implement for code structural reasons, but I will look at it.

> ::: jhbuild/defaults.jhbuildrc
> @@ +176,3 @@
>  # A string displayed before JHBuild executes a command. String may contain the
>  # variables %(command)s, %(cwd)s
> +print_command_pattern = '%(command)s'
> 
> Can't figure out what was changed here. Trailing newline added/removed?

Yes, added a newline.
 
> Can this just be done behind the scenes - do it on the first time, keep a flag?
> having a getter with a flag that means "return something different for this
> call and subsequent calls" seems like asking for problems.

Not really; as I said in the comments, this is recursion; the function calling itself.  Keeping state around would be conceptually wrong.
 
> @@ +243,3 @@
> +    def get_system_modules(self, modules):
> +        if not self.config.partial_build:
> +            logging.debug(_("Partial build is not enabled; can't get system
> modules"))
> 
> Isn't this an error not a return-empty-list situation?

Fixed.

> ::: jhbuild/utils/systeminstall.py
> @@ +37,3 @@
> +
> +    @classmethod
> +    def get_installed_pkgconfig(cls):
> 
> Shouldn't this be "pkgconfigs" or is it like sheep/sheep ?

Fixed.

> Hardcode /usr/bin/pkg-config? pkg-config is in bootstrap.modules though not
> built by default.

Eh.  If we hit a problem there we can look at it, but unsetting PKG_CONFIG_PATH should be fine.
 
> @@ +48,3 @@
> +        for line in StringIO(stdout):
> +            pkg, rest = line.split(None, 1)
> +            pkgs.append(pkg + '.pc')
> 
> See, this is what doesn't make sense :-)

Fixed.

> I really don't like using pkexec or sudo from something hidden inside a script
> - the auth dialog just says that we're trying to run yum, so that's sort of
> scary. Certainly, the gpk experience is theoretically better, though the fact
> that PackageKit has no way of installing anything other than literal package
> names is a serious deficiency in making that theoretical advantage practical.
> Plus other problems, eg:

Yeah, I'll address PackageKit later.
 
> # Ordered from best to worst
> _classes = [YumInstall, PkconSystemInstall]
> 
> since you wouldn't want to have to reorder the file if you fixed the pkgcon
> class

Fixed.

> @@ +121,3 @@
> +if __name__ == '__main__':
> +    installer = SystemInstall.find_best()
> +    print "Using %r" % (installer, )
> 
> Leftover debug spew or excess noise - the installer is about to provide its own
> spew :-)

It's not debug spew; it's just a runnable test case.
Comment 74 Colin Walters 2011-07-14 20:58:31 UTC
(In reply to comment #67)

> Please don't call yum or pkcon from inside jhbuild, it's a hack. We've got a
> low-level system DBus API if you want all the details, or a high level DBus
> session interface.

So, the high level interface apparently doesn't have a way to resolve provides.

The low level interface is complex...we'd need to use dbus-python.  Or I guess, just write it in C.

But really though, it'd be nice if PackageKit shipped a pkcon command to do this.
Comment 75 Colin Walters 2011-07-15 18:10:19 UTC
Created attachment 192047 [details] [review]
Parse <pkg-config> node
Comment 76 Colin Walters 2011-07-15 18:10:49 UTC
Created attachment 192048 [details] [review]
3.2: Add pkg-config identifiers for a lot of tarball modules

Some things are still missing .pc files.
Comment 77 Colin Walters 2011-07-15 18:11:11 UTC
Created attachment 192049 [details] [review]
Add new partial_build key, "sysdeps" command

Basically, when the partial_build key is set (which it is by default),
when we're gathering a module list (for say "jhbuild build", or just
"jhbuild list"), we look at the installed pkg-config files.  If there
is a local module which matches the id of the tarball, we skip it.

We should still be comparing versions.

But, this allows me to skip building e.g. cairo and nspr if I have
them on the system.

To allow developers to fill in using the system if available, add a
new "sysdeps" command.  This command parses the moduleset and finds
all .pc files that are needed by tarballs from the given moduleset,
and installs them.
Comment 78 Colin Walters 2011-07-15 18:11:21 UTC
Created attachment 192050 [details] [review]
config.py: If partial_build is set, explicitly include system paths

Previously, the way config.py handled environment variables was
inconsistent.  For some of them (e.g. PKG_CONFIG_PATH), we always
included the system path.  For others (e.g. XDG_DATA_DIRS), we didn't.
For still others, it's not (easily) possible for us to exclude the
system, even if we wanted to (e.g. LD_LIBRARY_PATH), so we just
inherit by default.

At a high level, people doing partial builds *definitely* want system
paths.  In particular, if you're doing a partial build of gnome-shell,
you won't get any system applications unless you set XDG_DATA_DIRS.
Comment 79 Colin Walters 2011-07-15 20:51:18 UTC
Ok, honestly I think we're in a state of rough consensus, and while this patch isn't *perfect* (i'm not totally happy with the sysdeps command), at this point I think we need more real-world experience, and that only comes from it landing - it's not like we can't change things later.

The main pain is to be sitting on the patch which touches almost every tarball in the moduleset.  We know we want the <pkg-config> node, so let's get that in now.
Comment 80 Colin Walters 2011-07-15 20:51:53 UTC
Attachment 192047 [details] pushed as 19b2e01 - Parse <pkg-config> node
Attachment 192048 [details] pushed as 78bd2bb - 3.2: Add pkg-config identifiers for a lot of tarball modules
Attachment 192049 [details] pushed as 3a2d06b - Add new partial_build key, "sysdeps" command
Attachment 192050 [details] pushed as faa20f5 - config.py: If partial_build is set, explicitly include system paths
Comment 81 Frederic Peters 2011-07-18 12:17:15 UTC
(In reply to comment #79)
> Ok, honestly I think we're in a state of rough consensus, and while this patch
> isn't *perfect* (i'm not totally happy with the sysdeps command), at this point
> I think we need more real-world experience, and that only comes from it landing
> - it's not like we can't change things later.

I am not really happy about how it turned out, but at least could you open a bug to track the implementation of version handling?
Comment 82 Colin Walters 2011-07-18 16:00:53 UTC
(In reply to comment #81)
> (In reply to comment #79)
> > Ok, honestly I think we're in a state of rough consensus, and while this patch
> > isn't *perfect* (i'm not totally happy with the sysdeps command), at this point
> > I think we need more real-world experience, and that only comes from it landing
> > - it's not like we can't change things later.
> 
> I am not really happy about how it turned out, but at least could you open a
> bug to track the implementation of version handling?

Hmm...which version handling?  I can think of two issues:

* Keep building even when system dependency becomes new enough (the bug owen mentioned) - just filed this as bug 654855
* Tarballs act as a minimum version
Comment 83 Frederic Peters 2011-07-21 16:52:05 UTC
> Problem 3:
> Differing versions. Eww. What if my distro has a version thats too old. Want if
> i want to get more than just external deps from pkgkit. I don't need latest gtk
> for example. This setup starts to completely fall apart. We need to make the
> dependency resolver aware of versions.. The more i think about that, the more i
> think this whole approach is fail...

That would be "Tarballs act as a minimum version".