GNOME Bugzilla – Bug 681344
sysdeps --install doesn't work for path systemmodule and c_include systemmodule
Last modified: 2012-10-23 11:29:51 UTC
Bug 671042 added systemmodule path search support and c_include search support. Enhance sysdeps --install to install the required systemmodule. For example: <systemmodule id="ruby" > <branch repo="system" version="1.8"/> <systemdependencies> <dep type="path" name="ruby"/> </systemdependencies> </systemmodule> Then 'jhbuild sysdeps --install' will install the ruby package.
For the PackageKit backend, use Transaction's SearchFiles() [1]. SearchFiles doesn't support globbing, so we can't search for file **/ruby. Have to hard-code search for /usr/bin/ruby for type="path". Hard-code prefix /usr/include for type="c_include". [1] http://www.packagekit.org/gtk-doc/Transaction.html#Transaction.SearchFiles
Created attachment 226215 [details] [review] Implement sysdeps --install for path systemmodules (PKSystemInstall) With this patch 'sysdeps --install' will install missing systemmodules like: <systemmodule id="cracklib"> <branch repo="system"/> <systemdependencies> <dep type="c_include" name="crack.h"/> </systemdependencies> </systemmodule> Both the type="c_include" and type="path" systemmodules are supported. This patch modifies the systeminstall.SystemInstall interface. Before was a list of pkgconfig names without the .pc extention. Now is a list of tuples (module name, filename). Some examples of filename are: gdbm.pc, /usr/include/crack.h, /usr/bin/bison PackageKit doesn't support globbing, so we can't search for file **/include/crack.h. Instead, ask PackageKit for /usr/include/crack.h, where /usr is customisable via new variable 'system_prefix'. 'system_prefix' can be changed via ~/.config/jhbuildrc. I refactored class PKSystemInstall to add a new method '_get_new_transaction'. '_get_new_transaction' sets up all the dbus interface, needed for each query. Before, this setup was need twice (and the code was copied and pasted). I needed it a 3rd time, so refactored.
Created attachment 226216 [details] [review] Implement sysdeps --install for path systemmodules (YumSystemInstall) Implement the modified systeminstall.SystemInstall interface for YumSystemInstall.
Created attachment 226217 [details] [review] Implement sysdeps --install for path systemmodules (AptSystemInstall) Implement the modified systeminstall.SystemInstall interface for AptSystemInstall. I'm not able to test the AptSystemInstall changes - I would appreciate it if someone with an apt based system could test please. Otherwise I'll setup a virtual machine when I have a chance.
Created attachment 226218 [details] [review] sysdeps --install should work with partial_build = False Attachment 226215 [details] needs to be applied before applying this patch.
Review of attachment 226215 [details] [review]: ::: jhbuild/utils/systeminstall.py @@ +210,2 @@ + for module_name, filename in uninstalled: + if filename.endswith('.pc'): This is a hack...can we use separate variables (e.g. pass down pkgconfig_ids, file_path, etc.?) Or maybe make separate classes like PkgConfigRequest, FilePathRequest and use isinstance()? Applies to all 3 backend updates.
Created attachment 226455 [details] [review] Implement sysdeps --install for path systemmodules (PKSystemInstall) Implement Colin's separate variables suggestion.
Created attachment 226456 [details] [review] Implement sysdeps --install for path systemmodules (YumSystemInstall)
Created attachment 226457 [details] [review] Implement sysdeps --install for path systemmodules (AptSystemInstall)
Created attachment 226458 [details] [review] sysdeps --install now work with partial_build = False
Review of attachment 226455 [details] [review]: This looks better.
Review of attachment 226456 [details] [review]: Looks correct.
Review of attachment 226457 [details] [review]: Also looks correct.
Review of attachment 226458 [details] [review]: I don't understand this patch - the commit message isn't clear, and I don't see any changes in the actual patch itself aside from whitespace?
(In reply to comment #14) > Review of attachment 226458 [details] [review]: > I don't understand this patch - the commit message isn't clear, and I don't see > any changes in the actual patch itself aside from whitespace? Thank you for the review. Currently, if you have 'partial_build = False', 'jhbuild sysdeps --install' doesn't do anything. It should install the systemmodules (only). Yes, it is whitespace only. Currently, the install code scoped below 'if config.partial_build:'. The patch moves the install code to method scope 'def run(...)'.
Review of attachment 226458 [details] [review]: Oh, I see. OK.
On Fedora 17, two packages provide /usr/include/gdbm.h. This causes sysdeps --install to fail with the error below. Nothing is installed: I: Installing dependencies on system: exiv2 bison cracklib gdbm I: Installing: gdbm-devel;1.10-2.fc17;x86_64;fedora bison;2.5-3.fc17;x86_64;fedora compat-gdbm-devel;1.8.3-11.fc17;x86_64;fedora cracklib-devel;2.8.18-3.fc17;x86_64;fedora exiv2-devel;0.22-6.fc17;x86_64;updates E: PackageKit: compat-gdbm-devel conflicts with gdbm-devel-1.10-2.fc17.x86_64 I: Complete!
Created attachment 226629 [details] [review] Don't install compat- packages on sysdeps --install I tested this patch. 'sysdeps --install' will install gdbm-devel and not compat-gdbm-devel. I also removed an erroneous parameter to SearchFiles. This patch applies on top of attachment 226455 [details] [review]. I did that to make the delta easier to review. I will squash this patch into attachment 226455 [details] [review] before committing. I tested YumSystemInstall - it doesn't have this problem so no extra patch necessary.
Comment on attachment 226455 [details] [review] Implement sysdeps --install for path systemmodules (PKSystemInstall) Committed. http://git.gnome.org/browse/jhbuild/commit/?id=0cb02c5f542ffaf7dbb3f4d551739cfe196b8625
Comment on attachment 226456 [details] [review] Implement sysdeps --install for path systemmodules (YumSystemInstall) Committed. http://git.gnome.org/browse/jhbuild/commit/?id=437d6d3f8a98848d7049ea96bce0c150069639d7
Comment on attachment 226457 [details] [review] Implement sysdeps --install for path systemmodules (AptSystemInstall) Committed. http://git.gnome.org/browse/jhbuild/commit/?id=3e27946ce6c57b35180839a3e745f618daeb90c2
Comment on attachment 226458 [details] [review] sysdeps --install now work with partial_build = False Committed. http://git.gnome.org/browse/jhbuild/commit/?id=0f5cb8a5a39b29ddcbc1bd04edfaad53dfaaed2b