# [PATCH v2 0/9] remote-helpers: fixes and cleanups 13 messages from 2013-04-26 to 2013-04-26. Participants: Felipe Contreras, John Szakmeister. Thread: https://gitlist.dev/t/33611 ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 0/9] remote-helpers: fixes and cleanups Message-ID: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-1-git-send-email-felipe.contreras%40gmail.com ``` Hi, Same as before, minus one fix. This time they are based on 'next' so there shouldn't be any conflicts. Here's a bunch of cleanups mostly to synchronize remote-bzr and remote-hg. Felipe Contreras (9): remote-helpers: trivial cleanups remote-hg: remove extra check remote-bzr: fix bad state issue remote-bzr: add support to push URLs remote-hg: use hashlib instead of hg sha1 util remote-bzr: store converted URL remote-bzr: tell bazaar to be quiet remote-bzr: strip extra newline remote-bzr: use proper push method contrib/remote-helpers/git-remote-bzr | 55 ++++++++++++++++++++++++++++------- contrib/remote-helpers/git-remote-hg | 10 ++----- 2 files changed, 48 insertions(+), 17 deletions(-) -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 1/9] remote-helpers: trivial cleanups Message-ID: <1366934902-18704-2-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-2-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` Signed-off-by: Felipe Contreras --- Do I have to say it? If code is removed, it's because it's not used. contrib/remote-helpers/git-remote-bzr | 7 ++++--- contrib/remote-helpers/git-remote-hg | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index cc6609b..87c54bf 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -94,7 +94,7 @@ class Marks: return self.last_mark def is_marked(self, rev): - return str(rev) in self.marks + return rev in self.marks def new_mark(self, rev, mark): self.marks[rev] = mark @@ -224,7 +224,7 @@ def export_files(tree, files): else: mode = '100644' - # is the blog already exported? + # is the blob already exported? if h in filenodes: mark = filenodes[h] final.append((mode, mark, path)) @@ -521,7 +521,7 @@ def c_style_unescape(string): return string def parse_commit(parser): - global marks, blob_marks, bmarks, parsed_refs + global marks, blob_marks, parsed_refs global mode parents = [] @@ -643,6 +643,7 @@ def do_export(parser): wt = repo.bzrdir.open_workingtree() wt.update() print "ok %s" % ref + print def do_capabilities(parser): diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index fda4199..cfa96c1 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -12,7 +12,7 @@ # For remote repositories a local clone is stored in # "$GIT_DIR/hg/origin/clone/.hg/". -from mercurial import hg, ui, bookmarks, context, util, encoding, node, error, extensions +from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions import re import sys -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 2/9] remote-hg: remove extra check Message-ID: <1366934902-18704-3-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-3-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` Not needed since we use xrange ourselves. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-hg | 4 ---- 1 file changed, 4 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index cfa96c1..80b3606 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -373,10 +373,6 @@ def export_ref(repo, name, kind, head): ename = '%s/%s' % (kind, name) tip = marks.get_tip(ename) - # mercurial takes too much time checking this - if tip and tip == head.rev(): - # nothing to do - return revs = xrange(tip, head.rev() + 1) count = 0 -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 3/9] remote-bzr: fix bad state issue Message-ID: <1366934902-18704-4-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-4-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` Carried from remote-hg. The problem reportedly happened after doing a push that fails, the abort causes the state of remote-hg to go bad, this happens because remote-hg's marks are not stored, but 'git fast-export' marks are. Ensure that the marks are _always_ stored. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 87c54bf..7b6584e 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -32,6 +32,7 @@ import os import json import re import StringIO +import atexit NAME_RE = re.compile('^([^<>]+)') AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$') @@ -728,6 +729,7 @@ def main(args): blob_marks = {} parsed_refs = {} files_cache = {} + marks = None gitdir = os.environ['GIT_DIR'] dirname = os.path.join(gitdir, 'bzr', alias) @@ -754,6 +756,10 @@ def main(args): die('unhandled command: %s' % line) sys.stdout.flush() +def bye(): + if not marks: + return marks.store() +atexit.register(bye) sys.exit(main(sys.argv)) -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 4/9] remote-bzr: add support to push URLs Message-ID: <1366934902-18704-5-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-5-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` Just like in remote-hg. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 7b6584e..f1d6d5e 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -32,7 +32,7 @@ import os import json import re import StringIO -import atexit +import atexit, shutil, hashlib NAME_RE = re.compile('^([^<>]+)') AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$') @@ -719,11 +719,11 @@ def main(args): global blob_marks global parsed_refs global files_cache + global is_tmp alias = args[1] url = args[2] - prefix = 'refs/bzr/%s' % alias tags = {} filenodes = {} blob_marks = {} @@ -731,6 +731,13 @@ def main(args): files_cache = {} marks = None + if alias[5:] == url: + is_tmp = True + alias = hashlib.sha1(alias).hexdigest() + else: + is_tmp = False + + prefix = 'refs/bzr/%s' % alias gitdir = os.environ['GIT_DIR'] dirname = os.path.join(gitdir, 'bzr', alias) @@ -759,7 +766,10 @@ def main(args): def bye(): if not marks: return - marks.store() + if not is_tmp: + marks.store() + else: + shutil.rmtree(dirname) atexit.register(bye) sys.exit(main(sys.argv)) -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 5/9] remote-hg: use hashlib instead of hg sha1 util Message-ID: <1366934902-18704-6-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-6-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` To be in sync with remote-bzr. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-hg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 80b3606..06920f2 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -22,7 +22,7 @@ import shutil import subprocess import urllib import atexit -import urlparse +import urlparse, hashlib # # If you want to switch to hg-git compatibility mode: @@ -933,7 +933,7 @@ def main(args): if alias[4:] == url: is_tmp = True - alias = util.sha1(alias).hexdigest() + alias = hashlib.sha1(alias).hexdigest() else: is_tmp = False -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 6/9] remote-bzr: store converted URL Message-ID: <1366934902-18704-7-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-7-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` Mercurial might convert the URL to something more appropriate, like an absolute path. Lets store that instead of the original URL, which won't work from a different working directory if it's relative. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index f1d6d5e..dda2932 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -32,7 +32,7 @@ import os import json import re import StringIO -import atexit, shutil, hashlib +import atexit, shutil, hashlib, urlparse, subprocess NAME_RE = re.compile('^([^<>]+)') AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$') @@ -713,6 +713,14 @@ def get_repo(url, alias): return branch +def fix_path(alias, orig_url): + url = urlparse.urlparse(orig_url, 'file') + if url.scheme != 'file' or os.path.isabs(url.path): + return + abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url) + cmd = ['git', 'config', 'remote.%s.url' % alias, "bzr::%s" % abs_url] + subprocess.call(cmd) + def main(args): global marks, prefix, dirname global tags, filenodes @@ -741,6 +749,9 @@ def main(args): gitdir = os.environ['GIT_DIR'] dirname = os.path.join(gitdir, 'bzr', alias) + if not is_tmp: + fix_path(alias, url) + if not os.path.exists(dirname): os.makedirs(dirname) -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 7/9] remote-bzr: tell bazaar to be quiet Message-ID: <1366934902-18704-8-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-8-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` Otherwise we get notification, progress bars, and what not. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index dda2932..8617e25 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -26,6 +26,7 @@ bzrlib.plugin.load_plugins() import bzrlib.generate_ids import bzrlib.transport import bzrlib.errors +import bzrlib.ui import sys import os @@ -755,6 +756,8 @@ def main(args): if not os.path.exists(dirname): os.makedirs(dirname) + bzrlib.ui.ui_factory.be_quiet(True) + repo = get_repo(url, alias) marks_path = os.path.join(dirname, 'marks-int') -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 8/9] remote-bzr: strip extra newline Message-ID: <1366934902-18704-9-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-9-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` It's added by fast-export, the user didn't type it. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 8617e25..9f56297 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -549,6 +549,10 @@ def parse_commit(parser): parents.append(parser.get_mark()) parser.next() + # fast-export adds an extra newline + if data[-1] == '\n': + data = data[:-1] + files = {} for line in parser: -- 1.8.2.1.884.g3532a8d ``` ## Felipe Contreras, 2013-04-26 00:08 Subject: [PATCH v2 9/9] remote-bzr: use proper push method Message-ID: <1366934902-18704-10-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1366934902-18704-10-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1366934902-18704-1-git-send-email-felipe.contreras@gmail.com> ``` Do not just randomly synchronize the revisions with no checks at all. I don't have any evidence that there's anything wrong with the current code, which Bazaar seems to use, but for different purposes. Let's use the logic Bazaar UI uses to avoid surprises. Also, add a non-ff check. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 9f56297..c19ed0e 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -639,12 +639,12 @@ 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() if peer: - if hasattr(peer, "import_last_revision_info_and_tags"): - peer.import_last_revision_info_and_tags(repo, revno, revid) - else: - peer.import_last_revision_info(repo.repository, revno, revid) + try: + repo.push(peer, stop_revision=revid) + except bzrlib.errors.DivergedBranches: + print "error %s non-fast forward" % ref + continue else: wt = repo.bzrdir.open_workingtree() wt.update() -- 1.8.2.1.884.g3532a8d ``` ## John Szakmeister, 2013-04-26 00:58 Subject: Re: [PATCH v2 6/9] remote-bzr: store converted URL Message-ID: URL: https://gitlist.dev/e/CAEBDL5XmrxV3SdtdyjJq%3DHxUqVcLCMz_Fcq9%3DPOm%3DoaWtrO4LQ%40mail.gmail.com In-Reply-To: <1366934902-18704-7-git-send-email-felipe.contreras@gmail.com> ``` On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras wrote: > Mercurial might convert the URL to something more appropriate, like an > absolute path. Lets store that instead of the original URL, which won't > work from a different working directory if it's relative. > > Signed-off-by: Felipe Contreras > --- > contrib/remote-helpers/git-remote-bzr | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) FWIW, it feels weird to be talking about Mercurial when you're patching git-remote-bzr. :-) -John ``` ## John Szakmeister, 2013-04-26 00:59 Subject: Re: [PATCH v2 7/9] remote-bzr: tell bazaar to be quiet Message-ID: URL: https://gitlist.dev/e/CAEBDL5Uaw6t%3DT1NOhRDutmH_V0dax3Pzger9Ti34Dy4tPT3K0A%40mail.gmail.com In-Reply-To: <1366934902-18704-8-git-send-email-felipe.contreras@gmail.com> ``` On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras wrote: > Otherwise we get notification, progress bars, and what not. > > Signed-off-by: Felipe Contreras > --- > contrib/remote-helpers/git-remote-bzr | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr > index dda2932..8617e25 100755 > --- a/contrib/remote-helpers/git-remote-bzr > +++ b/contrib/remote-helpers/git-remote-bzr > @@ -26,6 +26,7 @@ bzrlib.plugin.load_plugins() > import bzrlib.generate_ids > import bzrlib.transport > import bzrlib.errors > +import bzrlib.ui > > import sys > import os > @@ -755,6 +756,8 @@ def main(args): > if not os.path.exists(dirname): > os.makedirs(dirname) > > + bzrlib.ui.ui_factory.be_quiet(True) > + Nice change! -John ``` ## Felipe Contreras, 2013-04-26 01:12 Subject: Re: [PATCH v2 6/9] remote-bzr: store converted URL Message-ID: URL: https://gitlist.dev/e/CAMP44s0csoD7y-YND-VyVWuZc%2BGxK7JPf5DLibXZxnFHNakjDw%40mail.gmail.com In-Reply-To: ``` On Thu, Apr 25, 2013 at 7:58 PM, John Szakmeister wrote: > On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras > wrote: >> Mercurial might convert the URL to something more appropriate, like an >> absolute path. Lets store that instead of the original URL, which won't >> work from a different working directory if it's relative. >> >> Signed-off-by: Felipe Contreras >> --- >> contrib/remote-helpers/git-remote-bzr | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) > > > FWIW, it feels weird to be talking about Mercurial when you're > patching git-remote-bzr. :-) Yeah, it happens all the time in my local repos. I'm surprised it took so long to send a patch with this mistake =/ -- Felipe Contreras ```