[PATCH v3 0/7] New remote-bzr remote helper

[PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

Hi,

This is a re-roll of the previous series to add support to fetch and push
special modes, and refactor some related code.

Cheers.

Changes since v2:

 * Add support for special modes
 * Minor refactoring and cleanups

Changes since v1:

 * Rewritten to avoid bzr-fastimport

Felipe Contreras (7):
  Add new remote-bzr transport helper
  remote-bzr: add support for pushing
  remote-bzr: add support for remote repositories
  remote-bzr: update working tree
  remote-bzr: add simple tests
  remote-bzr: add support for fecthing special modes
  remote-bzr: add support to push special modes

 contrib/remote-helpers/git-remote-bzr | 713 ++++++++++++++++++++++++++++++++++
 contrib/remote-helpers/test-bzr.sh    | 143 +++++++
 2 files changed, 856 insertions(+)
 create mode 100755 contrib/remote-helpers/git-remote-bzr
 create mode 100755 contrib/remote-helpers/test-bzr.sh

-- 
1.8.0

[PATCH v3 1/7] Add new remote-bzr transport helper

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 352 ++++++++++++++++++++++++++++++++++
 1 file changed, 352 insertions(+)
 create mode 100755 contrib/remote-helpers/git-remote-bzr

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
new file mode 100755
index 0000000..b6be9d6
--- /dev/null
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -0,0 +1,352 @@
+#!/usr/bin/env python
+#
+# Copyright (c) 2012 Felipe Contreras
+#
+
+#
+# Just copy to your ~/bin, or anywhere in your $PATH.
+# Then you can clone with:
+# % git clone bzr::/path/to/bzr/repo/or/url
+#
+# For example:
+# % git clone bzr::$HOME/myrepo
+# or
+# % git clone bzr::lp:myrepo
+#
+
+import sys
+
+import bzrlib
+bzrlib.initialize()
+
+import bzrlib.plugin
+bzrlib.plugin.load_plugins()
+
+import sys
+import os
+import json
+import re
+
+NAME_RE = re.compile('^([^<>]+)')
+AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
+
+def die(msg, *args):
+    sys.stderr.write('ERROR: %s\n' % (msg % args))
+    sys.exit(1)
+
+def warn(msg, *args):
+    sys.stderr.write('WARNING: %s\n' % (msg % args))
+
+def gittz(tz):
+    return '%+03d%02d' % (tz / 3600, tz % 3600 / 60)
+
+class Marks:
+
+    def __init__(self, path):
+        self.path = path
+        self.tips = {}
+        self.marks = {}
+        self.last_mark = 0
+        self.load()
+
+    def load(self):
+        if not os.path.exists(self.path):
+            return
+
+        tmp = json.load(open(self.path))
+        self.tips = tmp['tips']
+        self.marks = tmp['marks']
+        self.last_mark = tmp['last-mark']
+
+    def dict(self):
+        return { 'tips': self.tips, 'marks': self.marks, 'last-mark' : self.last_mark }
+
+    def store(self):
+        json.dump(self.dict(), open(self.path, 'w'))
+
+    def __str__(self):
+        return str(self.dict())
+
+    def from_rev(self, rev):
+        return self.marks[rev]
+
+    def next_mark(self):
+        self.last_mark += 1
+        return self.last_mark
+
+    def get_mark(self, rev):
+        self.last_mark += 1
+        self.marks[rev] = self.last_mark
+        return self.last_mark
+
+    def is_marked(self, rev):
+        return self.marks.has_key(rev)
+
+    def get_tip(self, branch):
+        return self.tips.get(branch, None)
+
+    def set_tip(self, branch, tip):
+        self.tips[branch] = tip
+
+class Parser:
+
+    def __init__(self, repo):
+        self.repo = repo
+        self.line = self.get_line()
+
+    def get_line(self):
+        return sys.stdin.readline().strip()
+
+    def __getitem__(self, i):
+        return self.line.split()[i]
+
+    def check(self, word):
+        return self.line.startswith(word)
+
+    def each_block(self, separator):
+        while self.line != separator:
+            yield self.line
+            self.line = self.get_line()
+
+    def __iter__(self):
+        return self.each_block('')
+
+    def next(self):
+        self.line = self.get_line()
+        if self.line == 'done':
+            self.line = None
+
+def rev_to_mark(rev):
+    global marks
+    return marks.from_rev(rev)
+
+def fixup_user(user):
+    name = mail = None
+    user = user.replace('"', '')
+    m = AUTHOR_RE.match(user)
+    if m:
+        name = m.group(1)
+        mail = m.group(2).strip()
+    else:
+        m = NAME_RE.match(user)
+        if m:
+            name = m.group(1).strip()
+
+    return '%s <%s>' % (name, mail)
+
+def get_filechanges(cur, prev):
+    modified = {}
+    removed = {}
+
+    changes = cur.changes_from(prev)
+
+    for path, fid, kind in changes.added:
+        modified[path] = fid
+    for path, fid, kind in changes.removed:
+        removed[path] = None
+    for path, fid, kind, mod, _ in changes.modified:
+        modified[path] = fid
+    for oldpath, newpath, fid, kind, mod, _ in changes.renamed:
+        removed[oldpath] = None
+        modified[newpath] = fid
+
+    return modified, removed
+
+def export_files(tree, files):
+    global marks, filenodes
+
+    final = []
+    for path, fid in files.iteritems():
+        h = tree.get_file_sha1(fid)
+
+        mode = '100644'
+
+        # is the blob already exported?
+        if h in filenodes:
+            mark = filenodes[h]
+        else:
+            d = tree.get_file_text(fid)
+
+            mark = marks.next_mark()
+            filenodes[h] = mark
+
+            print "blob"
+            print "mark :%u" % mark
+            print "data %d" % len(d)
+            print d
+
+        final.append((mode, mark, path))
+
+    return final
+
+def export_branch(branch, name):
+    global prefix, dirname
+
+    ref = '%s/heads/%s' % (prefix, name)
+    tip = marks.get_tip(name)
+
+    repo = branch.repository
+    repo.lock_read()
+    revs = branch.iter_merge_sorted_revisions(None, tip, 'exclude', 'forward')
+    count = 0
+
+    revs = [revid for revid, _, _, _ in revs if not marks.is_marked(revid)]
+
+    for revid in revs:
+
+        rev = repo.get_revision(revid)
+
+        parents = rev.parent_ids
+        time = rev.timestamp
+        tz = rev.timezone
+        committer = rev.committer.encode('utf-8')
+        committer = "%s %u %s" % (fixup_user(committer), time, gittz(tz))
+        author = committer
+        msg = rev.message.encode('utf-8')
+
+        msg += '\n'
+
+        if len(parents) == 0:
+            parent = bzrlib.revision.NULL_REVISION
+        else:
+            parent = parents[0]
+
+        cur_tree = repo.revision_tree(revid)
+        prev = repo.revision_tree(parent)
+        modified, removed = get_filechanges(cur_tree, prev)
+
+        modified_final = export_files(cur_tree, modified)
+
+        if len(parents) == 0:
+            print 'reset %s' % ref
+
+        print "commit %s" % ref
+        print "mark :%d" % (marks.get_mark(revid))
+        print "author %s" % (author)
+        print "committer %s" % (committer)
+        print "data %d" % (len(msg))
+        print msg
+
+        for i, p in enumerate(parents):
+            try:
+                m = rev_to_mark(p)
+            except KeyError:
+                # ghost?
+                continue
+            if i == 0:
+                print "from :%s" % m
+            else:
+                print "merge :%s" % m
+
+        for f in modified_final:
+            print "M %s :%u %s" % f
+        for f in removed:
+            print "D %s" % (f)
+        print
+
+        count += 1
+        if (count % 100 == 0):
+            print "progress revision %s (%d/%d)" % (revid, count, len(revs))
+            print "#############################################################"
+
+    repo.unlock()
+
+    revid = branch.last_revision()
+
+    # make sure the ref is updated
+    print "reset %s" % ref
+    print "from :%u" % rev_to_mark(revid)
+    print
+
+    marks.set_tip(name, revid)
+
+def export_tag(repo, name):
+    global tags
+    try:
+        print "reset refs/tags/%s" % name
+        print "from :%u" % rev_to_mark(tags[name])
+        print
+    except KeyError:
+        warn("TODO: fetch tag '%s'" % name)
+
+def do_import(parser):
+    global dirname
+
+    branch = parser.repo
+    path = os.path.join(dirname, 'marks-git')
+
+    print "feature done"
+    if os.path.exists(path):
+        print "feature import-marks=%s" % path
+    print "feature export-marks=%s" % path
+    sys.stdout.flush()
+
+    while parser.check('import'):
+        ref = parser[1]
+        if ref.startswith('refs/heads/'):
+            name = ref[len('refs/heads/'):]
+            export_branch(branch, name)
+        if ref.startswith('refs/tags/'):
+            name = ref[len('refs/tags/'):]
+            export_tag(branch, name)
+        parser.next()
+
+    print 'done'
+
+    sys.stdout.flush()
+
+def do_capabilities(parser):
+    print "import"
+    print "refspec refs/heads/*:%s/heads/*" % prefix
+    print
+
+def do_list(parser):
+    global tags
+    print "? refs/heads/%s" % 'master'
+    for tag, revid in parser.repo.tags.get_tag_dict().items():
+        print "? refs/tags/%s" % tag
+        tags[tag] = revid
+    print "@refs/heads/%s HEAD" % 'master'
+    print
+
+def get_repo(url, alias):
+    origin = bzrlib.controldir.ControlDir.open(url)
+    return origin.open_branch()
+
+def main(args):
+    global marks, prefix, dirname
+    global tags, filenodes
+
+    alias = args[1]
+    url = args[2]
+
+    prefix = 'refs/bzr/%s' % alias
+    tags = {}
+    filenodes = {}
+
+    gitdir = os.environ['GIT_DIR']
+    dirname = os.path.join(gitdir, 'bzr', alias)
+
+    if not os.path.exists(dirname):
+        os.makedirs(dirname)
+
+    repo = get_repo(url, alias)
+
+    marks_path = os.path.join(dirname, 'marks-int')
+    marks = Marks(marks_path)
+
+    parser = Parser(repo)
+    for line in parser:
+        if parser.check('capabilities'):
+            do_capabilities(parser)
+        elif parser.check('list'):
+            do_list(parser)
+        elif parser.check('import'):
+            do_import(parser)
+        else:
+            die('unhandled command: %s' % line)
+        sys.stdout.flush()
+
+    marks.store()
+
+sys.exit(main(sys.argv))
-- 
1.8.0

[PATCH v3 2/7] remote-bzr: add support for pushing

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 295 ++++++++++++++++++++++++++++++++++
 1 file changed, 295 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index b6be9d6..8366234 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -22,13 +22,17 @@ bzrlib.initialize()
 import bzrlib.plugin
 bzrlib.plugin.load_plugins()
 
+import bzrlib.generate_ids
+
 import sys
 import os
 import json
 import re
+import StringIO
 
 NAME_RE = re.compile('^([^<>]+)')
 AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
+RAW_AUTHOR_RE = re.compile('^(\w+) (.+)? <(.*)> (\d+) ([+-]\d+)')
 
 def die(msg, *args):
     sys.stderr.write('ERROR: %s\n' % (msg % args))
@@ -46,6 +50,7 @@ class Marks:
         self.path = path
         self.tips = {}
         self.marks = {}
+        self.rev_marks = {}
         self.last_mark = 0
         self.load()
 
@@ -58,6 +63,9 @@ class Marks:
         self.marks = tmp['marks']
         self.last_mark = tmp['last-mark']
 
+        for rev, mark in self.marks.iteritems():
+            self.rev_marks[mark] = rev
+
     def dict(self):
         return { 'tips': self.tips, 'marks': self.marks, 'last-mark' : self.last_mark }
 
@@ -70,6 +78,9 @@ class Marks:
     def from_rev(self, rev):
         return self.marks[rev]
 
+    def to_rev(self, mark):
+        return self.rev_marks[mark]
+
     def next_mark(self):
         self.last_mark += 1
         return self.last_mark
@@ -82,6 +93,11 @@ class Marks:
     def is_marked(self, rev):
         return self.marks.has_key(rev)
 
+    def new_mark(self, rev, mark):
+        self.marks[rev] = mark
+        self.rev_marks[mark] = rev
+        self.last_mark = mark
+
     def get_tip(self, branch):
         return self.tips.get(branch, None)
 
@@ -116,10 +132,35 @@ class Parser:
         if self.line == 'done':
             self.line = None
 
+    def get_mark(self):
+        i = self.line.index(':') + 1
+        return int(self.line[i:])
+
+    def get_data(self):
+        if not self.check('data'):
+            return None
+        i = self.line.index(' ') + 1
+        size = int(self.line[i:])
+        return sys.stdin.read(size)
+
+    def get_author(self):
+        m = RAW_AUTHOR_RE.match(self.line)
+        if not m:
+            return None
+        _, name, email, date, tz = m.groups()
+        committer = '%s <%s>' % (name, email)
+        tz = int(tz)
+        tz = ((tz / 100) * 3600) + ((tz % 100) * 60)
+        return (committer, int(date), tz)
+
 def rev_to_mark(rev):
     global marks
     return marks.from_rev(rev)
 
+def mark_to_rev(mark):
+    global marks
+    return marks.to_rev(mark)
+
 def fixup_user(user):
     name = mail = None
     user = user.replace('"', '')
@@ -295,9 +336,255 @@ def do_import(parser):
 
     sys.stdout.flush()
 
+def parse_blob(parser):
+    global blob_marks
+
+    parser.next()
+    mark = parser.get_mark()
+    parser.next()
+    data = parser.get_data()
+    blob_marks[mark] = data
+    parser.next()
+
+class CustomTree():
+
+    def __init__(self, repo, revid, parents, files):
+        global files_cache
+
+        self.repo = repo
+        self.revid = revid
+        self.parents = parents
+        self.updates = files
+
+        def copy_tree(revid):
+            files = files_cache[revid] = {}
+            tree = repo.repository.revision_tree(revid)
+            repo.lock_read()
+            try:
+                for path, entry in tree.iter_entries_by_dir():
+                    files[path] = entry.file_id
+            finally:
+                repo.unlock()
+            return files
+
+        if len(parents) == 0:
+            self.base_id = bzrlib.revision.NULL_REVISION
+            self.base_files = {}
+        else:
+            self.base_id = parents[0]
+            self.base_files = files_cache.get(self.base_id, None)
+            if not self.base_files:
+                self.base_files = copy_tree(self.base_id)
+
+        self.files = files_cache[revid] = self.base_files.copy()
+
+    def last_revision(self):
+        return self.base_id
+
+    def iter_changes(self):
+        changes = []
+
+        def get_parent(dirname, basename):
+            parent_fid = self.base_files.get(dirname, None)
+            if parent_fid:
+                return parent_fid
+            parent_fid = self.files.get(dirname, None)
+            if parent_fid:
+                return parent_fid
+            if basename == '':
+                return None
+            d = add_entry(dirname, 'directory')
+            return d[0]
+
+        def add_entry(path, kind):
+            dirname, basename = os.path.split(path)
+            parent_fid = get_parent(dirname, basename)
+            fid = bzrlib.generate_ids.gen_file_id(path)
+            change = (fid,
+                    (None, path),
+                    True,
+                    (False, True),
+                    (None, parent_fid),
+                    (None, basename),
+                    (None, kind),
+                    (None, False))
+            self.files[path] = change[0]
+            changes.append(change)
+            return change
+
+        def update_entry(path, kind):
+            dirname, basename = os.path.split(path)
+            fid = self.base_files[path]
+            parent_fid = get_parent(dirname, basename)
+            change = (fid,
+                    (path, path),
+                    True,
+                    (True, True),
+                    (None, parent_fid),
+                    (None, basename),
+                    (None, kind),
+                    (None, False))
+            self.files[path] = change[0]
+            changes.append(change)
+            return change
+
+        def remove_entry(path, kind):
+            dirname, basename = os.path.split(path)
+            fid = self.base_files[path]
+            parent_fid = get_parent(dirname, basename)
+            change = (fid,
+                    (path, None),
+                    True,
+                    (True, False),
+                    (parent_fid, None),
+                    (None, None),
+                    (None, None),
+                    (None, None))
+            del self.files[path]
+            changes.append(change)
+            return change
+
+        for path, f in self.updates.iteritems():
+            if 'deleted' in f:
+                remove_entry(path, 'file')
+            elif path in self.base_files:
+                update_entry(path, 'file')
+            else:
+                add_entry(path, 'file')
+
+        return changes
+
+    def get_file_with_stat(self, file_id, path=None):
+        return (StringIO.StringIO(self.updates[path]['data']), None)
+
+def parse_commit(parser):
+    global marks, blob_marks, bmarks, parsed_refs
+    global mode
+
+    parents = []
+
+    ref = parser[1]
+    parser.next()
+
+    if ref != 'refs/heads/master':
+        die("bzr doesn't support multiple branches; use 'master'")
+
+    commit_mark = parser.get_mark()
+    parser.next()
+    author = parser.get_author()
+    parser.next()
+    committer = parser.get_author()
+    parser.next()
+    data = parser.get_data()
+    parser.next()
+    if parser.check('from'):
+        parents.append(parser.get_mark())
+        parser.next()
+    while parser.check('merge'):
+        parents.append(parser.get_mark())
+        parser.next()
+
+    files = {}
+
+    for line in parser:
+        if parser.check('M'):
+            t, m, mark_ref, path = line.split(' ', 3)
+            mark = int(mark_ref[1:])
+            f = { 'mode' : m, 'data' : blob_marks[mark] }
+        elif parser.check('D'):
+            t, path = line.split(' ')
+            f = { 'deleted' : True }
+        else:
+            die('Unknown file command: %s' % line)
+        files[path] = f
+
+    repo = parser.repo
+
+    committer, date, tz = committer
+    parents = [str(mark_to_rev(p)) for p in parents]
+    revid = bzrlib.generate_ids.gen_revision_id(committer, date)
+    props = {}
+    props['branch-nick'] = repo.nick
+
+    mtree = CustomTree(repo, revid, parents, files)
+    changes = mtree.iter_changes()
+
+    repo.lock_write()
+    try:
+        builder = repo.get_commit_builder(parents, None, date, tz, committer, props, revid, False)
+        try:
+            list(builder.record_iter_changes(mtree, mtree.last_revision(), changes))
+            builder.finish_inventory()
+            builder.commit(data.decode('utf-8', 'replace'))
+        except Exception, e:
+            builder.abort()
+            raise
+    finally:
+        repo.unlock()
+
+    parsed_refs[ref] = revid
+    marks.new_mark(revid, commit_mark)
+
+def parse_reset(parser):
+    global parsed_refs
+
+    ref = parser[1]
+    parser.next()
+
+    if ref != 'refs/heads/master':
+        die("bzr doesn't support multiple branches; use 'master'")
+
+    # ugh
+    if parser.check('commit'):
+        parse_commit(parser)
+        return
+    if not parser.check('from'):
+        return
+    from_mark = parser.get_mark()
+    parser.next()
+
+    parsed_refs[ref] = mark_to_rev(from_mark)
+
+def do_export(parser):
+    global parsed_refs, dirname
+
+    parser.next()
+
+    for line in parser.each_block('done'):
+        if parser.check('blob'):
+            parse_blob(parser)
+        elif parser.check('commit'):
+            parse_commit(parser)
+        elif parser.check('reset'):
+            parse_reset(parser)
+        elif parser.check('tag'):
+            pass
+        elif parser.check('feature'):
+            pass
+        else:
+            die('unhandled export command: %s' % line)
+
+    repo = parser.repo
+
+    for ref, revid in parsed_refs.iteritems():
+        if ref == 'refs/heads/master':
+            repo.generate_revision_history(revid, marks.get_tip('master'))
+        print "ok %s" % ref
+    print
+
 def do_capabilities(parser):
+    global dirname
+
     print "import"
+    print "export"
     print "refspec refs/heads/*:%s/heads/*" % prefix
+
+    path = os.path.join(dirname, 'marks-git')
+
+    if os.path.exists(path):
+        print "*import-marks %s" % path
+    print "*export-marks %s" % path
+
     print
 
 def do_list(parser):
@@ -316,6 +603,9 @@ def get_repo(url, alias):
 def main(args):
     global marks, prefix, dirname
     global tags, filenodes
+    global blob_marks
+    global parsed_refs
+    global files_cache
 
     alias = args[1]
     url = args[2]
@@ -323,6 +613,9 @@ def main(args):
     prefix = 'refs/bzr/%s' % alias
     tags = {}
     filenodes = {}
+    blob_marks = {}
+    parsed_refs = {}
+    files_cache = {}
 
     gitdir = os.environ['GIT_DIR']
     dirname = os.path.join(gitdir, 'bzr', alias)
@@ -343,6 +636,8 @@ def main(args):
             do_list(parser)
         elif parser.check('import'):
             do_import(parser)
+        elif parser.check('export'):
+            do_export(parser)
         else:
             die('unhandled command: %s' % line)
         sys.stdout.flush()
-- 
1.8.0

[PATCH v3 3/7] remote-bzr: add support for remote repositories

[Felipe Contreras]

Strictly speaking bzr doesn't need any changes to interact with remote
repositories, but it's dead slow.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 8366234..2c05f35 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -546,7 +546,7 @@ def parse_reset(parser):
     parsed_refs[ref] = mark_to_rev(from_mark)
 
 def do_export(parser):
-    global parsed_refs, dirname
+    global parsed_refs, dirname, peer
 
     parser.next()
 
@@ -569,6 +569,8 @@ def do_export(parser):
     for ref, revid in parsed_refs.iteritems():
         if ref == 'refs/heads/master':
             repo.generate_revision_history(revid, marks.get_tip('master'))
+            revno, revid = repo.last_revision_info()
+            peer.import_last_revision_info_and_tags(repo, revno, revid)
         print "ok %s" % ref
     print
 
@@ -597,8 +599,28 @@ def do_list(parser):
     print
 
 def get_repo(url, alias):
+    global dirname, peer
+
+    clone_path = os.path.join(dirname, 'clone')
     origin = bzrlib.controldir.ControlDir.open(url)
-    return origin.open_branch()
+    remote_branch = origin.open_branch()
+
+    if os.path.exists(clone_path):
+        # pull
+        d = bzrlib.controldir.ControlDir.open(clone_path)
+        branch = d.open_branch()
+        result = branch.pull(remote_branch, [], None, False)
+    else:
+        # clone
+        d = origin.sprout(clone_path, None,
+                hardlink=True, create_tree_if_local=False,
+                source_branch=remote_branch)
+        branch = d.open_branch()
+        branch.bind(remote_branch)
+
+    peer = remote_branch
+
+    return branch
 
 def main(args):
     global marks, prefix, dirname
-- 
1.8.0

[PATCH v3 4/7] remote-bzr: update working tree

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 2c05f35..5b89a05 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -571,6 +571,8 @@ def do_export(parser):
             repo.generate_revision_history(revid, marks.get_tip('master'))
             revno, revid = repo.last_revision_info()
             peer.import_last_revision_info_and_tags(repo, revno, revid)
+            wt = peer.bzrdir.open_workingtree()
+            wt.update()
         print "ok %s" % ref
     print
 
-- 
1.8.0

Re: [PATCH v3 4/7] remote-bzr: update working tree

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/remote-helpers/git-remote-bzr | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
> index 2c05f35..5b89a05 100755
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -571,6 +571,8 @@ def do_export(parser):
>              repo.generate_revision_history(revid, marks.get_tip('master'))
>              revno, revid = repo.last_revision_info()
>              peer.import_last_revision_info_and_tags(repo, revno, revid)
> +            wt = peer.bzrdir.open_workingtree()
> +            wt.update()
>          print "ok %s" % ref
>      print

Shouldn't this be squashed as part of one of the earlier patches?
The split between 1/7 (import first) and 2/7 (then support export)
makes sense, but this one looks more like "oops, we forgot to touch
the working tree while updating the history in the previous steps
and here is a fix" to me.

Re: [PATCH v3 4/7] remote-bzr: update working tree

[Felipe Contreras]

On Wed, Nov 28, 2012 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  contrib/remote-helpers/git-remote-bzr | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
>> index 2c05f35..5b89a05 100755
>> --- a/contrib/remote-helpers/git-remote-bzr
>> +++ b/contrib/remote-helpers/git-remote-bzr
>> @@ -571,6 +571,8 @@ def do_export(parser):
>>              repo.generate_revision_history(revid, marks.get_tip('master'))
>>              revno, revid = repo.last_revision_info()
>>              peer.import_last_revision_info_and_tags(repo, revno, revid)
>> +            wt = peer.bzrdir.open_workingtree()
>> +            wt.update()
>>          print "ok %s" % ref
>>      print
>
> Shouldn't this be squashed as part of one of the earlier patches?
> The split between 1/7 (import first) and 2/7 (then support export)
> makes sense, but this one looks more like "oops, we forgot to touch
> the working tree while updating the history in the previous steps
> and here is a fix" to me.

Perhaps. It's not really clear if we should update the working tree at
all. A 'git push' doesn't update the working directory on the remote,
but a 'bzr push' does. I thought it was better to leave this
distinction clear, in case this becomes an issue later on.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 4/7] remote-bzr: update working tree

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Wed, Nov 28, 2012 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>>  contrib/remote-helpers/git-remote-bzr | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
>>> index 2c05f35..5b89a05 100755
>>> --- a/contrib/remote-helpers/git-remote-bzr
>>> +++ b/contrib/remote-helpers/git-remote-bzr
>>> @@ -571,6 +571,8 @@ def do_export(parser):
>>>              repo.generate_revision_history(revid, marks.get_tip('master'))
>>>              revno, revid = repo.last_revision_info()
>>>              peer.import_last_revision_info_and_tags(repo, revno, revid)
>>> +            wt = peer.bzrdir.open_workingtree()
>>> +            wt.update()
>>>          print "ok %s" % ref
>>>      print
>>
>> Shouldn't this be squashed as part of one of the earlier patches?
>> The split between 1/7 (import first) and 2/7 (then support export)
>> makes sense, but this one looks more like "oops, we forgot to touch
>> the working tree while updating the history in the previous steps
>> and here is a fix" to me.
>
> Perhaps. It's not really clear if we should update the working tree at
> all. A 'git push' doesn't update the working directory on the remote,
> but a 'bzr push' does. I thought it was better to leave this
> distinction clear, in case this becomes an issue later on.

If you explained that in the log message, then we would have avoided
this exchange, and more importantly, if you had a in-code comment
there, it may help people who later want to add a knob to leave the
working tree intact.  The seed you sow now to help others, who may
be less skillful and knowledgeable than you are, will help the
project in the longer term.

Thanks.  I don't mind rephrasing that four lines and amend the
log message of what I have in 'pu', if that is what you want.

Re: [PATCH v3 4/7] remote-bzr: update working tree

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> Perhaps. It's not really clear if we should update the working tree at
>> all. A 'git push' doesn't update the working directory on the remote,
>> but a 'bzr push' does. I thought it was better to leave this
>> distinction clear, in case this becomes an issue later on.
> ...
> ...  I don't mind rephrasing that four lines and amend the
> log message of what I have in 'pu', if that is what you want.

... which may look like this.

    remote-bzr: update working tree upon pushing
    
    A 'git push' doesn't update the working directory on the remote, but
    a 'bzr push' does.  Teach the remote helper for bzr to update the
    working tree on the bzr side upon pushing via the "export" command.

Let me know if I grossly misinterpreted/misrepresented what you
wanted to do in this patch.

Thanks.

Re: [PATCH v3 4/7] remote-bzr: update working tree

[Felipe Contreras]

On Thu, Dec 13, 2012 at 5:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> Perhaps. It's not really clear if we should update the working tree at
>>> all. A 'git push' doesn't update the working directory on the remote,
>>> but a 'bzr push' does. I thought it was better to leave this
>>> distinction clear, in case this becomes an issue later on.
>> ...
>> ...  I don't mind rephrasing that four lines and amend the
>> log message of what I have in 'pu', if that is what you want.
>
> ... which may look like this.
>
>     remote-bzr: update working tree upon pushing
>
>     A 'git push' doesn't update the working directory on the remote, but
>     a 'bzr push' does.  Teach the remote helper for bzr to update the
>     working tree on the bzr side upon pushing via the "export" command.
>
> Let me know if I grossly misinterpreted/misrepresented what you
> wanted to do in this patch.

Looks good to me.

Cheers.

-- 
Felipe Contreras

[PATCH v3 5/7] remote-bzr: add simple tests

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-bzr.sh | 111 +++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100755 contrib/remote-helpers/test-bzr.sh

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
new file mode 100755
index 0000000..8594ffc
--- /dev/null
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Felipe Contreras
+#
+
+test_description='Test remote-bzr'
+
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON; then
+	skip_all='skipping remote-bzr tests; python not available'
+	test_done
+fi
+
+if ! "$PYTHON_PATH" -c 'import bzrlib'; then
+	skip_all='skipping remote-bzr tests; bzr not available'
+	test_done
+fi
+
+cmd=<<EOF
+import bzrlib
+bzrlib.initialize()
+import bzrlib.plugin
+bzrlib.plugin.load_plugins()
+import bzrlib.plugins.fastimport
+EOF
+
+if ! "$PYTHON_PATH" -c "$cmd"; then
+	echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
+	skip_all='skipping remote-bzr tests; bzr-fastimport not available'
+	test_done
+fi
+
+check () {
+	(cd $1 &&
+	git log --format='%s' -1 &&
+	git symbolic-ref HEAD) > actual &&
+	(echo $2 &&
+	echo "refs/heads/$3") > expected &&
+	test_cmp expected actual
+}
+
+bzr whoami "A U Thor <author@example.com>"
+
+test_expect_success 'cloning' '
+  (bzr init bzrrepo &&
+  cd bzrrepo &&
+  echo one > content &&
+  bzr add content &&
+  bzr commit -m one
+  ) &&
+
+  git clone "bzr::$PWD/bzrrepo" gitrepo &&
+  check gitrepo one master
+'
+
+test_expect_success 'pulling' '
+  (cd bzrrepo &&
+  echo two > content &&
+  bzr commit -m two
+  ) &&
+
+  (cd gitrepo && git pull) &&
+
+  check gitrepo two master
+'
+
+test_expect_success 'pushing' '
+  (cd gitrepo &&
+  echo three > content &&
+  git commit -a -m three &&
+  git push
+  ) &&
+
+  echo three > expected &&
+  cat bzrrepo/content > actual &&
+  test_cmp expected actual
+'
+
+test_expect_success 'roundtrip' '
+  (cd gitrepo &&
+  git pull &&
+  git log --format="%s" -1 origin/master > actual) &&
+  echo three > expected &&
+  test_cmp expected actual &&
+
+  (cd gitrepo && git push && git pull) &&
+
+  (cd bzrrepo &&
+  echo four > content &&
+  bzr commit -m four
+  ) &&
+
+  (cd gitrepo && git pull && git push) &&
+
+  check gitrepo four master &&
+
+  (cd gitrepo &&
+  echo five > content &&
+  git commit -a -m five &&
+  git push && git pull
+  ) &&
+
+  (cd bzrrepo && bzr revert) &&
+
+  echo five > expected &&
+  cat bzrrepo/content > actual &&
+  test_cmp expected actual
+'
+
+test_done
-- 
1.8.0

Re: [PATCH v3 5/7] remote-bzr: add simple tests

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/remote-helpers/test-bzr.sh | 111 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 111 insertions(+)
>  create mode 100755 contrib/remote-helpers/test-bzr.sh
>
> diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
> new file mode 100755
> index 0000000..8594ffc
> --- /dev/null
> +++ b/contrib/remote-helpers/test-bzr.sh
> @@ -0,0 +1,111 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Felipe Contreras
> +#
> +
> +test_description='Test remote-bzr'
> +
> +. ./test-lib.sh
> +
> +if ! test_have_prereq PYTHON; then
> +	skip_all='skipping remote-bzr tests; python not available'
> +	test_done
> +fi
> +
> +if ! "$PYTHON_PATH" -c 'import bzrlib'; then
> +	skip_all='skipping remote-bzr tests; bzr not available'
> +	test_done
> +fi

> +cmd=<<EOF
> +import bzrlib
> +bzrlib.initialize()
> +import bzrlib.plugin
> +bzrlib.plugin.load_plugins()
> +import bzrlib.plugins.fastimport
> +EOF
> +if ! "$PYTHON_PATH" -c "$cmd"; then

I cannot see how this could have ever worked.  It could be that you
wrote it for zsh and tested only with the version of zsh you have;
zsh I have here does not grok it (and of course dash and bash won't).

In any case, I do not think this is a POSIX shell.  Just replace
"=<<EOF" and "EOF" with single quote and everybody's shell should be
able to read it.

> +	echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
> +	skip_all='skipping remote-bzr tests; bzr-fastimport not available'
> +	test_done
> +fi
> +
> +check () {
> +	(cd $1 &&
> +	git log --format='%s' -1 &&
> +	git symbolic-ref HEAD) > actual &&
> +	(echo $2 &&
> +	echo "refs/heads/$3") > expected &&

Style: s/> expected/>expected/;
I won't repeat this but there are many others in this file.

Thanks.

[PATCH v3 6/7] remote-bzr: add support for fecthing special modes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 38 +++++++++++++++++++++++++----------
 contrib/remote-helpers/test-bzr.sh    | 32 +++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 5b89a05..2bae5d0 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -198,23 +198,39 @@ def export_files(tree, files):
 
     final = []
     for path, fid in files.iteritems():
+        kind = tree.kind(fid)
+
         h = tree.get_file_sha1(fid)
 
-        mode = '100644'
+        if kind == 'symlink':
+            d = tree.get_symlink_target(fid)
+            mode = '120000'
+        elif kind == 'file':
+
+            if tree.is_executable(fid):
+                mode = '100755'
+            else:
+                mode = '100644'
+
+            # is the blog already exported?
+            if h in filenodes:
+                mark = filenodes[h]
+                final.append((mode, mark, path))
+                continue
 
-        # is the blob already exported?
-        if h in filenodes:
-            mark = filenodes[h]
-        else:
             d = tree.get_file_text(fid)
+        elif kind == 'directory':
+            continue
+        else:
+            die("Unhandled kind '%s' for path '%s'" % (kind, path))
 
-            mark = marks.next_mark()
-            filenodes[h] = mark
+        mark = marks.next_mark()
+        filenodes[h] = mark
 
-            print "blob"
-            print "mark :%u" % mark
-            print "data %d" % len(d)
-            print d
+        print "blob"
+        print "mark :%u" % mark
+        print "data %d" % len(d)
+        print d
 
         final.append((mode, mark, path))
 
diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 8594ffc..c92d0c6 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -108,4 +108,36 @@ test_expect_success 'roundtrip' '
   test_cmp expected actual
 '
 
+cat > expected <<EOF
+100644 blob 54f9d6da5c91d556e6b54340b1327573073030af	content
+100755 blob 68769579c3eaadbe555379b9c3538e6628bae1eb	executable
+120000 blob 6b584e8ece562ebffc15d38808cd6b98fc3d97ea	link
+EOF
+
+test_expect_success 'special modes' '
+  (cd bzrrepo &&
+  echo exec > executable
+  chmod +x executable &&
+  bzr add executable
+  bzr commit -m exec &&
+  ln -s content link
+  bzr add link
+  bzr commit -m link &&
+  mkdir dir &&
+  bzr add dir &&
+  bzr commit -m dir) &&
+
+  (cd gitrepo &&
+  git pull
+  git ls-tree HEAD > ../actual) &&
+
+  test_cmp expected actual &&
+
+  (cd gitrepo &&
+  git cat-file -p HEAD:link > ../actual) &&
+
+  echo -n content > expected &&
+  test_cmp expected actual
+'
+
 test_done
-- 
1.8.0

[PATCH v3 7/7] remote-bzr: add support to push special modes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 60 +++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 17 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 2bae5d0..f8919f4 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -370,7 +370,7 @@ class CustomTree():
         self.repo = repo
         self.revid = revid
         self.parents = parents
-        self.updates = files
+        self.updates = {}
 
         def copy_tree(revid):
             files = files_cache[revid] = {}
@@ -394,6 +394,13 @@ class CustomTree():
 
         self.files = files_cache[revid] = self.base_files.copy()
 
+        for path, f in files.iteritems():
+            fid = self.files.get(path, None)
+            if not fid:
+                fid = bzrlib.generate_ids.gen_file_id(path)
+            f['path'] = path
+            self.updates[fid] = f
+
     def last_revision(self):
         return self.base_id
 
@@ -409,13 +416,20 @@ class CustomTree():
                 return parent_fid
             if basename == '':
                 return None
-            d = add_entry(dirname, 'directory')
-            return d[0]
+            fid = bzrlib.generate_ids.gen_file_id(path)
+            d = add_entry(fid, dirname, 'directory')
+            return fid
 
-        def add_entry(path, kind):
+        def add_entry(fid, path, kind, mode = None):
             dirname, basename = os.path.split(path)
             parent_fid = get_parent(dirname, basename)
-            fid = bzrlib.generate_ids.gen_file_id(path)
+
+            executable = False
+            if mode == '100755':
+                executable = True
+            elif mode == '120000':
+                kind = 'symlink'
+
             change = (fid,
                     (None, path),
                     True,
@@ -423,15 +437,21 @@ class CustomTree():
                     (None, parent_fid),
                     (None, basename),
                     (None, kind),
-                    (None, False))
+                    (None, executable))
             self.files[path] = change[0]
             changes.append(change)
             return change
 
-        def update_entry(path, kind):
+        def update_entry(fid, path, kind, mode = None):
             dirname, basename = os.path.split(path)
-            fid = self.base_files[path]
             parent_fid = get_parent(dirname, basename)
+
+            executable = False
+            if mode == '100755':
+                executable = True
+            elif mode == '120000':
+                kind = 'symlink'
+
             change = (fid,
                     (path, path),
                     True,
@@ -439,14 +459,13 @@ class CustomTree():
                     (None, parent_fid),
                     (None, basename),
                     (None, kind),
-                    (None, False))
+                    (None, executable))
             self.files[path] = change[0]
             changes.append(change)
             return change
 
-        def remove_entry(path, kind):
+        def remove_entry(fid, path, kind):
             dirname, basename = os.path.split(path)
-            fid = self.base_files[path]
             parent_fid = get_parent(dirname, basename)
             change = (fid,
                     (path, None),
@@ -460,18 +479,25 @@ class CustomTree():
             changes.append(change)
             return change
 
-        for path, f in self.updates.iteritems():
+        for fid, f in self.updates.iteritems():
+            path = f['path']
+
             if 'deleted' in f:
-                remove_entry(path, 'file')
-            elif path in self.base_files:
-                update_entry(path, 'file')
+                remove_entry(fid, path, 'file')
+                continue
+
+            if path in self.base_files:
+                update_entry(fid, path, 'file', f['mode'])
             else:
-                add_entry(path, 'file')
+                add_entry(fid, path, 'file', f['mode'])
 
         return changes
 
     def get_file_with_stat(self, file_id, path=None):
-        return (StringIO.StringIO(self.updates[path]['data']), None)
+        return (StringIO.StringIO(self.updates[file_id]['data']), None)
+
+    def get_symlink_target(self, file_id):
+        return self.updates[file_id]['data']
 
 def parse_commit(parser):
     global marks, blob_marks, bmarks, parsed_refs
-- 
1.8.0

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

On Sun, Nov 11, 2012 at 3:19 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> This is a re-roll of the previous series to add support to fetch and push
> special modes, and refactor some related code.

It seems this one got forgotten, I only see v2 in pu.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Sun, Nov 11, 2012 at 3:19 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> This is a re-roll of the previous series to add support to fetch and push
>> special modes, and refactor some related code.
>
> It seems this one got forgotten, I only see v2 in pu.

Oops; I think that was fell through the cracks during the maintainer
hand-off.  As the previous one has already been cooking in 'next'
for a week or so, I would appreciate if you send incremental updates
to fix or enhance what is in there.

Thanks.

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

On Mon, Nov 26, 2012 at 5:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Sun, Nov 11, 2012 at 3:19 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> This is a re-roll of the previous series to add support to fetch and push
>>> special modes, and refactor some related code.
>>
>> It seems this one got forgotten, I only see v2 in pu.
>
> Oops; I think that was fell through the cracks during the maintainer
> hand-off.  As the previous one has already been cooking in 'next'
> for a week or so, I would appreciate if you send incremental updates
> to fix or enhance what is in there.

Yes, that's what I have planned for the next patches, as I already did
for remote-hg, but the changes in remote-bzr were a bit bigger.

-- 
Felipe Contreras

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Mon, Nov 26, 2012 at 5:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> On Sun, Nov 11, 2012 at 3:19 PM, Felipe Contreras
>>> <felipe.contreras@gmail.com> wrote:
>>>> This is a re-roll of the previous series to add support to fetch and push
>>>> special modes, and refactor some related code.
>>>
>>> It seems this one got forgotten, I only see v2 in pu.
>>
>> Oops; I think that was fell through the cracks during the maintainer
>> hand-off.  As the previous one has already been cooking in 'next'
>> for a week or so, I would appreciate if you send incremental updates
>> to fix or enhance what is in there.
>
> Yes, that's what I have planned for the next patches, as I already did
> for remote-hg, but the changes in remote-bzr were a bit bigger.

OK.  Both fc/remote-hg and fc/remote-bzr are slated for 'master'
soonish, but I take the above to mean that fc/remote-hg is ready
while it is better to wait for updates to fc/remote-bzr before
merging it.

Thanks.

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

On Wed, Nov 28, 2012 at 12:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Mon, Nov 26, 2012 at 5:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> On Sun, Nov 11, 2012 at 3:19 PM, Felipe Contreras
>>>> <felipe.contreras@gmail.com> wrote:
>>>>> This is a re-roll of the previous series to add support to fetch and push
>>>>> special modes, and refactor some related code.
>>>>
>>>> It seems this one got forgotten, I only see v2 in pu.
>>>
>>> Oops; I think that was fell through the cracks during the maintainer
>>> hand-off.  As the previous one has already been cooking in 'next'
>>> for a week or so, I would appreciate if you send incremental updates
>>> to fix or enhance what is in there.
>>
>> Yes, that's what I have planned for the next patches, as I already did
>> for remote-hg, but the changes in remote-bzr were a bit bigger.
>
> OK.  Both fc/remote-hg and fc/remote-bzr are slated for 'master'
> soonish, but I take the above to mean that fc/remote-hg is ready
> while it is better to wait for updates to fc/remote-bzr before
> merging it.

I was waiting on both to be merged, let me see what I have pending and
would be nice to have before the merge.


-- 
Felipe Contreras

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> OK.  Both fc/remote-hg and fc/remote-bzr are slated for 'master'
>> soonish, but I take the above to mean that fc/remote-hg is ready
>> while it is better to wait for updates to fc/remote-bzr before
>> merging it.
>
> I was waiting on both to be merged, let me see what I have pending and
> would be nice to have before the merge.

OK.

At this point, both have been cooking for a week or more in 'next',
there is no existing users, they are on the fringe so breakages in
them won't negatively affect anybody anyway.  So it doesn't matter
much if they are merged to 'master' and then fixed up with follow up
patches after that, or fixed up with follow up patches while they
are in 'next', as they won't be rewound and restarted from scratch.

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

On Wed, Nov 28, 2012 at 2:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> OK.  Both fc/remote-hg and fc/remote-bzr are slated for 'master'
>>> soonish, but I take the above to mean that fc/remote-hg is ready
>>> while it is better to wait for updates to fc/remote-bzr before
>>> merging it.
>>
>> I was waiting on both to be merged, let me see what I have pending and
>> would be nice to have before the merge.
>
> OK.
>
> At this point, both have been cooking for a week or more in 'next',
> there is no existing users, they are on the fringe so breakages in
> them won't negatively affect anybody anyway.  So it doesn't matter
> much if they are merged to 'master' and then fixed up with follow up
> patches after that, or fixed up with follow up patches while they
> are in 'next', as they won't be rewound and restarted from scratch.

The fixes are affecting some people, that's why I did them. Some were
reported here in the mailing list, and some in my github's clone:

https://github.com/felipec/git/issues?page=1&state=closed

I tried to minimize the changes in the last patch series I sent, I
have more, but those truly can wait.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> At this point, both have been cooking for a week or more in 'next',
>> there is no existing users, they are on the fringe so breakages in
>> them won't negatively affect anybody anyway.  So it doesn't matter
>> much if they are merged to 'master' and then fixed up with follow up
>> patches after that, or fixed up with follow up patches while they
>> are in 'next', as they won't be rewound and restarted from scratch.
>
> The fixes are affecting some people, that's why I did them. Some were
> reported here in the mailing list, and some in my github's clone:
>
> https://github.com/felipec/git/issues?page=1&state=closed

Are you talking about -hg or -bzr or both?

In any case, I am mostly concerned about *my* next release, whose
rc0 will be tagged sometime this week or the next week.

People who have been bitten by bugs from *your* tree or versions in
'next' do not count.  When I said "no existing users", I was talking
about the end users who need rock solid stable "releases" because
tagged versions are the only ones they use.

If the code of these topics is still in flux and needs constant
fixes, probably it is a better idea to cook them longer in 'next',
skipping the upcoming 1.8.1 release.  If we are going to go that
route, we can drop the v2 fc/remote-bzr and queue v3 when we rewind
the tip of 'next' after 1.8.1 release (by that time you may have v4
of the series, but then we can skip v3).  Is that more preferrable
than rushing these topics forward before they are ready for general
audience?

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

On Wed, Nov 28, 2012 at 3:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> At this point, both have been cooking for a week or more in 'next',
>>> there is no existing users, they are on the fringe so breakages in
>>> them won't negatively affect anybody anyway.  So it doesn't matter
>>> much if they are merged to 'master' and then fixed up with follow up
>>> patches after that, or fixed up with follow up patches while they
>>> are in 'next', as they won't be rewound and restarted from scratch.
>>
>> The fixes are affecting some people, that's why I did them. Some were
>> reported here in the mailing list, and some in my github's clone:
>>
>> https://github.com/felipec/git/issues?page=1&state=closed
>
> Are you talking about -hg or -bzr or both?
>
> In any case, I am mostly concerned about *my* next release, whose
> rc0 will be tagged sometime this week or the next week.
>
> People who have been bitten by bugs from *your* tree or versions in
> 'next' do not count.  When I said "no existing users", I was talking
> about the end users who need rock solid stable "releases" because
> tagged versions are the only ones they use.

If users you call "fringe" have noticed these compatibility issues,
chances are your "existing users" are going to catch them as well.

Those issues were fixed right away, but I didn't sent them because I
wanted things to settle. I didn't see that v2 landed in next until
now.

> If the code of these topics is still in flux and needs constant
> fixes, probably it is a better idea to cook them longer in 'next',
> skipping the upcoming 1.8.1 release.  If we are going to go that
> route, we can drop the v2 fc/remote-bzr and queue v3 when we rewind
> the tip of 'next' after 1.8.1 release (by that time you may have v4
> of the series, but then we can skip v3).  Is that more preferrable
> than rushing these topics forward before they are ready for general
> audience?

They are not in constant flux, that's why I haven't send any new
re-rolls since v3, which was on November 11. I've been using v3 for
baseline since them, and the rest of the patches I've sent on top of
that.

In fact, these particular fixes were already sent on November 13 (on top of v3):
http://article.gmane.org/gmane.comp.version-control.git/209558

On November 10 Jeff threatened to to merge v2 to next on the "What's
cooking", and I told him I was about to sent a re-roll, he
acknowledged the same day, and I sent the next one.

Since v3 remote-bzr hasn't been on flux.

Now, what you do is up to you, but I think v3 plus the two patches I
sent on nov 13, and just resent today should be safe. That being said,
I don't use remote-bzr really, and I don't know how many people have
been using it, so I have no idea how ready it really is. If I were you
I would just merge v3 to next, or revert v2 and merge v3, and then
apply the two patches on top. Or if you want, revert v2, wait for
1.8.1, and then merge v3. Either way it's doubtful there will be a v4
(if there are next patches they will be on top of v3, as they have
been for quite some time now), so it's not clear what "existing users"
will gain by that.

I'm confident about remote-hg though.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> People who have been bitten by bugs from *your* tree or versions in
>> 'next' do not count.  When I said "no existing users", I was talking
>> about the end users who need rock solid stable "releases" because
>> tagged versions are the only ones they use.
>
> If users you call "fringe" have noticed these compatibility issues,
> chances are your "existing users" are going to catch them as well.

There seems to be some misunderstanding.

I have never called them "fringe"; they are "early adopters" who are
expected to be capable of "git pull" to pick up fixes from
between-releases trees (or "git am" patches from the list) and
rebuild their Git.  

We cannot expect that from the real end users (who do not exist yet,
luckily) who only follow tagged releases.  Hitting them with bugs we
need to fix after the release is not "letting them notice and
report", but just "irresponsibly hurting the end users".  "Letting
them notice and report" is what "early adopter" population who run
'next' are for.  Quality expectations between these two populations
are quite different.

> ... That being said,
> I don't use remote-bzr really, and I don't know how many people have
> been using it, so I have no idea how ready it really is. ...
> ... Either way it's doubtful there will be a v4

OK; thanks for clarification.  If you are not using it actively, it
probably is a better idea to proceed with more caution, as low rate
of update necessity does not directly relate to maturity of the
tool.  I'd feel better to cook it longer in 'next' to recruit early
adopters so that we can hear positive feedbacks (or negative ones
that can result in fixes to whatever is still uncovered, if any).

I hate reverting anything from 'next', but for the above to work
smoothly, it seems to me that reverting the merge of v2 and queuing
v3 plus remainder sounds like the best course.

> I'm confident about remote-hg though.

Meaning unlike remote-bzr, at least you are using it more actively
(or you know people who are), right?  I've queued the two patches
(out of four) from you today, so we can merge it to 'master' before
1.8.1-rc0.

Thanks.

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

On Wed, Nov 28, 2012 at 8:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> People who have been bitten by bugs from *your* tree or versions in
>>> 'next' do not count.  When I said "no existing users", I was talking
>>> about the end users who need rock solid stable "releases" because
>>> tagged versions are the only ones they use.
>>
>> If users you call "fringe" have noticed these compatibility issues,
>> chances are your "existing users" are going to catch them as well.
>
> There seems to be some misunderstanding.
>
> I have never called them "fringe"; they are "early adopters" who are
> expected to be capable of "git pull" to pick up fixes from
> between-releases trees (or "git am" patches from the list) and
> rebuild their Git.
>
> We cannot expect that from the real end users (who do not exist yet,
> luckily) who only follow tagged releases.  Hitting them with bugs we
> need to fix after the release is not "letting them notice and
> report", but just "irresponsibly hurting the end users".  "Letting
> them notice and report" is what "early adopter" population who run
> 'next' are for.  Quality expectations between these two populations
> are quite different.

Perhaps, but I still don't agree with the statement that the people
bitten by those bugs don't count. If those bugs are not fixed, they
will bite the "normal" population.

>> ... That being said,
>> I don't use remote-bzr really, and I don't know how many people have
>> been using it, so I have no idea how ready it really is. ...
>> ... Either way it's doubtful there will be a v4
>
> OK; thanks for clarification.  If you are not using it actively, it
> probably is a better idea to proceed with more caution, as low rate
> of update necessity does not directly relate to maturity of the
> tool.  I'd feel better to cook it longer in 'next' to recruit early
> adopters so that we can hear positive feedbacks (or negative ones
> that can result in fixes to whatever is still uncovered, if any).

Perhaps, on the other hand it's not like any of their existing
functionality will break. Chances are they will be happy to have
anything that somehow works, even if it has bugs. In fact, the current
remote-bzr, even if it hasn't received so much testing, is probably
already more stable than other tools:

As an example there is this bug:
http://bugs.launchpad.net/bzr/+bug/541626

Which has been affecting users of 'bzr fast-export' for years (most of
bzr<->git bridges use this tool), and nobody does anything about it,
even though the developers thought the problem was in bzr itself (and
that it was actually fixed, despite the reports to the contrary). I
fixed the problem and added a reliable test case to reproduce the
issue in bzr's test suite itself, only to receive no feedback.

I think there are very few users of bazaar, and if you read the source
code you would be happy that's the case, so I'm not sure letting it
cook in 'next' is going to achieve much, most of them are going to see
it only when it hits master. Most likely the few bazaar users are
accustomed to different quality standards, and this tool doesn't even
have known bugs.

>> I'm confident about remote-hg though.
>
> Meaning unlike remote-bzr, at least you are using it more actively
> (or you know people who are), right?  I've queued the two patches
> (out of four) from you today, so we can merge it to 'master' before
> 1.8.1-rc0.

I'm not using it actively, it seems some people are, but that's not
why I'm confident; it's because of the extensive automatic testing
that compares the output with hg-git (which is widely used).

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 0/7] New remote-bzr remote helper

[Felipe Contreras]

On Wed, Nov 28, 2012 at 12:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Mon, Nov 26, 2012 at 5:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> On Sun, Nov 11, 2012 at 3:19 PM, Felipe Contreras
>>>> <felipe.contreras@gmail.com> wrote:
>>>>> This is a re-roll of the previous series to add support to fetch and push
>>>>> special modes, and refactor some related code.
>>>>
>>>> It seems this one got forgotten, I only see v2 in pu.
>>>
>>> Oops; I think that was fell through the cracks during the maintainer
>>> hand-off.  As the previous one has already been cooking in 'next'
>>> for a week or so, I would appreciate if you send incremental updates
>>> to fix or enhance what is in there.
>>
>> Yes, that's what I have planned for the next patches, as I already did
>> for remote-hg, but the changes in remote-bzr were a bit bigger.
>
> OK.  Both fc/remote-hg and fc/remote-bzr are slated for 'master'
> soonish, but I take the above to mean that fc/remote-hg is ready
> while it is better to wait for updates to fc/remote-bzr before
> merging it.

Please update remote-bzr to series version 3. The rest of the patches
will be on top of that version.

-- 
Felipe Contreras