On Wed, Jun 08, 2011 at 04:21:21PM -0700, Junio C Hamano wrote:
> > diff --git a/git_remote_helpers/git/exporter.py b/git_remote_helpers/git/exporter.py
> > index f40f9d6..1855c6a 100644
> > --- a/git_remote_helpers/git/exporter.py
> > +++ b/git_remote_helpers/git/exporter.py
> > @@ -15,7 +15,7 @@ class GitExporter(object):
> >
> > self.repo = repo
> >
> > - def export_repo(self, base):
> > + def export_repo(self, base, refs = ["HEAD"]):
>
> This seems like an accident waiting to happen, even though it is Ok with
> the current code (because this method does not modify refs in any way), to
> specify a mutable object as the default value for an optional parameter.
Yeah, I didn't realize that python assigns default values by reference,
unlike other languages (this is actually the first python I've ever
written).
At any rate, I think this patch is obsoleted by Sverre's new series
(though I haven't read it in detail yet).
> > sys.stdout.flush()
> >
> > args = ["git", "--git-dir=" + self.repo.gitpath, "fast-export", "--export-marks=" + path]
> > + args.extend(refs)
> >
> > if os.path.exists(path):
> > args.append("--import-marks=" + path)
>
> Hmm, am I looking at the right version of this file?
>
> I see args.append("HEAD") after this --import-marks in the existing code,
> which this patch does not remove.
Ugh, yeah. I missed that "HEAD". So this version is buggy in that it
exports everything you asked for, _plus_ HEAD. Which isn't a problem in
practice, but it's inefficient. That "HEAD" should definitely go away.
I'll check for it in Sverre's series and make sure he did it right. :)
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html