Git development
 help / color / mirror / Atom feed
* Re: missing objects -- prevention
From: Sitaram Chamarty @ 2013-01-13  0:56 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20130112131358.GB21875@sigill.intra.peff.net>

On Sat, Jan 12, 2013 at 6:43 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Jan 12, 2013 at 06:39:52AM +0530, Sitaram Chamarty wrote:
>
>> >   1. The repo has a ref R pointing at commit X.
>> >
>> >   2. A user starts a push to another ref, Q, of commit Y that builds on
>> >      X. Git advertises ref R, so the sender knows they do not need to
>> >      send X, but only Y. The user then proceeds to send the packfile
>> >      (which might take a very long time).
>> >
>> >   3. Meanwhile, another user deletes ref R. X becomes unreferenced.
>>
>> The gitolite logs show that no deletion of refs has happened.
>
> To be pedantic, step 3 could also be rewinding R to a commit before X.
> Anything that causes X to become unreferenced.

Right, but there were no rewinds also; I should have mentioned that.
(Gitolite log files mark rewinds and deletes specially, so they're
easy to search.  There were two attempted rewinds but they failed the
gitolite update hook so -- while the new objects would have landed in
the object store -- the old ones were not dereferenced).

>> > There is a race with simultaneously deleting and packing refs. It
>> > doesn't cause object db corruption, but it will cause refs to "rewind"
>> > back to their packed versions. I have seen that one in practice (though
>> > relatively rare). I fixed it in b3f1280, which is not yet in any
>> > released version.
>>
>> This is for the packed-refs file right?  And it could result in a ref
>> getting deleted right?
>
> Yes, if the ref was not previously packed, it could result in the ref
> being deleted entirely.
>
>> I said above that the gitolite logs say no ref was deleted.  What if
>> the ref "deletion" happened because of this race, making the rest of
>> your 4-step scenario above possible?
>
> It's possible. I do want to highlight how unlikely it is, though.

Agreed.

>> > up in the middle, or fsck rejects the pack). We have historically left
>>
>> fsck... you mean if I had 'receive.fsckObjects' true, right?  I don't.
>>  Should I?  Would it help this overall situation?  As I understand it,
>> thats only about the internals of each object to check corruption, and
>> cannot detect a *missing* object on the local object store.
>
> Right, I meant if you have receive.fsckObjects on. It won't help this
> situation at all, as we already do a connectivity check separate from
> the fsck. But I do recommend it in general, just because it helps catch
> bad objects before they gets disseminated to a wider audience (at which
> point it is often infeasible to rewind history). And it has found git
> bugs (e.g., null sha1s in tree entries).

I will add this.  Any idea if there's a significant performance hit?

>> > At GitHub, we've taken to just cleaning them up aggressively (I think
>> > after an hour), though I am tempted to put in an optional signal/atexit
>>
>> OK; I'll do the same then.  I suppose a cron job is the best way; I
>> didn't find any config for expiring these files.
>
> If you run "git prune --expire=1.hour.ago", it should prune stale
> tmp_pack_* files more than an hour old. But you may not be comfortable
> with such a short expiration for the objects themselves. :)
>
>> Thanks again for your help.  I'm going to treat it (for now) as a
>> disk/fs error after hearing from you about the other possibility I
>> mentioned above, although I find it hard to believe one repo can be
>> hit buy *two* races occurring together!
>
> Yeah, the race seems pretty unlikely (though it could be just the one
> race with a rewind). As I said, I haven't actually ever seen it in
> practice. In my experience, though, disk/fs issues do not manifest as
> just missing objects, but as corrupted packfiles (e.g., the packfile
> directory entry ends up pointing to the wrong inode, which is easy to
> see because the inode's content is actually a reflog). And then of
> course with the packfile unreadable, you have missing objects. But YMMV,
> depending on the fs and what's happened to the machine to cause the fs
> problem.

That's always the hard part.  System admins (at the Unix level) insist
there's nothing wrong and no disk errors and so on...  that is why I
was interested in network errors causing problems and so on.

Anyway, now that I know the tmp_pack_* files are caused mostly by
failed pushes than by failed auto-gc, at least I can deal with the
immediate problem easily!

Thanks once again for your patient replies!

sitaram


-- 
Sitaram

^ permalink raw reply

* Re: [PATCH 0/8] Initial support for Python 3
From: John Keeping @ 2013-01-13  0:41 UTC (permalink / raw)
  To: Pete Wyckoff
  Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier,
	Sebastian Morr
In-Reply-To: <20130112234304.GC23079@padd.com>

On Sat, Jan 12, 2013 at 06:43:04PM -0500, Pete Wyckoff wrote:
> john@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +0000:
>> I started having a look to see how much work would be needed to make Git
>> work with Python 3 and the answer is mostly not much.  The exception is
>> git-p4.py which is hit hard by the distinction between byte strings and
>> unicode strings, particularly because the Python output mode of p4
>> targets Python 2.
>> 
>> I don't know if it's worthwhile to actually apply these but here they
>> are in case anyone's interested.
>> 
>> Having said that, the changes are minimal and involve either wrapping
>> parentheses around arguments to print or being a bit more explicit about
>> how we expect byte strings to be decoded to unicode.
>> 
>> With these patches all tests pass with python3 except t98* (git-p4), but
>> there are a couple of topics in-flight which will affect that
>> (fc/remote-testgit-feature-done and er/replace-cvsimport).
>> 
>> John Keeping (8):
>>   git_remote_helpers: Allow building with Python 3
>>   git_remote_helpers: fix input when running under Python 3
>>   git_remote_helpers: Force rebuild if python version changes
>>   git_remote_helpers: Use 2to3 if building with Python 3
>>   svn-fe: allow svnrdump_sim.py to run with Python 3
>>   git-remote-testpy: hash bytes explicitly
>>   git-remote-testpy: don't do unbuffered text I/O
>>   git-remote-testpy: call print as a function
>> 
>>  contrib/svn-fe/svnrdump_sim.py     |  4 ++--
>>  git-remote-testpy.py               | 40 +++++++++++++++++++-------------------
>>  git_remote_helpers/.gitignore      |  1 +
>>  git_remote_helpers/Makefile        | 10 ++++++++--
>>  git_remote_helpers/git/importer.py |  2 +-
>>  git_remote_helpers/setup.py        | 10 ++++++++++
>>  6 files changed, 42 insertions(+), 25 deletions(-)
> 
> These look good, in that there are relatively few changed needed.
> 
> Sebastian Morr tried a similar patch a year ago, in
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/187545
> 
> He made changes beyond yours, in particular "print >>" lines,
> that you seem to handle with 2to3 during the build.  I'm not sure
> which approach is better in the long run.  He worked on the
> other .py in contrib/ too.

In the long run I'd want to move away from "print >>" to use
"print(file=..., ...)" but that's only available from Python 2.6 onwards
(via a __future__ import) and I think we probably don't want to rule out
Python 2.5 yet.

Without 2to3 the only way to do this for both Python 2 and 3 is as
"file.write('...\n')".

> Can you give me some hints about the byte/unicode string issues
> in git-p4.py?  There's really only one place that does:
> 
>     p4 = subprocess.Popen("p4 -G ...")
>     marshal.load(p4.stdout)
> 
> If that's the only issue, this might not be too paniful.

The problem is that what gets loaded there is a dictionary (encoded by
p4) that maps byte strings to byte strings, so all of the accesses to
that dictionary need to either:

   1) explicitly call encode() on a string constant
or 2) use a byte string constant with a "b" prefix

Or we could re-write the dictionary once, which handles the keys... but
some of the values are also used as strings and we can't handle that as
a one-off conversion since in other places we really do want the byte
string (think content of binary files).

Basically a thorough audit of all access to variables that come from p4
would be needed, with explicit decode()s for authors, dates, etc.

> I hesitated to take Sebastian's changes due to the huge number of
> print() lines, but maybe a 2to3 approach would make that aspect
> of python3 support not too onerous.

I think we'd want to change to print() eventually and having a single
codebase for 2 and 3 would be nicer for development, but I think we need
to be able to say "no one is using Python 2.5 or earlier" before we can
do that and I'm not sure we're there yet.  From where we are at the
moment I think 2to3 is a good answer, particularly where we're already
using distutils to generate a release image.


John

^ permalink raw reply

* Re: [PATCH 0/8] Initial support for Python 3
From: Pete Wyckoff @ 2013-01-12 23:43 UTC (permalink / raw)
  To: John Keeping
  Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier,
	Sebastian Morr
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

john@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +0000:
> I started having a look to see how much work would be needed to make Git
> work with Python 3 and the answer is mostly not much.  The exception is
> git-p4.py which is hit hard by the distinction between byte strings and
> unicode strings, particularly because the Python output mode of p4
> targets Python 2.
> 
> I don't know if it's worthwhile to actually apply these but here they
> are in case anyone's interested.
> 
> Having said that, the changes are minimal and involve either wrapping
> parentheses around arguments to print or being a bit more explicit about
> how we expect byte strings to be decoded to unicode.
> 
> With these patches all tests pass with python3 except t98* (git-p4), but
> there are a couple of topics in-flight which will affect that
> (fc/remote-testgit-feature-done and er/replace-cvsimport).
> 
> John Keeping (8):
>   git_remote_helpers: Allow building with Python 3
>   git_remote_helpers: fix input when running under Python 3
>   git_remote_helpers: Force rebuild if python version changes
>   git_remote_helpers: Use 2to3 if building with Python 3
>   svn-fe: allow svnrdump_sim.py to run with Python 3
>   git-remote-testpy: hash bytes explicitly
>   git-remote-testpy: don't do unbuffered text I/O
>   git-remote-testpy: call print as a function
> 
>  contrib/svn-fe/svnrdump_sim.py     |  4 ++--
>  git-remote-testpy.py               | 40 +++++++++++++++++++-------------------
>  git_remote_helpers/.gitignore      |  1 +
>  git_remote_helpers/Makefile        | 10 ++++++++--
>  git_remote_helpers/git/importer.py |  2 +-
>  git_remote_helpers/setup.py        | 10 ++++++++++
>  6 files changed, 42 insertions(+), 25 deletions(-)

These look good, in that there are relatively few changed needed.

Sebastian Morr tried a similar patch a year ago, in

    http://thread.gmane.org/gmane.comp.version-control.git/187545

He made changes beyond yours, in particular "print >>" lines,
that you seem to handle with 2to3 during the build.  I'm not sure
which approach is better in the long run.  He worked on the
other .py in contrib/ too.

Can you give me some hints about the byte/unicode string issues
in git-p4.py?  There's really only one place that does:

    p4 = subprocess.Popen("p4 -G ...")
    marshal.load(p4.stdout)

If that's the only issue, this might not be too paniful.

I hesitated to take Sebastian's changes due to the huge number of
print() lines, but maybe a 2to3 approach would make that aspect
of python3 support not too onerous.

		-- Pete

^ permalink raw reply

* Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
From: Pete Wyckoff @ 2013-01-12 23:30 UTC (permalink / raw)
  To: John Keeping; +Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <89f55d20da9a4c0a8490f95107cbf5d04219d0fb.1358018078.git.john@keeping.me.uk>

john@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +0000:
> When different version of python are used to build via distutils, the
> behaviour can change.  Detect changes in version and pass --force in
> this case.
[..]
> diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
[..]
> +py_version=$(shell $(PYTHON_PATH) -c \
> +	'import sys; print("%i.%i" % sys.version_info[:2])')
> +
>  all: $(pysetupfile)
> -	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
> +	$(QUIET)test "$$(cat GIT-PYTHON_VERSION 2>/dev/null)" = "$(py_version)" || \
> +	flags=--force; \
> +	$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
> +	$(QUIET)echo "$(py_version)" >GIT-PYTHON_VERSION

Can you depend on ../GIT-PYTHON-VARS instead?  It comes from
96a4647 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
It doesn't check version, just path, but hopefully that's good
enough.  I'm imagining a rule that would do "clean" if
../GIT-PYTHON-VARS changed, then build without --force.

		-- Pete

^ permalink raw reply

* Re: Suggestion: add option in git-p4 to preserve user in Git repository
From: Pete Wyckoff @ 2013-01-12 22:56 UTC (permalink / raw)
  To: Olivier Delalleau; +Cc: git, Luke Diamand
In-Reply-To: <CAFXk4bpM8X3k=iwRjM9kvm4XbZyKS+hTCiVbHOjH3jK6MkkBSg@mail.gmail.com>

shish@keba.be wrote on Sat, 12 Jan 2013 14:44 -0500:
> 2013/1/12 Pete Wyckoff <pw@padd.com>:
> > shish@keba.be wrote on Thu, 10 Jan 2013 22:38 -0500:
> >> I'm in a situation where I don't have P4 admin rights to use the
> >> --preserve-user option of git-p4. However, I would like to keep user
> >> information in the associated Git branch.
> >>
> >> Would it be possible to add an option for this?
> >
> > The --preserve-user option is used to submit somebody else's work
> > from git to p4.  It does "p4 change -f" to edit the author of the
> > change after it has been submitted to p4.  P4 requires admin
> > privileges to do that.
> >
> > Changes that are imported _from_ p4 to git do have the correct
> > author information.
> >
> > Can you explain a bit more what you're looking for?
> 
> Sorry I wasn't clear enough. When "git p4 submit" submits changes from
> Git to P4, it also edits the Git history and replaces the Git commits'
> authors by the information from the Perforce account submitting the
> changes. The advantage is that both the P4 and Git repositories share
> the same author information, but in my case I would like to keep in
> the Git repository the original authors (because the P4 account I'm
> using to submit to P4 is shared by all Git users).

Ah, I see what you're looking for now.  It's certainly possible
to keep a mapping in the git side to remember who really wrote
each change that went into p4, but there's nothing set up to do
that now.  And it would be a fair amount of work, with many
little details.

You could put the true name in the commit message, like
we do signed-off-by messages: "Author: Real Coder <rc@my.com>".
That would keep the proper attribution, but not work with "git
log --author", e.g.; you'd have to use "--grep='Real Coder'"
instead.

		-- Pete

^ permalink raw reply

* Re: [PATCH v2 05/21] commit: convert to use parse_pathspec
From: Martin von Zweigbergk @ 2013-01-12 22:54 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1357903275-16804-6-git-send-email-pclouds@gmail.com>

On Fri, Jan 11, 2013 at 3:20 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
> diff --git a/cache.h b/cache.h
> index e52365d..a3c316f 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -476,6 +476,9 @@ extern int ie_modified(const struct index_state *, struct cache_entry *, struct
>  /* Pathspec magic */
>  #define PATHSPEC_FROMTOP    (1<<0)
>
> +/* Pathspec flags */
> +#define PATHSPEC_EMPTY_MATCH_ALL (1<<0) /* No args means match everything */
> +
>  struct pathspec {
>         const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
>         int nr;
> diff --git a/setup.c b/setup.c
> index 6e960b9..a26b6c0 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -280,6 +280,9 @@ void parse_pathspec(struct pathspec *pathspec,
>         if (!entry && !prefix)
>                 return;
>
> +       if (!*argv && (flags & PATHSPEC_EMPTY_MATCH_ALL))
> +               return;
> +
>         /* No arguments with prefix -> prefix pathspec */
>         if (!entry) {
>                 static const char *raw[2];

I was surprised not to find these two hunks in 02/21. If they were
there, you wouldn't have to explain in the log message of that patch
that "flags" is for future-proofing. Also, "*argv" is written "entry"
in the surrounding conditions.

^ permalink raw reply

* [PATCH] rebase --preserve-merges keeps empty merge commits
From: Phil Hord @ 2013-01-12 20:46 UTC (permalink / raw)
  To: git
  Cc: phil.hord, Neil Horman, Martin von Zweigbergk, Junio C Hamano,
	Phil Hord

Since 90e1818f9a  (git-rebase: add keep_empty flag, 2012-04-20)
'git rebase --preserve-merges' fails to preserve empty merge commits
unless --keep-empty is also specified.  Merge commits should be
preserved in order to preserve the structure of the rebased graph,
even if the merge commit does not introduce changes to the parent.

Teach rebase not to drop merge commits only because they are empty.

A special case which is not handled by this change is for a merge commit
whose parents are now the same commit because all the previous different
parents have been dropped as a result of this rebase or some previous
operation.
---
 git-rebase--interactive.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 44901d5..8ed7fcc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -190,6 +190,11 @@ is_empty_commit() {
 	test "$tree" = "$ptree"
 }
 
+is_merge_commit()
+{
+	git rev-parse --verify --quiet "$1"^2 >/dev/null 2>&1
+}
+
 # Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
 # GIT_AUTHOR_DATE exported from the current environment.
 do_with_author () {
@@ -874,7 +879,7 @@ git rev-list $merges_option --pretty=oneline --abbrev-commit \
 while read -r shortsha1 rest
 do
 
-	if test -z "$keep_empty" && is_empty_commit $shortsha1
+	if test -z "$keep_empty" && is_empty_commit $shortsha1 && ! is_merge_commit $shortsha1
 	then
 		comment_out="# "
 	else
-- 
1.8.1.dirty

^ permalink raw reply related

* Re: Suggestion: add option in git-p4 to preserve user in Git repository
From: Olivier Delalleau @ 2013-01-12 19:44 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Luke Diamand
In-Reply-To: <20130112163838.GA28722@padd.com>

2013/1/12 Pete Wyckoff <pw@padd.com>:
> shish@keba.be wrote on Thu, 10 Jan 2013 22:38 -0500:
>> I'm in a situation where I don't have P4 admin rights to use the
>> --preserve-user option of git-p4. However, I would like to keep user
>> information in the associated Git branch.
>>
>> Would it be possible to add an option for this?
>
> The --preserve-user option is used to submit somebody else's work
> from git to p4.  It does "p4 change -f" to edit the author of the
> change after it has been submitted to p4.  P4 requires admin
> privileges to do that.
>
> Changes that are imported _from_ p4 to git do have the correct
> author information.
>
> Can you explain a bit more what you're looking for?
>
>                 -- Pete

Hi,

Sorry I wasn't clear enough. When "git p4 submit" submits changes from
Git to P4, it also edits the Git history and replaces the Git commits'
authors by the information from the Perforce account submitting the
changes. The advantage is that both the P4 and Git repositories share
the same author information, but in my case I would like to keep in
the Git repository the original authors (because the P4 account I'm
using to submit to P4 is shared by all Git users).

Hope it makes more sense now :)

-=- Olivier

^ permalink raw reply

* [PATCH 8/8] git-remote-testpy: call print as a function
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

This is harmless in Python 2, which sees the parentheses as redundant
grouping, but is required for Python 3.  Since this is the only change
required to make this script just run under Python 3 without needing
2to3 it seems worthwhile.

The case of an empty print must be handled specially because in that
case Python 2 will interpret '()' as an empty tuple and print it as
'()'; inserting an empty string fixes this.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git-remote-testpy.py | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index 815222f..8ba5d28 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -87,9 +87,9 @@ def do_capabilities(repo, args):
     """Prints the supported capabilities.
     """
 
-    print "import"
-    print "export"
-    print "refspec refs/heads/*:%s*" % repo.prefix
+    print("import")
+    print("export")
+    print("refspec refs/heads/*:%s*" % repo.prefix)
 
     dirname = repo.get_base_path(repo.gitdir)
 
@@ -98,11 +98,11 @@ def do_capabilities(repo, args):
 
     path = os.path.join(dirname, 'git.marks')
 
-    print "*export-marks %s" % path
+    print("*export-marks %s" % path)
     if os.path.exists(path):
-        print "*import-marks %s" % path
+        print("*import-marks %s" % path)
 
-    print # end capabilities
+    print('') # end capabilities
 
 
 def do_list(repo, args):
@@ -115,16 +115,16 @@ def do_list(repo, args):
 
     for ref in repo.revs:
         debug("? refs/heads/%s", ref)
-        print "? refs/heads/%s" % ref
+        print("? refs/heads/%s" % ref)
 
     if repo.head:
         debug("@refs/heads/%s HEAD" % repo.head)
-        print "@refs/heads/%s HEAD" % repo.head
+        print("@refs/heads/%s HEAD" % repo.head)
     else:
         debug("@refs/heads/master HEAD")
-        print "@refs/heads/master HEAD"
+        print("@refs/heads/master HEAD")
 
-    print # end list
+    print('') # end list
 
 
 def update_local_repo(repo):
@@ -167,7 +167,7 @@ def do_import(repo, args):
     repo = update_local_repo(repo)
     repo.exporter.export_repo(repo.gitdir, refs)
 
-    print "done"
+    print("done")
 
 
 def do_export(repo, args):
@@ -184,8 +184,8 @@ def do_export(repo, args):
         repo.non_local.push(repo.gitdir)
 
     for ref in changed:
-        print "ok %s" % ref
-    print
+        print("ok %s" % ref)
+    print('')
 
 
 COMMANDS = {
-- 
1.8.1

^ permalink raw reply related

* [PATCH 7/8] git-remote-testpy: don't do unbuffered text I/O
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

Python 3 forbids unbuffered I/O in text mode.  Change the reading of
stdin in git-remote-testpy so that we read the lines as bytes and then
decode them a line at a time.

This allows us to keep the I/O unbuffered in order to avoid
reintroducing the bug fixed by commit 7fb8e16 (git-remote-testgit: fix
race when spawning fast-import).

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git-remote-testpy.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index 58aa1ae..815222f 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -154,7 +154,7 @@ def do_import(repo, args):
     refs = [ref]
 
     while True:
-        line = sys.stdin.readline()
+        line = sys.stdin.readline().decode()
         if line == '\n':
             break
         if not line.startswith('import '):
@@ -217,7 +217,7 @@ def read_one_line(repo):
 
     line = sys.stdin.readline()
 
-    cmdline = line
+    cmdline = line.decode()
 
     if not cmdline:
         warn("Unexpected EOF")
@@ -269,7 +269,7 @@ def main(args):
 
     more = True
 
-    sys.stdin = os.fdopen(sys.stdin.fileno(), 'r', 0)
+    sys.stdin = os.fdopen(sys.stdin.fileno(), 'rb', 0)
     while (more):
         more = read_one_line(repo)
 
-- 
1.8.1

^ permalink raw reply related

* [PATCH 6/8] git-remote-testpy: hash bytes explicitly
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

Under Python 3 'hasher.update(...)' must take a byte string and not a
unicode string.  Explicitly encode the argument to this method as UTF-8
so that this code works under Python 3.

This moves the required Python version forward to 2.0.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git-remote-testpy.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index e4533b1..58aa1ae 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -31,9 +31,9 @@ from git_remote_helpers.git.exporter import GitExporter
 from git_remote_helpers.git.importer import GitImporter
 from git_remote_helpers.git.non_local import NonLocalGit
 
-if sys.hexversion < 0x01050200:
-    # os.makedirs() is the limiter
-    sys.stderr.write("git-remote-testgit: requires Python 1.5.2 or later.\n")
+if sys.hexversion < 0x02000000:
+    # string.encode() is the limiter
+    sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
     sys.exit(1)
 
 def get_repo(alias, url):
@@ -45,7 +45,7 @@ def get_repo(alias, url):
     repo.get_head()
 
     hasher = _digest()
-    hasher.update(repo.path)
+    hasher.update(repo.path.encode('utf-8'))
     repo.hash = hasher.hexdigest()
 
     repo.get_base_path = lambda base: os.path.join(
-- 
1.8.1

^ permalink raw reply related

* [PATCH 5/8] svn-fe: allow svnrdump_sim.py to run with Python 3
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

The changes to allow this script to run with Python 3 are minimal and do
not affect its functionality on the versions of Python 2 that are
already supported (2.4 onwards).

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 contrib/svn-fe/svnrdump_sim.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 17cf6f9..4e78a1c 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -14,7 +14,7 @@ if sys.hexversion < 0x02040000:
 
 def getrevlimit():
         var = 'SVNRMAX'
-        if os.environ.has_key(var):
+        if var in os.environ:
                 return os.environ[var]
         return None
 
@@ -44,7 +44,7 @@ def writedump(url, lower, upper):
 
 if __name__ == "__main__":
         if not (len(sys.argv) in (3, 4, 5)):
-                print "usage: %s dump URL -rLOWER:UPPER"
+                print("usage: %s dump URL -rLOWER:UPPER")
                 sys.exit(1)
         if not sys.argv[1] == 'dump': raise NotImplementedError('only "dump" is suppported.')
         url = sys.argv[2]
-- 
1.8.1

^ permalink raw reply related

* [PATCH 4/8] git_remote_helpers: Use 2to3 if building with Python 3
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

Using the approach detailed on the Python wiki[1], run 2to3 on the code
as part of the build if building with Python 3.

The code itself requires no changes to convert cleanly.

[1] http://wiki.python.org/moin/PortingPythonToPy3k

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git_remote_helpers/setup.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/git_remote_helpers/setup.py b/git_remote_helpers/setup.py
index 4d434b6..6de41de 100644
--- a/git_remote_helpers/setup.py
+++ b/git_remote_helpers/setup.py
@@ -4,6 +4,15 @@
 
 from distutils.core import setup
 
+# If building under Python3 we need to run 2to3 on the code, do this by
+# trying to import distutils' 2to3 builder, which is only available in
+# Python3.
+try:
+    from distutils.command.build_py import build_py_2to3 as build_py
+except ImportError:
+    # 2.x
+    from distutils.command.build_py import build_py
+
 setup(
     name = 'git_remote_helpers',
     version = '0.1.0',
@@ -14,4 +23,5 @@ setup(
     url = 'http://www.git-scm.com/',
     package_dir = {'git_remote_helpers': ''},
     packages = ['git_remote_helpers', 'git_remote_helpers.git'],
+    cmdclass = {'build_py': build_py},
 )
-- 
1.8.1

^ permalink raw reply related

* [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

When different version of python are used to build via distutils, the
behaviour can change.  Detect changes in version and pass --force in
this case.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git_remote_helpers/.gitignore | 1 +
 git_remote_helpers/Makefile   | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/git_remote_helpers/.gitignore b/git_remote_helpers/.gitignore
index 2247d5f..06c664f 100644
--- a/git_remote_helpers/.gitignore
+++ b/git_remote_helpers/.gitignore
@@ -1,2 +1,3 @@
+/GIT-PYTHON_VERSION
 /build
 /dist
diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
index f65f064..91f458f 100644
--- a/git_remote_helpers/Makefile
+++ b/git_remote_helpers/Makefile
@@ -25,8 +25,14 @@ PYLIBDIR=$(shell $(PYTHON_PATH) -c \
 	 "import sys; \
 	 print('lib/python%i.%i/site-packages' % sys.version_info[:2])")
 
+py_version=$(shell $(PYTHON_PATH) -c \
+	'import sys; print("%i.%i" % sys.version_info[:2])')
+
 all: $(pysetupfile)
-	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
+	$(QUIET)test "$$(cat GIT-PYTHON_VERSION 2>/dev/null)" = "$(py_version)" || \
+	flags=--force; \
+	$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
+	$(QUIET)echo "$(py_version)" >GIT-PYTHON_VERSION
 
 install: $(pysetupfile)
 	$(PYTHON_PATH) $(pysetupfile) install --prefix $(DESTDIR_SQ)$(prefix)
-- 
1.8.1

^ permalink raw reply related

* [PATCH 2/8] git_remote_helpers: fix input when running under Python 3
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

Although 2to3 will fix most issues in Python 2 code to make it run under
Python 3, it does not handle the new strict separation between byte
strings and unicode strings.  There is one instance in
git_remote_helpers where we are caught by this.

Fix it by explicitly decoding the incoming byte string into a unicode
string.  In this instance, use the locale under which the application is
running.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git_remote_helpers/git/importer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py
index e28cc8f..6814003 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -20,7 +20,7 @@ class GitImporter(object):
         """Returns a dictionary with refs.
         """
         args = ["git", "--git-dir=" + gitdir, "for-each-ref", "refs/heads"]
-        lines = check_output(args).strip().split('\n')
+        lines = check_output(args).decode().strip().split('\n')
         refs = {}
         for line in lines:
             value, name = line.split(' ')
-- 
1.8.1

^ permalink raw reply related

* [PATCH 1/8] git_remote_helpers: Allow building with Python 3
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <cover.1358018078.git.john@keeping.me.uk>

Change inline Python to call "print" as a function not a statement.

This is harmless because Python 2 will see the parentheses as redundant
grouping but they are necessary to run this code with Python 3.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git_remote_helpers/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
index 74b05dc..f65f064 100644
--- a/git_remote_helpers/Makefile
+++ b/git_remote_helpers/Makefile
@@ -23,7 +23,7 @@ endif
 
 PYLIBDIR=$(shell $(PYTHON_PATH) -c \
 	 "import sys; \
-	 print 'lib/python%i.%i/site-packages' % sys.version_info[:2]")
+	 print('lib/python%i.%i/site-packages' % sys.version_info[:2])")
 
 all: $(pysetupfile)
 	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
-- 
1.8.1

^ permalink raw reply related

* [PATCH 0/8] Initial support for Python 3
From: John Keeping @ 2013-01-12 19:23 UTC (permalink / raw)
  To: git; +Cc: John Keeping, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier

I started having a look to see how much work would be needed to make Git
work with Python 3 and the answer is mostly not much.  The exception is
git-p4.py which is hit hard by the distinction between byte strings and
unicode strings, particularly because the Python output mode of p4
targets Python 2.

I don't know if it's worthwhile to actually apply these but here they
are in case anyone's interested.

Having said that, the changes are minimal and involve either wrapping
parentheses around arguments to print or being a bit more explicit about
how we expect byte strings to be decoded to unicode.

With these patches all tests pass with python3 except t98* (git-p4), but
there are a couple of topics in-flight which will affect that
(fc/remote-testgit-feature-done and er/replace-cvsimport).

John Keeping (8):
  git_remote_helpers: Allow building with Python 3
  git_remote_helpers: fix input when running under Python 3
  git_remote_helpers: Force rebuild if python version changes
  git_remote_helpers: Use 2to3 if building with Python 3
  svn-fe: allow svnrdump_sim.py to run with Python 3
  git-remote-testpy: hash bytes explicitly
  git-remote-testpy: don't do unbuffered text I/O
  git-remote-testpy: call print as a function

 contrib/svn-fe/svnrdump_sim.py     |  4 ++--
 git-remote-testpy.py               | 40 +++++++++++++++++++-------------------
 git_remote_helpers/.gitignore      |  1 +
 git_remote_helpers/Makefile        | 10 ++++++++--
 git_remote_helpers/git/importer.py |  2 +-
 git_remote_helpers/setup.py        | 10 ++++++++++
 6 files changed, 42 insertions(+), 25 deletions(-)

-- 
1.8.1

^ permalink raw reply

* git-completion.bash should not add a space after a ref
From: Manlio Perillo @ 2013-01-12 18:35 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: SZEDER Gábor, Felipe Contreras

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi.

This is not really a bug, but a small usability problem.

When completing a reference, Bash will add a space after the reference name.

As an example in:

    $git show master<TAB>

The problem is that an user may want to show a tree or blog object from
master:

    $git show master:git.c


A possible solution is to define a new __gitcomp_nospace function and
use it where appropriate.

Probably the __gitcomp_nospace should be used when git_complete_file is
called, and __gitcomp_nl should be used when __git_complete_revlist  is
called, but I'm not sure.

P.S.:
it seems that __gitcomp_nl is **never** called with 4 arguments.



Regards  Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDxrQ8ACgkQscQJ24LbaURHmACfRXoM+uEVDgFUtZFzUcPC5oSZ
FGsAnAxQf+SN7GrNljxU1io4IuayHmed
=JRVU
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: erratic behavior commit --allow-empty
From: Jan Engelhardt @ 2013-01-12 18:30 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Angelo Borsotti, git
In-Reply-To: <506AA51E.9010209@viscovery.net>


On Tuesday 2012-10-02 10:26, Johannes Sixt wrote:
>
>Note that git commit -m A --allow-empty *DID* create a commit. Only, that
>it received the same name (SHA1) as the commit you created before it
>because it had the exact same contents (files, parents, author, committer,
>and timestamps). Obviously, your script was executed sufficiently fast
>that the two commits happend in the same second.

What about introducing nanosecond-granular timestamps into Git?

^ permalink raw reply

* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Jonathan Nieder @ 2013-01-12 18:26 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Eric S. Raymond, Junio C Hamano, git, Chris Rorvick
In-Reply-To: <50F17DB0.2050802@alum.mit.edu>

Michael Haggerty wrote:

> Regarding your claim that "within a few months the Perl git-cvsimport is
> going to cease even pretending to work": It might be that the old
> git-cvsimport will stop working *for people who upgrade to cvsps 3.x*.
> But it is not realistic to expect people to synchronize their git and
> cvsps version upgrades.  It is even quite possible that this or that
> Linux distribution will package incompatible versions of the two packages.

Moreover, I feel an obligation to point the following out:

In a hypothetical world where cvsps 3.x simply breaks "git cvsimport"
it is likely that some distributions would just stick to the existing
cvsps and not upgrade to 3.x.  Maybe that's a wrong choice, but that's
a choice some would make.  An even more likely outcome in that
hypothetical world is that they would ship it renamed to something
like "cvsps3" alongside the existing cvsps.  Or they could rename the
old version to "cvsps2".  If we were the last holdout, we could even
bundle it as compat/cvsps.

So please do not act as though the cvsps upgrade is a crisis that we
need to break ourselves for at threat of no longer working at all.
The threat doesn't hold water.

Luckily you have already written patches to make "git cvsimport" work
with cvsps 3.x, and through your work you are making a better
argument: "The new cvsimport + cvsps will work better, at least for
some users, than the old tool."

Just don't pretend you have the power to force a change for a less
sensible reason than that!

Hope that helps,
Jonathan

^ permalink raw reply

* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Jonathan Nieder @ 2013-01-12 18:16 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: Michael Haggerty, Junio C Hamano, git, Chris Rorvick
In-Reply-To: <20130112161105.GB3270@thyrsus.com>

Hi Eric,

Eric S. Raymond wrote:

>             But in practice the git crew was going to lose that
> capability anyway simply because the new wrapper will support three
> engines rather than just one.  It's not practical for the git tests to
> handle that many variant external dependencies.

See the git-blame/git-annotate tests for an example of how the
testsuite handles "variations on a theme".

It works fine.

Hope that helps,
Jonathan

^ permalink raw reply

* Re: Suggestion: add option in git-p4 to preserve user in Git repository
From: Pete Wyckoff @ 2013-01-12 16:38 UTC (permalink / raw)
  To: Olivier Delalleau; +Cc: git, Luke Diamand
In-Reply-To: <CAFXk4bpQo26sAfHkE5_VLi_UkZcgsYvwYNH8byZjuXs=EAhu+A@mail.gmail.com>

shish@keba.be wrote on Thu, 10 Jan 2013 22:38 -0500:
> I'm in a situation where I don't have P4 admin rights to use the
> --preserve-user option of git-p4. However, I would like to keep user
> information in the associated Git branch.
> 
> Would it be possible to add an option for this?

The --preserve-user option is used to submit somebody else's work
from git to p4.  It does "p4 change -f" to edit the author of the
change after it has been submitted to p4.  P4 requires admin
privileges to do that.

Changes that are imported _from_ p4 to git do have the correct
author information.

Can you explain a bit more what you're looking for?

		-- Pete

^ permalink raw reply

* [PATCH 0/3] Rework git-diff algorithm selection
From: Michal Privoznik @ 2013-01-12 16:02 UTC (permalink / raw)
  To: git; +Cc: peff, trast

It's been a while I was trying to get this in. Recently, I realized how
important this is.

Please keep me CC'ed as I am not subscribed to the list.

Michal Privoznik (3):
  git-completion.bash: Autocomplete --minimal and --histogram for
    git-diff
  config: Introduce diff.algorithm variable
  diff: Introduce --diff-algorithm command line option

 Documentation/diff-config.txt          | 17 +++++++++++++++++
 Documentation/diff-options.txt         | 22 ++++++++++++++++++++++
 contrib/completion/git-completion.bash | 14 +++++++++++++-
 diff.c                                 | 33 +++++++++++++++++++++++++++++++++
 diff.h                                 |  2 ++
 merge-recursive.c                      |  6 ++++++
 6 files changed, 93 insertions(+), 1 deletion(-)

-- 
1.8.0.2

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2013, #05; Fri, 11)
From: Duy Nguyen @ 2013-01-12 16:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vip739su3.fsf@alter.siamese.dyndns.org>

On Sat, Jan 12, 2013 at 6:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/parse-pathspec (2013-01-11) 20 commits
>
>  Uses the parsed pathspec structure in more places where we used to
>  use the raw "array of strings" pathspec.
>
>  Unfortunately, this conflicts a couple of topics in flight. I tried
>  to be careful while resolving conflicts, though.

parse_pathspec has not picked up init_pathspec changes from
jk/pathspec-literal and nd/pathspec-wildcard (already in master) so
--literal-pathspecs is probably broken in 'pu' after a lot of
init_pathspec -> parse_pathspec conversion.
-- 
Duy

^ permalink raw reply

* [PATCH 1/3] git-completion.bash: Autocomplete --minimal and --histogram for git-diff
From: Michal Privoznik @ 2013-01-12 16:02 UTC (permalink / raw)
  To: git; +Cc: peff, trast
In-Reply-To: <cover.1358006339.git.mprivozn@redhat.com>

Even though --patience was already there, we missed --minimal and
--histogram for some reason.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a4c48e1..383518c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1031,7 +1031,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
 			--no-ext-diff
 			--no-prefix --src-prefix= --dst-prefix=
 			--inter-hunk-context=
-			--patience
+			--patience --histogram --minimal
 			--raw
 			--dirstat --dirstat= --dirstat-by-file
 			--dirstat-by-file= --cumulative
-- 
1.8.0.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox