Created attachment 68728 [details] [review]
Patch to set up checkout modes

This patch adds support to establish checkout modes in .jhbuildrc file. It adds three options:
   * checkout_mode (can be export, update, clobber and copy). It establishes the default checkout mode for all modules
   * copy_dir: the directory where checkouts are done in copy mode.
   * module_checkout_mode: array of checkout modes for specific modules.

An example of use can be adding this parameters to jhbuildrc:
checkout_mode = 'update'
module_checkout_mode['cairo'] = 'copy'
copy_dir = os.path.join('/usr','local','devel','user', 'copy')

I don't understand the point of the "export" mode. Would it make checkouts much faster? Otherwise why would you use it?

(In reply to comment #2)
> I don't understand the point of the "export" mode. Would it make checkouts much
> faster? Otherwise why would you use it?
> 

Should make easier to implement "tarball" steps in a continuous integration environment. The idea would be always provide (for any kind of repository) a single way to get the source tree without version control annoyance.

This way we can implement generic methods for getting source. "export" checkout mode makes this easier. Anyway, it's not a priority, we're using clobber and copy modes, but not export (we're not generating automagic tarballs yet).

Created attachment 83650 [details] [review]
Patch update

The build brigade is using this patch nowadays for its modified jhbuild. It would be nice to get it merged. This patch makes the other one obsolete.

In tarball.py, could the new branch_id be renamed to module_id ?

Also, _wipedir, could it be implemented using buildscript.execute('rm -rf...'), the advantage is the command is shown in logs.  (and it could probably go into
the base class instead of being copy/pasted in all modules)

Same remark for _copy.

There is "cmd = ['svn', 'export', self.module]" in git.py; which is probably a copy/paste error.  If you can't implement the feature right now for git and other
alternative vcs; it is not such a big problem, it can always be done later since current behaviour will not stop working.


That's all for a first look; thanks for your work on this.

I will fix those issues and upload a new patch

Iago: What is the status of a newer patch?

(In reply to comment #7)
> Iago: What is the status of a newer patch?

Still not done, It is pity because it is the only one remaining to be able to use jhbuild head in the gnome buildbot. I hope I can have a new version in a couple of weeks.

Created attachment 96926 [details] [review]
Updated version of the previous patch

Includes:

- Updated patch to match current jhbuild head (r1646)
- Uses buildscript to remove and copy files in _copy and _wipedir methods.
- Fixes copy&paste error in git.py.

Some remarks:

- Only implements functionality for tarball, cvs, svn add git repos. This should not be a problem as Frederic states in his last comment. 
- I don't know how I can implement an export in git (tried using git-archive but I'm not sure I can make it work as I'd like), so this just checkouts. 
- Some stuff might be refactored in a base class, as frederic suggested, but did not implement this yet to not complicate the patch.
- Support for other repos like bzr, arch, hg and darcs should be added in the future with another patch.

I've tested this and seems to work ok but let me know if anything is wrong with the patch anyway and I'll fix it.

I would add a remove method to packagedb, so
  buildscript.packagedb.add(self.name, '')
be changed to
  buildscript.packagedb.remove(self.name)

I would remove unused distutil import.

I would use lists instead of strings in buildsript.execute() calls.

Then it will be fine.

Created attachment 96930 [details] [review]
updated patch according to my comments

Iago, could you check this updated patch in your buildbot setup ?

It does not seem to work properly, some error cases:

$ jhbuild buildone cairo-gtk-engine # This is a cvs repo
...

+ Trace 169063

---

$ jhbuild buildone gtk+ # This is a gtk+ repo
...
  File "/home/develslave/sources/jhbuild-HEAD/jhbuild/versioncontrol/__init__.py", line 98, in _checkout_mode
    if self.config.module_checkout_mode.get(self.branch_id):
AttributeError: 'SubversionBranch' object has no attribute 'branch_id'

$ jhbuild buildone loudmouth # This is a git repo

  File "/home/develslave/sources/jhbuild-HEAD/jhbuild/versioncontrol/__init__.py", line 98, in _checkout_mode
    if self.config.module_checkout_mode.get(self.branch_id):
AttributeError: 'GitBranch' object has no attribute 'branch_id'


In order to test the patch I usually test the above repos as long as 'cairo' (tarball repo), in the following scenarios:

1. checkout_mode = 'clobber'

2. checkout_mode = 'copy'
   copy_dir = '/tmp'

3. checkout_mode = 'export'

4. checkout_mode = 'update'

Created attachment 96951 [details] [review]
updated patch

Fixed mistake when factoring things to Branch base class.

Misses os import in jhbuild/versioncontrol/__init__.py.

Besides that I see:

'update' mode for svn does not work well (it always does a checkout)
'copy' mode for svn tries to checkout instead of update. Probably the same error as above.

I can take a deeper look at these issues tomorrow and upload a new patch.

Created attachment 96985 [details] [review]
Fixes commented issues

Fixed issues with svn. 

The problem was the check in order to know whether we have to update or checkout, it was using os.path.basename(self.module), but as self.module contains the URL, this is not valid. I changed it to use os.path.basename(self.srcdir). Before that I tried using self.checkoutdir but this does not always work and it seems srcdir property properly handles these situations (well, for git I had to use get_checkoutdir() instead since the srcdir property seems to be defined in a different way here)

Frederic, I've tested it and seems to work ok in all cases, however I'd feel better if you do some tests too, just in case :). There is no hurry, so take the time you need for this.

Created attachment 96987 [details] [review]
new version

New pass on it, and I actually tested it a little bit.  It had a big problem with regards to module_checkout_mode (where the key would not be module @id attribute) which was confusing.  So I moved setting of checkout_mode out of versioncontrol/.  I tested both CVS and Subversion with the following jhbuildrc snippet and it works.  I also added a check for checkout_mode value (so an invalid value is caugth early on).

checkout_mode = 'update'
module_checkout_mode['gtk-doc'] = 'clobber'
module_checkout_mode['gstreamer-0-8'] = 'clobber'

It seems to work fine, feel free to commit :)
Thanks Frederic!

Urgh, just found one minor problem, module_checkout_mode does not seem to work ok for tarballs.

checkout_mode = 'update'
module_checkout_mode['cairo'] = 'clobber'

This is supossed to remove cairo src and then unpack the tarball (this is the way it works if we set checkout_mode = 'clobber'), but it just unpacks. It is not a major issue anyway, since this functionality for tarballs is not that important after all.

Created attachment 97006 [details] [review]
Fixes module_checkout_mode for tarballs

Fixes the issue I commented before. The problem was that in jhbuild/modtypes/tarball.py in do_start method, the module_checkout_mode was not being checked.

I removed your last edit to jhbuild/versioncontrol/tarball.py since it is required for "new style" tarball support (such as the ones in bootstrap.modules).

Then I commited :)

2007-10-11  Frederic Peters  <fpeters@0d.be>

        * jhbuild/config.py:
        * jhbuild/defaults.jhbuildrc:
        * jhbuild/modtypes/autotools.py:
        * jhbuild/modtypes/distutils.py:
        * jhbuild/modtypes/linux.py:
        * jhbuild/modtypes/mesa.py:
        * jhbuild/modtypes/perl.py:
        * jhbuild/modtypes/tarball.py:
        * jhbuild/modtypes/testmodule.py:
        * jhbuild/utils/packagedb.py:
        * jhbuild/versioncontrol/__init__.py:
        * jhbuild/versioncontrol/cvs.py:
        * jhbuild/versioncontrol/git.py:
        * jhbuild/versioncontrol/svn.py:
        * jhbuild/versioncontrol/tarball.py: added other checkout modes
        (clobber, export, copy), initial patch by Jose Dapena Paz, updated by
        Iago Toral, commented and edited by myself.  (closes: #347114)

Created attachment 97989 [details] [review]
fix git-svn and git-cvsimport

The previous patch breaks git-svn and git-cvsimport usage.
This one fix it, and also try to make GitBranch code easier to read.

Furthermore, those tools have now better command line (git-svn clone/rebase).

git stash makes also life easier to save uncommitted changes.

Marc-Andre: oops, forgot to tell you it was okay to commit.

For reference that last patch had been commited.