[PATCH v2 3/3] Add sample pre-push hook script

2013-01-12 Thread Aaron Schrab
Create a sample of a script for a pre-push hook.  The main purpose is to
illustrate how a script may parse the information which is supplied to
such a hook.  The script may also be useful to some people as-is for
avoiding to push commits which are marked as a work in progress.

Signed-off-by: Aaron Schrab 
---
 templates/hooks--pre-push.sample | 53 
 1 file changed, 53 insertions(+)
 create mode 100644 templates/hooks--pre-push.sample

diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
new file mode 100644
index 000..15ab6d8
--- /dev/null
+++ b/templates/hooks--pre-push.sample
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+# An example hook script to verify what is about to be pushed.  Called by "git
+# push" after it has checked the remote status, but before anything has been
+# pushed.  If this script exits with a non-zero status nothing will be pushed.
+#
+# This hook is called with the following parameters:
+#
+# $1 -- Name of the remote to which the push is being done
+# $2 -- URL to which the push is being done
+#
+# If pushing without using a named remote those arguments will be equal.
+#
+# Information about the commits which are being pushed is supplied as lines to
+# the standard input in the form:
+#
+#  
+#
+# This sample shows how to prevent push of commits where the log message starts
+# with "WIP" (work in progress).
+
+remote="$1"
+url="$2"
+
+z40=
+
+IFS=' '
+while read local_ref local_sha remote_ref remote_sha
+do
+   if [ "$local_sha" = $z40 ]
+   then
+   # Handle delete
+   else
+   if [ "$remote_sha" = $z40 ]
+   then
+   # New branch, examine all commits
+   range="$local_sha"
+   else
+   # Update to existing branch, examine new commits
+   range="$remote_sha..$local_sha"
+   fi
+
+   # Check for WIP commit
+   commit=`git rev-list -n 1 --grep '^WIP' "$range"`
+   if [ -n "$commit" ]
+   then
+   echo "Found WIP commit in $local_ref, not pushing"
+   exit 1
+   fi
+   fi
+done
+
+exit 0
-- 
1.8.1.340.g425b78d

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/3] push: Add support for pre-push hooks

2013-01-12 Thread Aaron Schrab
Add support for a pre-push hook which can be used to determine if the
set of refs to be pushed is suitable for the target repository.  The
hook is run with two arguments specifying the name and location of the
destination repository.

Information about what is to be pushed is provided by sending lines of
the following form to the hook's standard input:

   SP  SP  SP  LF

If the hook exits with a non-zero status, the push will be aborted.

This will allow the script to determine if the push is acceptable based
on the target repository and branch(es), the commits which are to be
pushed, and even the source branches in some cases.

Signed-off-by: Aaron Schrab 
---
 Documentation/githooks.txt |  29 ++
 builtin/push.c |   1 +
 t/t5571-pre-push-hook.sh   | 129 +
 transport.c|  60 +
 transport.h|   1 +
 5 files changed, 220 insertions(+)
 create mode 100755 t/t5571-pre-push-hook.sh

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..d839233 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -176,6 +176,35 @@ save and restore any form of metadata associated with the 
working tree
 (eg: permissions/ownership, ACLS, etc).  See contrib/hooks/setgitperms.perl
 for an example of how to do this.
 
+pre-push
+
+
+This hook is called by 'git push' and can be used to prevent a push from taking
+place.  The hook is called with two parameters which provide the name and
+location of the destination remote, if a named remote is not being used both
+values will be the same.
+
+Information about what is to be pushed is provided on the hook's standard
+input with lines of the form:
+
+   SP  SP  SP  LF
+
+For instance, if the command +git push origin master:foreign+ were run the
+hook would receive a line like the following:
+
+  refs/heads/master 67890 refs/heads/foreign 12345
+
+although the full, 40-character SHA1s would be supplied.  If the foreign ref
+does not yet exist the `` will be 40 `0`.  If a ref is to be
+deleted, the `` will be supplied as `(delete)` and the `` will be 40 `0`.  If the local commit was specified by something other
+than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be
+supplied as it was originally given.
+
+If this hook exits with a non-zero status, 'git push' will abort without
+pushing anything.  Information about why the push is rejected may be sent
+to the user by writing to standard error.
+
 [[pre-receive]]
 pre-receive
 ~~~
diff --git a/builtin/push.c b/builtin/push.c
index 8491e43..b158028 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -407,6 +407,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, "progress", &progress, N_("force progress 
reporting")),
OPT_BIT(0, "prune", &flags, N_("prune locally removed refs"),
TRANSPORT_PUSH_PRUNE),
+   OPT_BIT(0, "no-verify", &flags, N_("bypass pre-push hook"), 
TRANSPORT_PUSH_NO_HOOK),
OPT_END()
};
 
diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
new file mode 100755
index 000..d68fed7
--- /dev/null
+++ b/t/t5571-pre-push-hook.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+
+test_description='check pre-push hooks'
+. ./test-lib.sh
+
+# Setup hook that always succeeds
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+HOOK="$HOOKDIR/pre-push"
+mkdir -p "$HOOKDIR"
+write_script "$HOOK" >actual
+cat >>actual
+EOF
+
+cat >expected >expected
+   done
+
+   test_expect_success 'push many refs' '
+   git push parent1 "refs/heads/b/*:refs/heads/b/*" &&
+   diff expected actual
+   '
+fi
+
+test_done
diff --git a/transport.c b/transport.c
index 2673d27..0750a5f 100644
--- a/transport.c
+++ b/transport.c
@@ -1034,6 +1034,62 @@ static void die_with_unpushed_submodules(struct 
string_list *needs_pushing)
die("Aborting.");
 }
 
+static int run_pre_push_hook(struct transport *transport,
+struct ref *remote_refs)
+{
+   int ret = 0, x;
+   struct ref *r;
+   struct child_process proc;
+   struct strbuf buf;
+   const char *argv[4];
+
+   if (!(argv[0] = find_hook("pre-push")))
+   return 0;
+
+   argv[1] = transport->remote->name;
+   argv[2] = transport->url;
+   argv[3] = NULL;
+
+   memset(&proc, 0, sizeof(proc));
+   proc.argv = argv;
+   proc.in = -1;
+
+   if (start_command(&proc)) {
+   finish_command(&proc);
+   return -1;
+   }
+
+   strbuf_init(&buf, 256);
+
+   for (r = remote_re

[PATCH v2 1/3] hooks: Add function to check if a hook exists

2013-01-12 Thread Aaron Schrab
Create find_hook() function to determine if a given hook exists and is
executable.  If it is, the path to the script will be returned,
otherwise NULL is returned.

This encapsulates the tests that are used to check for the existence of
a hook in one place, making it easier to modify those checks if that is
found to be necessary.  This also makes it simple for places that can
use a hook to check if a hook exists before doing, possibly lengthy,
setup work which would be pointless if no such hook is present.

The returned value is left as a static value from get_pathname() rather
than a duplicate because it is anticipated that the return value will
either be used as a boolean, immediately added to an argv_array list
which would result in it being duplicated at that point, or used to
actually run the command without much intervening work.  Callers which
need to hold onto the returned value for a longer time are expected to
duplicate the return value themselves.

Signed-off-by: Aaron Schrab 
---
 builtin/commit.c   |  6 ++
 builtin/receive-pack.c | 25 +++--
 run-command.c  | 15 +--
 run-command.h  |  1 +
 4 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..65d08d2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1327,8 +1327,6 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
return git_status_config(k, v, s);
 }
 
-static const char post_rewrite_hook[] = "hooks/post-rewrite";
-
 static int run_rewrite_hook(const unsigned char *oldsha1,
const unsigned char *newsha1)
 {
@@ -1339,10 +1337,10 @@ static int run_rewrite_hook(const unsigned char 
*oldsha1,
int code;
size_t n;
 
-   if (access(git_path(post_rewrite_hook), X_OK) < 0)
+   argv[0] = find_hook("post-rewrite");
+   if (!argv[0])
return 0;
 
-   argv[0] = git_path(post_rewrite_hook);
argv[1] = "amend";
argv[2] = NULL;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ff781fe..e8878de 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -182,9 +182,6 @@ struct command {
char ref_name[FLEX_ARRAY]; /* more */
 };
 
-static const char pre_receive_hook[] = "hooks/pre-receive";
-static const char post_receive_hook[] = "hooks/post-receive";
-
 static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 
2)));
 static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 
2)));
 
@@ -242,10 +239,10 @@ static int run_and_feed_hook(const char *hook_name, 
feed_fn feed, void *feed_sta
const char *argv[2];
int code;
 
-   if (access(hook_name, X_OK) < 0)
+   argv[0] = find_hook(hook_name);
+   if (!argv[0])
return 0;
 
-   argv[0] = hook_name;
argv[1] = NULL;
 
memset(&proc, 0, sizeof(proc));
@@ -331,15 +328,14 @@ static int run_receive_hook(struct command *commands, 
const char *hook_name,
 
 static int run_update_hook(struct command *cmd)
 {
-   static const char update_hook[] = "hooks/update";
const char *argv[5];
struct child_process proc;
int code;
 
-   if (access(update_hook, X_OK) < 0)
+   argv[0] = find_hook("update");
+   if (!argv[0])
return 0;
 
-   argv[0] = update_hook;
argv[1] = cmd->ref_name;
argv[2] = sha1_to_hex(cmd->old_sha1);
argv[3] = sha1_to_hex(cmd->new_sha1);
@@ -532,24 +528,25 @@ static const char *update(struct command *cmd)
}
 }
 
-static char update_post_hook[] = "hooks/post-update";
-
 static void run_update_post_hook(struct command *commands)
 {
struct command *cmd;
int argc;
const char **argv;
struct child_process proc;
+   char *hook;
 
+   hook = find_hook("post-update");
for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
if (cmd->error_string || cmd->did_not_exist)
continue;
argc++;
}
-   if (!argc || access(update_post_hook, X_OK) < 0)
+   if (!argc || !hook)
return;
+
argv = xmalloc(sizeof(*argv) * (2 + argc));
-   argv[0] = update_post_hook;
+   argv[0] = hook;
 
for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
char *p;
@@ -704,7 +701,7 @@ static void execute_commands(struct command *commands, 
const char *unpacker_erro
   0, &cmd))
set_connectivity_errors(commands);
 
-   if (run_receive_hook(commands, pre_receive_hook, 0)) {
+   if (run_receive_hook(commands, "pre-receive", 0)) {
for (cmd = commands; cmd; cmd = cmd->next) {
if (!cmd->error_string)
cmd->error_string = "pre-receive hook declined";
@@ -994,7 +991,7 @@ int cmd_receive_pack(int argc

[PATCH v2 0/3] pre-push hook support

2013-01-12 Thread Aaron Schrab
Main changes since the initial version:

 * The first patch converts the existing hook callers to use the new
   find_hook() function.
 * Information about what is to be pushed is now sent over a pipe rather
   than passed as command-line parameters.

Aaron Schrab (3):
  hooks: Add function to check if a hook exists
  push: Add support for pre-push hooks
  Add sample pre-push hook script

 Documentation/githooks.txt   |  29 +
 builtin/commit.c |   6 +-
 builtin/push.c   |   1 +
 builtin/receive-pack.c   |  25 
 run-command.c|  15 -
 run-command.h|   1 +
 t/t5571-pre-push-hook.sh | 129 +++
 templates/hooks--pre-push.sample |  53 
 transport.c  |  60 ++
 transport.h  |   1 +
 10 files changed, 300 insertions(+), 20 deletions(-)
 create mode 100755 t/t5571-pre-push-hook.sh
 create mode 100644 templates/hooks--pre-push.sample

-- 
1.8.1.340.g425b78d

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Suggestion: add option in git-p4 to preserve user in Git repository

2013-01-12 Thread Olivier Delalleau
2013/1/12 Pete Wyckoff :
> sh...@keba.be wrote on Sat, 12 Jan 2013 14:44 -0500:
>> 2013/1/12 Pete Wyckoff :
>> > sh...@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 ".
> That would keep the proper attribution, but not work with "git
> log --author", e.g.; you'd have to use "--grep='Real Coder'"
> instead.

Ok, thanks. I actually manage to hack my way around it, restoring the
author information with "git filter-branch" and overriding the remote
p4 tracking branch with "git update-ref". Did some limited testing and
it seems to work -- hopefully I won't have nasty surprises down the
road ;)

-=- Olivier
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/8] git_remote_helpers: fix input when running under Python 3

2013-01-12 Thread Michael Haggerty
On 01/12/2013 08:23 PM, John Keeping wrote:
> 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 
> ---
>  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(' ')
> 

Won't this change cause an exception if the branch names are not all
valid strings in the current locale's encoding?  I don't see how this
assumption is justified (e.g., see git-check-ref-format(1) for the rules
governing reference names).

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to setup bash completion for alias of git command

2013-01-12 Thread Ping Yin
On Sat, Jan 12, 2013 at 10:30 PM, Ping Yin  wrote:
> Following setup works for me  in ubuntu (10.04,11.04) for a long time
>
> alias gtlg='git log'
> complete -o default -o nospace -F _git_log gtlg
>
> However, in debian (testing, wheezy), it doesn't work
>
> $ gtlg or
> gtlg or-bash: [: 1: unary operator expected
> -bash: [: 1: unary operator expected
>

with newest git version built with next branch, the same problem remains.

Ping Yin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/RFC 7/7] contrib/subtree: Handle '--prefix' argument with a slash appended

2013-01-12 Thread Techlive Zheng
'git subtree merge' will fail if the argument of '--prefix' has a slash
appended.

Signed-off-by: Techlive Zheng 
---
 contrib/subtree/git-subtree.sh |  2 +-
 contrib/subtree/t/t7900-subtree.sh | 19 +++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 018ee32..574ff04 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -83,7 +83,7 @@ while [ $# -gt 0 ]; do
--annotate) annotate="$1"; shift ;;
--no-annotate) annotate= ;;
-b) branch="$1"; shift ;;
-   -P) prefix="$1"; shift ;;
+   -P) prefix="${1%/}"; shift ;;
-m) message="$1"; shift ;;
--no-prefix) prefix= ;;
--onto) onto="$1"; shift ;;
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 1492303..8e09606 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -238,6 +238,25 @@ test_expect_success 'merge new subproj history into 
subdir/ with --squash and --
 )
 '
 
+test_expect_success 'merge new subproj history into subdir/ with a slash 
appended to the argument of --prefix' '
+test_create_repo "$test_count" &&
+test_create_repo "$test_count/subproj" &&
+test_create_commit "$test_count" main1 &&
+test_create_commit "$test_count/subproj" sub1 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
+git subtree add --prefix=subdir/ FETCH_HEAD
+) &&
+test_create_commit "$test_count/subproj" sub2 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
+git subtree merge --prefix=subdir/ FETCH_HEAD &&
+test_equal "$(last_commit_message)" "Merge commit '\''$(git rev-parse 
FETCH_HEAD)'\''"
+)
+'
+
 #
 # Tests for 'git subtree split'
 #
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/RFC 6/7] contrib/subtree: Use %B for the split commit message

2013-01-12 Thread Techlive Zheng
Use %B rather than %s%n%n%b to handle the special case of a commit that
only has a subject line.  We don't want to introduce a newline after the
subject, causing generation of a new hash.

After this commit, the newly split branch might differ from the previous
one. If this is the case, --fallback option could help.

Signed-off-by: Techlive Zheng 
Signed-off-by: David A. Greene 
---
 contrib/subtree/git-subtree.sh | 13 ++-
 contrib/subtree/git-subtree.txt| 13 +++
 contrib/subtree/t/t7900-subtree.sh | 47 ++
 3 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 91e6e87..018ee32 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -25,6 +25,7 @@ b,branch= create a new branch from the split subtree
 ignore-joins  ignore prior --rejoin commits
 onto= try connecting new tree to an existing one
 rejoinmerge the new branch back into HEAD
+fallback  fallback to the obsolete commit generating mechanism
  options for 'add', 'merge', 'pull' and 'push'
 squashmerge subtree changes as a single commit
 "
@@ -45,6 +46,7 @@ ignore_joins=
 annotate=
 squash=
 message=
+fallback=
 
 debug()
 {
@@ -92,6 +94,8 @@ while [ $# -gt 0 ]; do
--no-ignore-joins) ignore_joins= ;;
--squash) squash=1 ;;
--no-squash) squash= ;;
+   --fallback) fallback=1 ;;
+   --no-fallback) fallback= ;;
--) break ;;
*) die "Unexpected option: $opt" ;;
esac
@@ -296,7 +300,14 @@ copy_commit()
# We're going to set some environment vars here, so
# do it in a subshell to get rid of them safely later
debug copy_commit "{$1}" "{$2}" "{$3}"
-   git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' 
"$1" |
+
+   if [ -z "$fallback" ]; then
+   log_format='%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B'
+   else
+   log_format='%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b'
+   fi
+
+   git log -1 --pretty=format:"$log_format" "$1" |
(
read GIT_AUTHOR_NAME
read GIT_AUTHOR_EMAIL
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index c5bce41..ca9f199 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -254,6 +254,19 @@ OPTIONS FOR split
'--rejoin' when you split, because you don't want the
subproject's history to be part of your project anyway.
 
+--fallback::
+   Previously, git subtree would introduce an extra new line for
+   the commits whose commit message contains only one line.
+   This behavior has been correct. Unfortunately, for those whose
+   current split branch contains these kind of commits, git subtree
+   will generate a new split branch which differs from the existing
+   split branch in these commits. It is better to use this new
+   split branch, because its commits stay intact within the mainline.
+   
+   Otherwise, the previous fault behavior could still be used with
+   this option. This option is only for a compatible purpose, newly
+   split branch should never use this option.
+
 
 EXAMPLE 1. Add command
 --
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index ece2064..1492303 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -399,6 +399,53 @@ test_expect_success 'split subdir/ with --branch for an 
incompatible branch' '
 )
 '
 
+test_expect_success 'make sure commits with one line message stay intact after 
split' '
+test_create_repo $test_count &&
+test_create_repo $test_count/subproj &&
+test_create_commit $test_count main1 &&
+test_create_commit $test_count/subproj sub1 &&
+(
+cd $test_count &&
+git fetch ./subproj master &&
+ori_hash=$(git rev-parse FETCH_HEAD) &&
+git branch subori FETCH_HEAD &&
+git filter-branch --index-filter '\''git ls-files -s | sed 
"s-\t-&subdir/-" | GIT_INDEX_FILE=$GIT_INDEX_FILE.new git update-index 
--index-info && mv "$GIT_INDEX_FILE.new" "$GIT_INDEX_FILE"'\'' subori
+git merge -m "Merge B project as our subdirectory" subori &&
+git subtree split --prefix subdir --branch splitbr1 &&
+new_hash_1=$(git rev-parse splitbr1) &&
+test_equal "$ori_hash" "$new_hash_1" &&
+git subtree split --prefix subdir --branch splitbr2 --fallback &&
+new_hash_2=$(git rev-parse splitbr2) &&
+test_must_fail test_equal "$ori_hash" "$new_hash_2"
+)
+'
+
+test_expect_success 'make sure --fallback option works correctly for the 
existing split branch' '
+test_create_repo "$test_count" &&
+test_create_repo "$test_count"/subproj &&
+test_create_commit "$test_count" main1

[PATCH/RFC 5/7] contrib/subtree: Make each test self-contained

2013-01-12 Thread Techlive Zheng
Signed-off-by: Techlive Zheng 
---
 contrib/subtree/t/t7900-subtree.sh | 865 ++---
 1 file changed, 614 insertions(+), 251 deletions(-)

diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index bb4fd1f..ece2064 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -12,12 +12,6 @@ export TEST_DIRECTORY=$(pwd)/../../../t
 
 . ../../../t/test-lib.sh
 
-create()
-{
-echo "$1" >"$1"
-git add "$1"
-}
-
 fixnl()
 {
 t=""
@@ -37,11 +31,6 @@ multiline()
 done
 }
 
-undo()
-{
-git reset --hard HEAD~
-}
-
 test_equal()
 {
 test_debug 'echo'
@@ -78,373 +67,746 @@ join_commits()
 echo "$commit $all"
 }
 
+test_create_commit() (
+repo=$1
+commit=$2
+cd "$repo"
+mkdir -p "$(dirname "$commit")"
+echo "$commit" > "$commit"
+git add "$commit"
+git commit -m "$commit"
+)
+
 last_commit_message()
 {
 git log --pretty=format:%s -1
 }
 
-test_expect_success 'init subproj' '
-test_create_repo subproj
-'
-
-# To the subproject!
-cd subproj
-
-test_expect_success 'add sub1' '
-create sub1 &&
-git commit -m "sub1" &&
-git branch sub1 &&
-git branch -m master subproj
-'
-
-test_expect_success 'add sub2' '
-create sub2 &&
-git commit -m "sub2" &&
-git branch sub2
-'
-
-test_expect_success 'add sub3' '
-create sub3 &&
-git commit -m "sub3" &&
-git branch sub3
-'
-
-# Back to mainline
-cd ..
-
-test_expect_success 'add main4' '
-create main4 &&
-git commit -m "main4" &&
-git branch -m master mainline &&
-git branch init
-'
-
-test_expect_success 'fetch subproj history' '
-git fetch ./subproj sub1 &&
-git branch sub1 FETCH_HEAD
-'
+#
+# Tests for 'git subtree add'
+#
 
 test_expect_success 'no pull from non-existant subtree' '
-test_must_fail git subtree pull --prefix=subdir ./subproj sub1
+test_create_repo "$test_count" &&
+test_create_repo "$test_count/subproj" &&
+test_create_commit "$test_count" main1 &&
+test_create_commit "$test_count/subproj" sub1 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
+test_must_fail git subtree pull --prefix=subdir ./subproj master
+)
 '
 
 test_expect_success 'no merge from non-existant subtree' '
+test_create_repo "$test_count" &&
+test_create_repo "$test_count/subproj" &&
+test_create_commit "$test_count" main1 &&
+test_create_commit "$test_count/subproj" sub1 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
 test_must_fail git subtree merge --prefix=subdir FETCH_HEAD
+)
 '
 
 test_expect_success 'add subproj as subtree into subdir/ with --prefix' '
+test_create_repo "$test_count" &&
+test_create_repo "$test_count/subproj" &&
+test_create_commit "$test_count" main1 &&
+test_create_commit "$test_count/subproj" sub1 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
 git subtree add --prefix=subdir FETCH_HEAD &&
-test_equal "$(last_commit_message)" "Add '\''subdir/'\'' from commit 
'\''$(git rev-parse FETCH_HEAD)'\''" &&
-undo
+test_equal "$(last_commit_message)" "Add '\''subdir/'\'' from commit 
'\''$(git rev-parse FETCH_HEAD)'\''"
+)
 '
 
 test_expect_success 'add subproj as subtree into subdir/ with --prefix and 
--message' '
+test_create_repo "$test_count" &&
+test_create_repo "$test_count/subproj" &&
+test_create_commit "$test_count" main1 &&
+test_create_commit "$test_count/subproj" sub1 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
 git subtree add --prefix=subdir --message="Added subproject" 
FETCH_HEAD &&
-test_equal "$(last_commit_message)" "Added subproject" &&
-undo
+test_equal "$(last_commit_message)" "Added subproject"
+)
 '
 
 test_expect_success 'add subproj as subtree into subdir/ with --prefix as -P 
and --message as -m' '
+test_create_repo "$test_count" &&
+test_create_repo "$test_count/subproj" &&
+test_create_commit "$test_count" main1 &&
+test_create_commit "$test_count/subproj" sub1 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
 git subtree add -P subdir -m "Added subproject" FETCH_HEAD &&
-test_equal "$(last_commit_message)" "Added subproject" &&
-undo
+test_equal "$(last_commit_message)" "Added subproject"
+)
 '
 
 test_expect_success 'add subproj as subtree into subdir/ with --squash and 
--prefix and --message' '
+test_create_repo "$test_count" &&
+test_create_repo "$test_count/subproj" &&
+test_create_commit "$test_count" main1 &&
+test_create_commit "$test_count/subproj" sub1 &&
+(
+cd "$test_count" &&
+git fetch ./subproj master &&
 git subtree add --prefix=subdir --mess

[PATCH/RFC 4/7] contrib/subtree: Code cleaning and refactoring

2013-01-12 Thread Techlive Zheng
Mostly prepare for the later tests refactoring.

Signed-off-by: Techlive Zheng 
---
 contrib/subtree/git-subtree.sh |  66 -
 contrib/subtree/t/t7900-subtree.sh | 283 +++--
 2 files changed, 179 insertions(+), 170 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 138e1e0..91e6e87 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -5,7 +5,7 @@
 # Copyright (C) 2009 Avery Pennarun 
 #
 if [ $# -eq 0 ]; then
-set -- -h
+   set -- -h
 fi
 OPTS_SPEC="\
 git subtree add   --prefix= 
@@ -110,9 +110,9 @@ if [ -z "$prefix" ]; then
 fi
 
 case "$command" in
-   add) [ -e "$prefix" ] && 
+   add) [ -e "$prefix" ] &&
die "prefix '$prefix' already exists." ;;
-   *)   [ -e "$prefix" ] || 
+   *)   [ -e "$prefix" ] ||
die "'$prefix' does not exist; use 'git subtree add'" ;;
 esac
 
@@ -181,8 +181,8 @@ cache_set()
oldrev="$1"
newrev="$2"
if [ "$oldrev" != "latest_old" \
--a "$oldrev" != "latest_new" \
--a -e "$cachedir/$oldrev" ]; then
+   -a "$oldrev" != "latest_new" \
+   -a -e "$cachedir/$oldrev" ]; then
die "cache for $oldrev already exists!"
fi
echo "$newrev" >"$cachedir/$oldrev"
@@ -327,7 +327,7 @@ add_msg()
fi
cat <<-EOF
$commit_message
-   
+
git-subtree-dir: $dir
git-subtree-mainline: $latest_old
git-subtree-split: $latest_new
@@ -355,7 +355,7 @@ rejoin_msg()
fi
cat <<-EOF
$commit_message
-   
+
git-subtree-dir: $dir
git-subtree-mainline: $latest_old
git-subtree-split: $latest_new
@@ -368,7 +368,7 @@ squash_msg()
oldsub="$2"
newsub="$3"
newsub_short=$(git rev-parse --short "$newsub")
-   
+
if [ -n "$oldsub" ]; then
oldsub_short=$(git rev-parse --short "$oldsub")
echo "Squashed '$dir/' changes from 
$oldsub_short..$newsub_short"
@@ -378,7 +378,7 @@ squash_msg()
else
echo "Squashed '$dir/' content from commit $newsub_short"
fi
-   
+
echo
echo "git-subtree-dir: $dir"
echo "git-subtree-split: $newsub"
@@ -427,7 +427,7 @@ new_squash_commit()
newsub="$3"
tree=$(toptree_for_commit $newsub) || exit $?
if [ -n "$old" ]; then
-   squash_msg "$dir" "$oldsub" "$newsub" | 
+   squash_msg "$dir" "$oldsub" "$newsub" |
git commit-tree "$tree" -p "$old" || exit $?
else
squash_msg "$dir" "" "$newsub" |
@@ -455,7 +455,7 @@ copy_or_skip()
else
nonidentical="$parent"
fi
-   
+
# sometimes both old parents map to the same newparent;
# eliminate duplicates
is_new=1
@@ -470,7 +470,7 @@ copy_or_skip()
p="$p -p $parent"
fi
done
-   
+
if [ -n "$identical" ]; then
echo $identical
else
@@ -495,14 +495,14 @@ cmd_add()
fi
 
ensure_clean
-   
+
if [ $# -eq 1 ]; then
"cmd_add_commit" "$@"
elif [ $# -eq 2 ]; then
"cmd_add_repository" "$@"
else
-   say "error: parameters were '$@'"
-   die "Provide either a refspec or a repository and refspec."
+   say "error: parameters were '$@'"
+   die "Provide either a refspec or a repository and refspec."
fi
 }
 
@@ -522,19 +522,19 @@ cmd_add_commit()
revs=$(git rev-parse $default --revs-only "$@") || exit $?
set -- $revs
rev="$1"
-   
+
debug "Adding $dir as '$rev'..."
git read-tree --prefix="$dir" $rev || exit $?
git checkout -- "$dir" || exit $?
tree=$(git write-tree) || exit $?
-   
+
headrev=$(git rev-parse HEAD) || exit $?
if [ -n "$headrev" -a "$headrev" != "$rev" ]; then
headp="-p $headrev"
else
headp=
fi
-   
+
if [ -n "$squash" ]; then
rev=$(new_squash_commit "" "" "$rev") || exit $?
commit=$(add_squashed_msg "$rev" "$dir" |
@@ -544,7 +544,7 @@ cmd_add_commit()
 git commit-tree $tree $headp -p "$rev") || exit $?
fi
git reset "$commit" || exit $?
-   
+
say "Added dir '$dir'"
 }
 
@@ -552,7 +552,7 @@ cmd_split()
 {
debug "Splitting $dir..."
cache_setup || exit $?
-   
+
if [ -n "$onto" ]; then
debug "Reading history for --onto=$onto..."
git rev-list $onto |
@@ -563,13 +563,13 @@ cmd_split()
cache_s

[PATCH/RFC 3/7] contrib/subtree: Remove test number comments

2013-01-12 Thread Techlive Zheng
From: "David A. Greene" 

Delete the comments indicating test numbers as it causes maintenance
headaches.  t*.sh -i will help us find any broken tests.

Signed-off-by: David A. Greene 
Signed-off-by: Techlive Zheng 
---
 contrib/subtree/t/t7900-subtree.sh | 55 --
 1 file changed, 55 deletions(-)

diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 3e02aeb..abdcddb 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -60,7 +60,6 @@ last_commit_message()
git log --pretty=format:%s -1
 }
 
-# 1
 test_expect_success 'init subproj' '
 test_create_repo subproj
 '
@@ -68,7 +67,6 @@ test_expect_success 'init subproj' '
 # To the subproject!
 cd subproj
 
-# 2
 test_expect_success 'add sub1' '
 create sub1 &&
 git commit -m "sub1" &&
@@ -76,14 +74,12 @@ test_expect_success 'add sub1' '
 git branch -m master subproj
 '
 
-# 3
 test_expect_success 'add sub2' '
 create sub2 &&
 git commit -m "sub2" &&
 git branch sub2
 '
 
-# 4
 test_expect_success 'add sub3' '
 create sub3 &&
 git commit -m "sub3" &&
@@ -93,7 +89,6 @@ test_expect_success 'add sub3' '
 # Back to mainline
 cd ..
 
-# 5
 test_expect_success 'add main4' '
 create main4 &&
 git commit -m "main4" &&
@@ -101,101 +96,85 @@ test_expect_success 'add main4' '
 git branch subdir
 '
 
-# 6
 test_expect_success 'fetch subproj history' '
 git fetch ./subproj sub1 &&
 git branch sub1 FETCH_HEAD
 '
 
-# 7
 test_expect_success 'no subtree exists in main tree' '
 test_must_fail git subtree merge --prefix=subdir sub1
 '
 
-# 8
 test_expect_success 'no pull from non-existant subtree' '
 test_must_fail git subtree pull --prefix=subdir ./subproj sub1
 '
 
-# 9
 test_expect_success 'check if --message works for add' '
 git subtree add --prefix=subdir --message="Added subproject" sub1 &&
 check_equal ''"$(last_commit_message)"'' "Added subproject" &&
 undo
 '
 
-# 10
 test_expect_success 'check if --message works as -m and --prefix as -P' '
 git subtree add -P subdir -m "Added subproject using git subtree" sub1 
&&
 check_equal ''"$(last_commit_message)"'' "Added subproject using git 
subtree" &&
 undo
 '
 
-# 11
 test_expect_success 'check if --message works with squash too' '
 git subtree add -P subdir -m "Added subproject with squash" --squash 
sub1 &&
 check_equal ''"$(last_commit_message)"'' "Added subproject with 
squash" &&
 undo
 '
 
-# 12
 test_expect_success 'add subproj to mainline' '
 git subtree add --prefix=subdir/ FETCH_HEAD &&
 check_equal ''"$(last_commit_message)"'' "Add '"'subdir/'"' from 
commit '"'"'''"$(git rev-parse sub1)"'''"'"'"
 '
 
-# 13
 # this shouldn't actually do anything, since FETCH_HEAD is already a parent
 test_expect_success 'merge fetched subproj' '
 git merge -m "merge -s -ours" -s ours FETCH_HEAD
 '
 
-# 14
 test_expect_success 'add main-sub5' '
 create subdir/main-sub5 &&
 git commit -m "main-sub5"
 '
 
-# 15
 test_expect_success 'add main6' '
 create main6 &&
 git commit -m "main6 boring"
 '
 
-# 16
 test_expect_success 'add main-sub7' '
 create subdir/main-sub7 &&
 git commit -m "main-sub7"
 '
 
-# 17
 test_expect_success 'fetch new subproj history' '
 git fetch ./subproj sub2 &&
 git branch sub2 FETCH_HEAD
 '
 
-# 18
 test_expect_success 'check if --message works for merge' '
 git subtree merge --prefix=subdir -m "Merged changes from subproject" 
sub2 &&
 check_equal ''"$(last_commit_message)"'' "Merged changes from 
subproject" &&
 undo
 '
 
-# 19
 test_expect_success 'check if --message for merge works with squash too' '
 git subtree merge --prefix subdir -m "Merged changes from subproject 
using squash" --squash sub2 &&
 check_equal ''"$(last_commit_message)"'' "Merged changes from 
subproject using squash" &&
 undo
 '
 
-# 20
 test_expect_success 'merge new subproj history into subdir' '
 git subtree merge --prefix=subdir FETCH_HEAD &&
 git branch pre-split &&
 check_equal ''"$(last_commit_message)"'' "Merge commit '"'"'"$(git 
rev-parse sub2)"'"'"' into mainline"
 '
 
-# 21
 test_expect_success 'Check that prefix argument is required for split' '
 echo "You must provide the --prefix option." > expected &&
 test_must_fail git subtree split > actual 2>&1 &&
@@ -207,7 +186,6 @@ test_expect_success 'Check that prefix argument is required 
for split' '
 rm -f expected actual
 '
 
-# 22
 test_expect_success 'Check that the  exists for a split' '
 echo "'"'"'non-existent-directory'"'"'" does not exist\; use "'"'"'git 
subtree add'"'"'" > expected &&
 test_must_fail git subtree split --prefix=non-existent-directory > 
actu

[PATCH/RFC 2/7] contrib/subtree: Ignore testing directory

2013-01-12 Thread Techlive Zheng
Signed-off-by: Techlive Zheng 
---
 contrib/subtree/.gitignore | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
index 91360a3..59aeeb4 100644
--- a/contrib/subtree/.gitignore
+++ b/contrib/subtree/.gitignore
@@ -1,6 +1,5 @@
 *~
 git-subtree
-git-subtree.xml
 git-subtree.1
-mainline
-subproj
+git-subtree.xml
+t/trash\ directory.*
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/RFC 1/7] contrib/subtree: Add vim modeline

2013-01-12 Thread Techlive Zheng
Signed-off-by: Techlive Zheng 
---
 contrib/subtree/git-subtree.sh | 2 ++
 contrib/subtree/t/t7900-subtree.sh | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 920c664..138e1e0 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -710,3 +710,5 @@ cmd_push()
 }
 
 "cmd_$command" "$@"
+
+# vim: set ts=4 sw=4 noet
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index bc2eeb0..3e02aeb 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -506,3 +506,5 @@ test_expect_success 'verify one file change per commit' '
 '
 
 test_done
+
+# vim: set et ts=4 sw=4
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/RFC 0/7] mutiple improvements

2013-01-12 Thread Techlive Zheng
* refactor tests for 'git subtree'
  * rearrange some tests
  * clean up unnecessary quotes
  * make each test self-contained
* keep commit intact after the split by using '%B'
* handle '--prefix' argument with slash appended correctly

David A. Greene (1):
  contrib/subtree: Remove test number comments

Techlive Zheng (6):
  contrib/subtree: Add vim modeline
  contrib/subtree: Ignore testing directory
  contrib/subtree: Code cleaning and refactoring
  contrib/subtree: Make each test self-contained
  contrib/subtree: Use %B for the split commit message
  contrib/subtree: Handle '--prefix' argument with a slash appended

 contrib/subtree/.gitignore |5 +-
 contrib/subtree/git-subtree.sh |   83 ++-
 contrib/subtree/git-subtree.txt|   13 +
 contrib/subtree/t/t7900-subtree.sh | 1233 +++-
 4 files changed, 872 insertions(+), 462 deletions(-)

-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: missing objects -- prevention

2013-01-12 Thread Sitaram Chamarty
On Sat, Jan 12, 2013 at 6:43 PM, Jeff King  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/8] Initial support for Python 3

2013-01-12 Thread John Keeping
On Sat, Jan 12, 2013 at 06:43:04PM -0500, Pete Wyckoff wrote:
> j...@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +:
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/8] Initial support for Python 3

2013-01-12 Thread Pete Wyckoff
j...@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes

2013-01-12 Thread Pete Wyckoff
j...@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +:
> 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Suggestion: add option in git-p4 to preserve user in Git repository

2013-01-12 Thread Pete Wyckoff
sh...@keba.be wrote on Sat, 12 Jan 2013 14:44 -0500:
> 2013/1/12 Pete Wyckoff :
> > sh...@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 ".
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/21] commit: convert to use parse_pathspec

2013-01-12 Thread Martin von Zweigbergk
On Fri, Jan 11, 2013 at 3:20 AM, Nguyễn Thái Ngọc Duy  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] rebase --preserve-merges keeps empty merge commits

2013-01-12 Thread 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Suggestion: add option in git-p4 to preserve user in Git repository

2013-01-12 Thread Olivier Delalleau
2013/1/12 Pete Wyckoff :
> sh...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 8/8] git-remote-testpy: call print as a function

2013-01-12 Thread John Keeping
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 7/8] git-remote-testpy: don't do unbuffered text I/O

2013-01-12 Thread John Keeping
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/8] git-remote-testpy: hash bytes explicitly

2013-01-12 Thread John Keeping
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 
---
 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 < 0x0200:
+# 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/8] svn-fe: allow svnrdump_sim.py to run with Python 3

2013-01-12 Thread John Keeping
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 
---
 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 < 0x0204:
 
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/8] git_remote_helpers: Use 2to3 if building with Python 3

2013-01-12 Thread John Keeping
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/8] git_remote_helpers: Force rebuild if python version changes

2013-01-12 Thread John Keeping
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/8] git_remote_helpers: fix input when running under Python 3

2013-01-12 Thread John Keeping
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/8] git_remote_helpers: Allow building with Python 3

2013-01-12 Thread John Keeping
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/8] Initial support for Python 3

2013-01-12 Thread John Keeping
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git-completion.bash should not add a space after a ref

2013-01-12 Thread Manlio Perillo
-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

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-
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: erratic behavior commit --allow-empty

2013-01-12 Thread Jan Engelhardt

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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

2013-01-12 Thread Jonathan Nieder
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

2013-01-12 Thread Jonathan Nieder
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Suggestion: add option in git-p4 to preserve user in Git repository

2013-01-12 Thread Pete Wyckoff
sh...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/3] Rework git-diff algorithm selection

2013-01-12 Thread Michal Privoznik
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] git-completion.bash: Autocomplete --minimal and --histogram for git-diff

2013-01-12 Thread Michal Privoznik
Even though --patience was already there, we missed --minimal and
--histogram for some reason.

Signed-off-by: Michal Privoznik 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jan 2013, #05; Fri, 11)

2013-01-12 Thread Duy Nguyen
On Sat, Jan 12, 2013 at 6:56 AM, Junio C Hamano  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

2013-01-12 Thread Eric S. Raymond
Michael Haggerty :
> Otherwise, how do we know that cvsps currently works with git-cvsimport?
> (OK, you claim that it does, but in the next breath you admit that
> there is a new failure in "one pathological tagging case".)  How can we
> understand its strengths/weaknesses?  How can we gain confidence that it
> works on different platforms?  How will we find out if a future versions
> of cvsps stops working (e.g., because of a breakage or a
> non-backwards-compatible change)?

You can't.  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.

However, there is a solution.

The solution is for git to test that the wrapper is *generating the
expected commands*.  So what the git tree ends up with is conditional
assurance; the wrapper will do the right thing if the engine it calls
is working correctly.  I think that's really all the git-tree tests
can hope for.

Michael, the engines are my problem and yours - it's *our*
responsibility to develop a (hopefully shared) test suite to verify
that they convert repos correctly.  I'm working my end as fast as I can;
I hope to have the test suite factored out of cvsps and ready to check 
multiple engines by around Wednesday.  I still need to convert t9604,
too.

I have parsecvs working since yesterday, so we really are up to three
engines.

I have two minor features I need to merge into parsecvs before 
I can start on splitting out the test suite.
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] diff: Introduce --diff-algorithm command line option

2013-01-12 Thread Michal Privoznik
Since command line options have higher priority than config file
variables and taking previous commit into account, we need a way
how to specify myers algorithm on command line. However,
inventing `--myers` is not the right answer. We need far more
general option, and that is `--diff-algorithm`. The older options
(`--minimal`, `--patience` and `--histogram`) are kept for
backward compatibility.

Signed-off-by: Michal Privoznik 
---
 Documentation/diff-options.txt | 22 ++
 contrib/completion/git-completion.bash | 11 +++
 diff.c | 12 +++-
 diff.h |  2 ++
 merge-recursive.c  |  6 ++
 5 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 39f2c50..4091f52 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -45,6 +45,9 @@ ifndef::git-format-patch[]
Synonym for `-p --raw`.
 endif::git-format-patch[]
 
+The next three options are kept for compatibility reason. You should use the
+`--diff-algorithm` option instead.
+
 --minimal::
Spend extra time to make sure the smallest possible
diff is produced.
@@ -55,6 +58,25 @@ endif::git-format-patch[]
 --histogram::
Generate a diff using the "histogram diff" algorithm.
 
+--diff-algorithm={patience|minimal|histogram|myers}::
+   Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+   The basic greedy diff algorithm.
+`minimal`;;
+   Spend extra time to make sure the smallest possible diff is
+   produced.
+`patience`;;
+   Use "patience diff" algorithm when generating patches.
+`histogram`;;
+   This algorithm extends the patience algorithm to "support
+   low-occurrence common elements".
+--
++
+You should prefer this option over the `--minimal`, `--patience` and
+`--histogram` which are kept just for backwards compatibility.
+
 --stat[=[,[,]]]::
Generate a diffstat. By default, as much space as necessary
will be used for the filename part, and the rest for the graph
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 33e25dc..d592cf9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1021,6 +1021,8 @@ _git_describe ()
__gitcomp_nl "$(__git_refs)"
 }
 
+__git_diff_algorithms="myers minimal patience histogram"
+
 __git_diff_common_options="--stat --numstat --shortstat --summary
--patch-with-stat --name-only --name-status --color
--no-color --color-words --no-renames --check
@@ -1035,6 +1037,7 @@ __git_diff_common_options="--stat --numstat --shortstat 
--summary
--raw
--dirstat --dirstat= --dirstat-by-file
--dirstat-by-file= --cumulative
+   --diff-algorithm=
 "
 
 _git_diff ()
@@ -1042,6 +1045,10 @@ _git_diff ()
__git_has_doubledash && return
 
case "$cur" in
+   --diff-algorithm=*)
+   __gitcomp "$__git_diff_algorithms" "" 
"${cur##--diff-algorithm=}"
+   return
+   ;;
--*)
__gitcomp "--cached --staged --pickaxe-all --pickaxe-regex
--base --ours --theirs --no-index
@@ -2114,6 +2121,10 @@ _git_show ()
" "" "${cur#*=}"
return
;;
+   --diff-algorithm=*)
+   __gitcomp "$__git_diff_algorithms" "" 
"${cur##--diff-algorithm=}"
+   return
+   ;;
--*)
__gitcomp "--pretty= --format= --abbrev-commit --oneline
$__git_diff_common_options
diff --git a/diff.c b/diff.c
index ddae5c4..6418076 100644
--- a/diff.c
+++ b/diff.c
@@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char 
*value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
 }
 
-static long parse_algorithm_value(const char *value)
+long parse_algorithm_value(const char *value)
 {
if (!value || !strcasecmp(value, "myers"))
return 0;
@@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, "--histogram"))
options->xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
+   else if (!prefixcmp(arg, "--diff-algorithm=")) {
+   long value = parse_algorithm_value(arg+17);
+   if (value < 0)
+   return error("option diff-algorithm accepts \"myers\", "
+"\"minimal\", \"patience\" and 
\"histogram\"");
+   /* clear out previous settings */
+   DIFF_XDL_CLR(options, NEED_MINIMAL);
+  

[PATCH 2/3] config: Introduce diff.algorithm variable

2013-01-12 Thread Michal Privoznik
Some users or projects prefer different algorithms over others, e.g.
patience over myers or similar. However, specifying appropriate
argument every time diff is to be used is impractical. Moreover,
creating an alias doesn't play nicely with other tools based on diff
(git-show for instance). Hence, a configuration variable which is able
to set specific algorithm is needed. For now, these four values are
accepted: 'myers' (which has the same effect as not setting the config
variable at all), 'minimal', 'patience' and 'histogram'.

Signed-off-by: Michal Privoznik 
---
 Documentation/diff-config.txt  | 17 +
 contrib/completion/git-completion.bash |  1 +
 diff.c | 23 +++
 3 files changed, 41 insertions(+)

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 4314ad0..be31276 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -155,3 +155,20 @@ diff.tool::
"kompare".  Any other value is treated as a custom diff tool,
and there must be a corresponding `difftool..cmd`
option.
+
+diff.algorithm::
+   Choose a diff algorithm.  The variants are as follows:
++
+--
+`myers`;;
+   The basic greedy diff algorithm.
+`minimal`;;
+   Spend extra time to make sure the smallest possible diff is
+   produced.
+`patience`;;
+   Use "patience diff" algorithm when generating patches.
+`histogram`;;
+   This algorithm extends the patience algorithm to "support
+   low-occurrence common elements".
+--
++
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 383518c..33e25dc 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1839,6 +1839,7 @@ _git_config ()
diff.suppressBlankEmpty
diff.tool
diff.wordRegex
+   diff.algorithm
difftool.
difftool.prompt
fetch.recurseSubmodules
diff --git a/diff.c b/diff.c
index 732d4c2..ddae5c4 100644
--- a/diff.c
+++ b/diff.c
@@ -36,6 +36,7 @@ static int diff_no_prefix;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
+static long diff_algorithm = 0;
 
 static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -143,6 +144,20 @@ static int git_config_rename(const char *var, const char 
*value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
 }
 
+static long parse_algorithm_value(const char *value)
+{
+   if (!value || !strcasecmp(value, "myers"))
+   return 0;
+   else if (!strcasecmp(value, "minimal"))
+   return XDF_NEED_MINIMAL;
+   else if (!strcasecmp(value, "patience"))
+   return XDF_PATIENCE_DIFF;
+   else if (!strcasecmp(value, "histogram"))
+   return XDF_HISTOGRAM_DIFF;
+   else
+   return -1;
+}
+
 /*
  * These are to give UI layer defaults.
  * The core-level commands such as git-diff-files should
@@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char *value, 
void *cb)
return 0;
}
 
+   if (!strcmp(var, "diff.algorithm")) {
+   diff_algorithm = parse_algorithm_value(value);
+   if (diff_algorithm < 0)
+   return -1;
+   return 0;
+   }
+
if (git_color_config(var, value, cb) < 0)
return -1;
 
@@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)
options->add_remove = diff_addremove;
options->use_color = diff_use_color_default;
options->detect_rename = diff_detect_rename_default;
+   options->xdl_opts |= diff_algorithm;
 
if (diff_no_prefix) {
options->a_prefix = options->b_prefix = "";
-- 
1.8.0.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

2013-01-12 Thread Eric S. Raymond
Junio C Hamano :
> And here is what I got:

Hm. In my version of these tests, I only have one regression from the
old combo (in the pathological tags test, t9602).  You're seeing more
breakage than that, obviously.

> A funny thing was that without cvsps-3.7 on $PATH (which means I am
> getting distro packaged cvsps 2.1), I got identical errors.

That suggests that something in your test setup has gone bad and is
introducing spurious errors. Which would be consistent with the above.

> Looking
> at the log message, it seems that you meant to remove t960[123], so
> perhaps the patch simply forgot to remove 9601 and 9602?

Yes.
 
> As neither test runs "git cvsimport" with -o/-m/-M options, ideally
> we should be able to pass them with and without having cvsps-3.x.
> Not passing them without cvsps-3.x would mean that the fallback mode
> of rewritten cvsimport is not working as expected. Not passing them
> with cvsps-3.x may mean the tests were expecting a wrong conversion
> result, or they uncover bugs in the replacement cvsimport.

That's possible, but seems unlikely.  Because the new cvsimport is
such a thin wrapper around the conversion engine, bugs in it should
lead to obvious crashes or failure to run the engine rather than the 
sort of conversion error the t960* tests are designed to check.  Really
all it does is assemble options to pass to the conversion engines.

My test strategy is aimed at the engine, not the wrapper. I took the
repos from t960*  and wrote a small Python framework to check the same 
assertions as the git-tree tests do, but using the engine.  For example,
here's how my t9602 looks:

import os, cvspstest

cc = cvspstest.ConvertComparison("t9602", "module")
cc.cmp_branch_tree("test of branch", "master", True)
cc.cmp_branch_tree("test of branch", "vendorbranch", True)
cc.cmp_branch_tree("test of branch", "B_FROM_INITIALS", False)
cc.cmp_branch_tree("test of branch", "B_FROM_INITIALS_BUT_ONE", False)
cc.cmp_branch_tree("test of branch", "B_MIXED", False)
cc.cmp_branch_tree("test of branch", "B_SPLIT", True)
cc.cmp_branch_tree("test of tag", "vendortag", False)
# This is the only test new cvsps fails that old git-cvsimport passed.
cc.cmp_branch_tree("test of tag", "T_ALL_INITIAL_FILES", True)
cc.cmp_branch_tree("test of tag", "T_ALL_INITIAL_FILES_BUT_ONE", False)
cc.cmp_branch_tree("test of tag", "T_MIXED", False)
cc.cleanup()
 
> t9600 fails with "-a is no longer supported", even without having
> cvsps-3.x on the $PATH (i.e. attempting to use the fallback).  I
> wonder if this is an option the updated cvsimport would want to
> simply ignore?

Probably.  But I don't think you should keep these tests in the git tree.
That wasn't a great idea even when you were supporting just one engine;
with two (and soon three) it's going to be just silly.  Let sanity-checking
the engines be *my* problem, since I have to do it anyway.

(I'm working towards the generalized test suite as fast as I can.  First
results probably in four days or so.)

> It is a way to tell the old cvsps/cvsimport to disable its
> heuristics to ignore commits made within the last 10 minutes (this
> is done in the hope of waiting for the per-file nature of CVS
> commits to stabilize, IIUC); the user tells the command that he
> knows that the CVS repository is now quiescent and it is safe to
> import the whole thing.

Yes, that's just what -a is supposed to do.  But is should be
irrelevant for testing - in the test framework CVS is running locally, 
so there's no network lag.

> So... does this mean that we now set the minimum required version of
> Python to 2.7?  I dunno.

That would be bad, IMO.  I'll put backporting to 2.6 high on my to-do list.
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t/lib-cvs: cvsimport no longer works without Python >= 2.7

2013-01-12 Thread Michael Haggerty
I have the feeling I'm only seeing one side of this conversation...

On 01/12/2013 09:40 AM, Junio C Hamano wrote:
> The new cvsimport requires at least Python 2.7 to work; do not fail
> the cvsimport tests on platforms without one.
> 
> Signed-off-by: Junio C Hamano 
> ---
> 
>  Junio C Hamano  writes:
> 
>  > http://docs.python.org/2/library/subprocess.html tells me that
>  > check_output has first become available in 2.7.
>  >
>  > So... does this mean that we now set the minimum required version of
>  > Python to 2.7?  I dunno.

It would be unfortunate to set the minimum Python version to 2.7 if "git
cvsimport" is considered an integral part of git.

>  Even if we were to rip out the fallback code that uses the 2.7-only
>  subprocess.check_output() on "cvsps -V", the function is also used
>  for doing the real work interacting with cvsps-3.x, so I think this
>  patch will be necessary.  Unless new cvsimport is tweaked not to
>  use the method, that is.
> 
>  A suggestion for a better alternative is of course very much
>  appreciated.

If the only reason to require Python 2.7 is subprocess.check_output(),
it would be easy to reimplement it (it is only 12 lines of
straightforward code, plus a few lines to define the exception type
CalledProcessError).  According to [1], the Python license is
GPL-compatible; therefore these lines could even be copied into
git-cvsimport.

Michael

[1] http://www.gnu.org/licenses/license-list.html#Python

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

2013-01-12 Thread Michael Haggerty
On 01/11/2013 04:32 AM, Junio C Hamano wrote:
> From: "Eric S. Raymond" 
> 
> The combination of git-cvsimport and cvsps had serious problems.

Agreed.

> [...]
> This patch also removes Michael Haggerty's git-cvsimport tests
> (t960[123]) from the git tree.  These are actually conversion-engine
> tests and have been merged into a larger cvsps test suite, which I
> intend to spin out into a general CVS-lifting test that can also be
> applied to utilities such as cvs2git and parsecvs.  The t9604 test
> will move in a future patch, when I likewise have it integrated
> into the general test suite.
> 
> The following known bug has not been fixed: "If any files were ever
> "cvs import"ed more than once (e.g., import of more than one vendor
> release) the HEAD contains the wrong content." However, cvsps now
> emits a warning in this case. There is also one pathological tagging
> case that was successful in the former t9602 test that now fails
> (with a warning).
> 
> I plan to address these problems. This patch at least gets the
> cvsps-3.x/git-cvsimport combination to a state that is not too
> broken to ship - that is, in all failure cases known to me it
> now emits useful warnings rather than silently botching the
> import.

I don't understand the logic of removing the cvsimport tests, at least
not at this time.  It is true that the tests mostly ensure that the
conversion engine is working correctly, especially with your new version
of cvsps.  But I think the git project, by implicitly endorsing the use
of cvsps, has some responsibility to verify that the combination cvsps +
git-cvsimport continues to work and to document any known breakages via
its test suite.

Otherwise, how do we know that cvsps currently works with git-cvsimport?
 (OK, you claim that it does, but in the next breath you admit that
there is a new failure in "one pathological tagging case".)  How can we
understand its strengths/weaknesses?  How can we gain confidence that it
works on different platforms?  How will we find out if a future versions
of cvsps stops working (e.g., because of a breakage or a
non-backwards-compatible change)?

Normally one would expect an improvement like this to be combined with
patches that turn test expected failures into expected successes, not to
rip out the very tests that establish the correctness of the change that
is being proposed!


Let me describe what I would consider to be the optimum state of the
test suite.  Maybe your idea of "optimum" differs from mine, or maybe
the optimum is unrealistic due to lack of resources or for some other
reason.  But if so, let's explicitly spell out why we are deviating from
whatever optimum we define.

* The old tests should be retained (and possibly new tests added to show
off your improvements).

* There should be a way for users to choose which cvsps executable to
use when running test suite.  (In the future, the selection might be
expanded to cover altogether different conversion engines.)

* The tests should determine which version of cvsps has been selected
(e.g., by running "cvsps --version").

* The individual tests should be marked expected success/expected
failure based on the selected version of cvsps; in other words, some
tests might be marked "expected failure" if cvsps 2.x is being used but
"expected success" if cvsps 3.x is being used.


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.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] git-completion.bash: add support for path completion

2013-01-12 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 11/01/2013 23:02, Junio C Hamano ha scritto:
> Manlio Perillo  writes:
> 
>> +# Process path list returned by "ls-files" and "diff-index --name-only"
>> +# commands, in order to list only file names relative to a specified
>> +# directory, and append a slash to directory names.
>> +__git_index_file_list_filter ()
>> +{
>> +# Default to Bash >= 4.x
>> +__git_index_file_list_filter_bash
>> +}
>> +
>> +# Execute git ls-files, returning paths relative to the directory
>> +# specified in the first argument, and using the options specified in
>> +# the second argument.
>> +__git_ls_files_helper ()
>> +{
>> +# NOTE: $2 is not quoted in order to support multiple options
>> +cd "$1" && git ls-files --exclude-standard $2
>> +} 2>/dev/null
> 
> I think this redirection is correct but a bit tricky;

It's not tricky: it is POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10


> it is in
> effect during the execution of the { block } (in other words, it is
> not about squelching errors during the function definition).
> 

What do you mean by "squelching"?

Note that I originally wrote the code as

__git_ls_files_helper ()
{
# NOTE: $2 is not quoted in order to support multiple options
 { cd "$1" && git ls-files --exclude-standard $2 } 2>/dev/null
}

but then I checked the POSIX standard, noting that it is redundant.

> -- >8 --
> #!/bin/sh
> cat >t.sh <<\EOF &&
> echo I am "$1"
> t () { echo "Goes to stdout"; echo >&2 "Goes to stderr"; } 2>/dev/null
> t
> for sh in bash dash ksh zsh
> do
>   $sh t.sh $sh
> done
> -- 8< --
> 

There is a missing EOF delimiter.
And I'm not sure to understand the meaning of && after EOF.

> Bash does (so do dash and real AT&T ksh) grok this correctly, but
> zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
> pretending to be ksh gets this wrong as well).  Not that what ksh
> does matters, as it won't be dot-sourcing bash completion script.
>

I have added tcsh to the sh list, but it fails with:
Badly placed ()'s.


> It however may affect zsh, which does seem to dot-source this file.
> Perhaps zsh completion may have to be rewritten in a similar way as
> tcsh completion is done (i.e. does not dot-source this file but ask
> bash to do the heavy-lifting).
> 

Ok, I was wrong on assuming all modern shells were POSIX compliant.
I will change the code to use a nested {} group.

> This function seems to be always called in an subshell (e.g. as an
> upstream of a pipeline), so the "cd" may be harmless, but don't you
> need to disable CDPATH while doing this?
> 

I don't know.

> [..]


>> +# Try to count non option arguments passed on the command line for the
>> +# specified git command.
>> +# When options are used, it is necessary to use the special -- option to
>> +# tell the implementation were non option arguments begin.
>> +# XXX this can not be improved, since options can appear everywhere, as
>> +# an example:
>> +#   git mv x -n y
> 
> If that is the case, it is a bug in the command line parser, I
> think.  We should reject it, and the command line completer
> certainly should not encourage it.
> 

$ mkdir y
$ git mv x -n y
Checking rename of 'x' to 'y/x'
Renaming x to y/x
$ git status
# On branch master
nothing to commit, working directory clean

I was assuming it to be "normal", given how complex Git command line
parsing is (IMHO).


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

iEYEARECAAYFAlDxeMgACgkQscQJ24LbaUTmaQCeMbZ0lRJxZIx3U31gMPmcqTLp
54sAmwYrjJVuvRYcsbGaMa3rb9/EKrBU
=ky30
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Support FTP-over-SSL/TLS for regular FTP

2013-01-12 Thread Modestas Vainius
Hello,

Saturday 12 January 2013 06:25:21 rašė:
> On Sat, Jan 12, 2013 at 03:59:52PM +0200, Modestas Vainius wrote:
> > @@ -306,6 +311,11 @@ static CURL *get_curl_handle(void)
> > 
> > if (curl_ftp_no_epsv)
> > 
> > curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
> > 
> > +#ifdef CURLOPT_USE_SSL
> > +if (curl_ssl_try)
> > +   curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
> > +#endif
> > +
> > 
> > if (curl_http_proxy) {
> > 
> > curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> > curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> 
> It looks like the indentation of the "if" line you added is messed up.

Yeah, sorry about that. I will fix it.

-- 
Modestas Vainius 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Support FTP-over-SSL/TLS for regular FTP

2013-01-12 Thread Matt Kraai
On Sat, Jan 12, 2013 at 03:59:52PM +0200, Modestas Vainius wrote:
> @@ -306,6 +311,11 @@ static CURL *get_curl_handle(void)
>   if (curl_ftp_no_epsv)
>   curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
>  
> +#ifdef CURLOPT_USE_SSL
> +if (curl_ssl_try)
> + curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
> +#endif
> +
>   if (curl_http_proxy) {
>   curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>   curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);

It looks like the indentation of the "if" line you added is messed up.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Support FTP-over-SSL/TLS for regular FTP

2013-01-12 Thread Modestas Vainius
Add a boolean http.sslTry option which allows to enable AUTH SSL/TLS and
encrypted data transfers when connecting via regular FTP protocol.

Default is false since it might trigger certificate verification errors on
misconfigured servers.

Signed-off-by: Modestas Vainius 
---
 Documentation/config.txt |8 
 http.c   |   10 ++
 http.h   |9 +
 3 files changed, 27 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..1abd161 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1406,6 +1406,14 @@ http.sslCAPath::
with when fetching or pushing over HTTPS. Can be overridden
by the 'GIT_SSL_CAPATH' environment variable.
 
+http.sslTry::
+   Attempt to use AUTH SSL/TLS and encrypted data transfers
+   when connecting via regular FTP protocol. This might be needed
+   if the FTP server requires it for security reasons or you wish
+   to connect securely whenever remote FTP server supports it.
+   Default is false since it might trigger certificate verification
+   errors on misconfigured servers.
+
 http.maxRequests::
How many HTTP requests to launch in parallel. Can be overridden
by the 'GIT_HTTP_MAX_REQUESTS' environment variable. Default is 5.
diff --git a/http.c b/http.c
index 44f3525..d49a3d4 100644
--- a/http.c
+++ b/http.c
@@ -30,6 +30,7 @@ static CURL *curl_default;
 char curl_errorstr[CURL_ERROR_SIZE];
 
 static int curl_ssl_verify = -1;
+static int curl_ssl_try;
 static const char *ssl_cert;
 #if LIBCURL_VERSION_NUM >= 0x070903
 static const char *ssl_key;
@@ -162,6 +163,10 @@ static int http_options(const char *var, const char 
*value, void *cb)
ssl_cert_password_required = 1;
return 0;
}
+   if (!strcmp("http.ssltry", var)) {
+   curl_ssl_try = git_config_bool(var, value);
+   return 0;
+   }
if (!strcmp("http.minsessions", var)) {
min_curl_sessions = git_config_int(var, value);
 #ifndef USE_CURL_MULTI
@@ -306,6 +311,11 @@ static CURL *get_curl_handle(void)
if (curl_ftp_no_epsv)
curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
 
+#ifdef CURLOPT_USE_SSL
+if (curl_ssl_try)
+   curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
+#endif
+
if (curl_http_proxy) {
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
diff --git a/http.h b/http.h
index 0a80d30..f861662 100644
--- a/http.h
+++ b/http.h
@@ -42,6 +42,15 @@
 #define NO_CURL_IOCTL
 #endif
 
+/*
+ * CURLOPT_USE_SSL was known as CURLOPT_FTP_SSL up to 7.16.4,
+ * and the constants were known as CURLFTPSSL_*
+*/
+#if !defined(CURLOPT_USE_SSL) && defined(CURLOPT_FTP_SSL)
+#define CURLOPT_USE_SSL CURLOPT_FTP_SSL
+#define CURLUSESSL_TRY CURLFTPSSL_TRY
+#endif
+
 struct slot_results {
CURLcode curl_result;
long http_code;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: missing objects -- prevention

2013-01-12 Thread Jeff King
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.

> > 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.

> > 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).

> > 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.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] git-completion.bash: add support for path completion

2013-01-12 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 11/01/2013 19:48, Manlio Perillo ha scritto:
> The git-completion.bash script did not implemented full, git aware,
> support to complete paths, for git commands that operate on files within
> the current working directory or the index.
> [...]
>  
> +# Try to count non option arguments passed on the command line for the
> +# specified git command.
> +# When options are used, it is necessary to use the special -- option to
> +# tell the implementation were non option arguments begin.
> +# XXX this can not be improved, since options can appear everywhere, as
> +# an example:
> +#git mv x -n y
> +#
> +# __git_count_arguments requires 1 argument: the git command executed.
> +__git_count_arguments ()
> +{
> + local word i c=0
> +
> + # Skip "git" (first argument)
> + for ((i=1; i < ${#words[@]}; i++)); do
> + word="${words[i]}"
> +
> + case "$word" in
> + --)

Sorry, I have incorrectly (again) indented the case labels.
I have now configured my editor to correctly indent this.

> + # Good; we can assume that the following are 
> only non
> + # option arguments.
> + ((c = 0))
> + ;;

Here I was thinking to do something like this (not tested):

-*)
if [ -n ${2-} ]; then
# Assume specified git command only
# accepts simple options
# (without arguments)
((c = 0))

Since git mv only accepts simple options, this will make the use of '--'
not required.

Note that I'm assuming the single '-' character is used as a non-option
argument; not sure this is the case of Git.

> [...]


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

iEYEARECAAYFAlDxXLkACgkQscQJ24LbaUR+QQCaA4WZP5h5lktXJqSB7c494fAY
B6IAoIRWyIzBq29S7+l+TfRjbyp19HNL
=JRpR
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Possible bug in `remote set-url --add --push`

2013-01-12 Thread Jardel Weyrich
On Sat, Jan 12, 2013 at 6:44 AM, Sascha Cunz  wrote:
> Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano:
>> Jardel Weyrich  writes:
>> > I believe `remote set-url --add --push` has a bug. Performed tests
>> > with v1.8.0.1 and v1.8.1 (Mac OS X).
>> >
>> > Quoting the relevant part of the documentation:
>> >> set-url
>> >>
>> >> Changes URL remote points to. Sets first URL remote points to
>> >> matching regex  (first URL if no  is given) to
>> >> . If  doesn’t match any URL, error occurs and
>> >> nothing is changed.
>> >>
>> >> With --push, push URLs are manipulated instead of fetch URLs.
>> >> With --add, instead of changing some URL, new URL is added.
>> >> With --delete, instead of changing some URL, all URLs matching regex
>> >>  are deleted. Trying to delete all non-push URLs is an error.>
>> > Here are some steps to reproduce:
>> >
>> > 1. Show the remote URLs
>> >
>> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> > origin  /Volumes/sandbox/test (fetch)
>> > origin  /Volumes/sandbox/test (push)
>> >
>> > 2. Add a new push URL for origin
>> >
>> > jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push
>> > origin \>
>> > /Volumes/sandbox/test_clone2
>> >
>> > 3. Check what happened
>> >
>> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> > origin  /Volumes/sandbox/test (fetch)
>> > origin  /Volumes/sandbox/test_clone2 (push)
>>
>> The original pushurl was replaced with the additional one, instead
>> of being left and the new one getting added.  That looks certainly
>> wrong.
>>
>> However, the result of applying the attached patch (either to
>> v1.7.12 or v1.8.1) still passes the test and I do not think it is
>> doing anything differently from what you described above.
>>
>> What do you get from
>>
>>   git config -l | grep '^remote\.origin'
>>
>> in steps 1. and 3. in your procedure?  This question is trying to
>> tell if your bug is in "git remote -v" or in "git remote set-url".
>
> I'm not sure, if there is a bug at all. According to man git-push:
>
> The  is used for pushes only. It is optional and defaults to
>.
> (From the section REMOTES -> Named remote in configuration file)
>
> the command:
> git remote add foo g...@foo-fetch.org/some.git
>
> will set "remote.foo.url" to "g...@foo-fetch.org". Subsequently, fetch and 
> push
> will use g...@foo-fetch.org as url.
> Fetch will use this url, because "remote.foo.url" explicitly sets this. push
> will use it in absence of a "remote.foo.pushurl".
>
> Now, we're adding a push-url:
> git remote set-url --add --push foo g...@foo-push.org/some.git
>
> Relevant parts of config are now looking like:
> [remote "foo"]
> url = g...@foo-fetch.org/some.git
> pushurl = g...@foo-push.org/some.git
>
> Since, pushurl is now given explicitly, git push will use that one (and only
> that one).
>
> If we add another push-url now,
> git remote set-url --add --push foo g...@foo-push-also.org/some.git
>
> the next git-push will push to foo-push.org and foo-push-also.org.
>
> Now, using --set-url --delete on both of these urls restores the original
> state: only "remote.foo.url" is set; meaning implicitly pushurl defaults to
> url again.
>
> To me this is exactly what Jardel was observing:
>
>> In step 2, Git replaced the original push URL instead of adding a new
>> one. But it seems to happen only the first time I use `remote set-url
>> --add --push`. Re-adding the original URL using the same command seems
>> to work properly.
>
>> And FWIW, if I delete (with "set-url --delete") both URLs push, Git
>> restores the original URL.
>
> Or am I missing something here?

You're right. However, as I quoted earlier, the git-remote man-page states:

   set-url
   Changes URL remote points to. 
   With --push, push URLs are manipulated instead of fetch URLs.
   With --add, instead of changing some URL, new URL is added.

It explicitly mentions that it should **add a new URL**.
So when I do `git remote set-url --add --push origin
git://another/repo.git`, I expect git-push to use both the default
push URL and the new one. Or am I misinterpreting the man-page?

>
> Might be that the "bug" actually is that the expectation was
>
> git remote add foo g...@foo-fetch.org/some.git
>
> should have created a config like:
>
> [remote "foo"]
> url = g...@foo-fetch.org/some.git
> pushurl = g...@foo-fetch.org/some.git
>
> since that is what "git remote -v" reports.
>
> If that is the case, we might want to amend the output of 'git remote -v' with
> the information that a pushurl is not explicitly given and thus defaults to
> url.

Correct. Adding a remote doesn't automatically generate a pushurl for it.

To me, it seems that git-push checks for the existence of pushurl's in
the config, and if it finds any, it ignores the defaul push URL during
the actual push. In better words, it pushes only 

Re: [BUG] Possible bug in `remote set-url --add --push`

2013-01-12 Thread Sascha Cunz
Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano:
> Jardel Weyrich  writes:
> > I believe `remote set-url --add --push` has a bug. Performed tests
> > with v1.8.0.1 and v1.8.1 (Mac OS X).
> > 
> > Quoting the relevant part of the documentation:
> >> set-url
> >> 
> >> Changes URL remote points to. Sets first URL remote points to
> >> matching regex  (first URL if no  is given) to
> >> . If  doesn’t match any URL, error occurs and
> >> nothing is changed.
> >> 
> >> With --push, push URLs are manipulated instead of fetch URLs.
> >> With --add, instead of changing some URL, new URL is added.
> >> With --delete, instead of changing some URL, all URLs matching regex
> >>  are deleted. Trying to delete all non-push URLs is an error.> 
> > Here are some steps to reproduce:
> > 
> > 1. Show the remote URLs
> > 
> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
> > origin  /Volumes/sandbox/test (fetch)
> > origin  /Volumes/sandbox/test (push)
> > 
> > 2. Add a new push URL for origin
> > 
> > jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push
> > origin \> 
> > /Volumes/sandbox/test_clone2
> > 
> > 3. Check what happened
> > 
> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
> > origin  /Volumes/sandbox/test (fetch)
> > origin  /Volumes/sandbox/test_clone2 (push)
> 
> The original pushurl was replaced with the additional one, instead
> of being left and the new one getting added.  That looks certainly
> wrong.
> 
> However, the result of applying the attached patch (either to
> v1.7.12 or v1.8.1) still passes the test and I do not think it is
> doing anything differently from what you described above.
> 
> What do you get from
> 
>   git config -l | grep '^remote\.origin'
> 
> in steps 1. and 3. in your procedure?  This question is trying to
> tell if your bug is in "git remote -v" or in "git remote set-url".

I'm not sure, if there is a bug at all. According to man git-push:

The  is used for pushes only. It is optional and defaults to
   .
(From the section REMOTES -> Named remote in configuration file)

the command:
git remote add foo g...@foo-fetch.org/some.git

will set "remote.foo.url" to "g...@foo-fetch.org". Subsequently, fetch and push 
will use g...@foo-fetch.org as url.
Fetch will use this url, because "remote.foo.url" explicitly sets this. push 
will use it in absence of a "remote.foo.pushurl".

Now, we're adding a push-url:
git remote set-url --add --push foo g...@foo-push.org/some.git

Relevant parts of config are now looking like:
[remote "foo"]
url = g...@foo-fetch.org/some.git
pushurl = g...@foo-push.org/some.git

Since, pushurl is now given explicitly, git push will use that one (and only 
that one).

If we add another push-url now,
git remote set-url --add --push foo g...@foo-push-also.org/some.git

the next git-push will push to foo-push.org and foo-push-also.org.

Now, using --set-url --delete on both of these urls restores the original 
state: only "remote.foo.url" is set; meaning implicitly pushurl defaults to 
url again.

To me this is exactly what Jardel was observing:

> In step 2, Git replaced the original push URL instead of adding a new
> one. But it seems to happen only the first time I use `remote set-url
> --add --push`. Re-adding the original URL using the same command seems
> to work properly.

> And FWIW, if I delete (with "set-url --delete") both URLs push, Git
> restores the original URL.

Or am I missing something here?

Might be that the "bug" actually is that the expectation was

git remote add foo g...@foo-fetch.org/some.git

should have created a config like:

[remote "foo"]
url = g...@foo-fetch.org/some.git
pushurl = g...@foo-fetch.org/some.git

since that is what "git remote -v" reports.

If that is the case, we might want to amend the output of 'git remote -v' with 
the information that a pushurl is not explicitly given and thus defaults to 
url.

Sascha
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t/lib-cvs: cvsimport no longer works without Python >= 2.7

2013-01-12 Thread Junio C Hamano
The new cvsimport requires at least Python 2.7 to work; do not fail
the cvsimport tests on platforms without one.

Signed-off-by: Junio C Hamano 
---

 Junio C Hamano  writes:

 > http://docs.python.org/2/library/subprocess.html tells me that
 > check_output has first become available in 2.7.
 >
 > So... does this mean that we now set the minimum required version of
 > Python to 2.7?  I dunno.

 Even if we were to rip out the fallback code that uses the 2.7-only
 subprocess.check_output() on "cvsps -V", the function is also used
 for doing the real work interacting with cvsps-3.x, so I think this
 patch will be necessary.  Unless new cvsimport is tweaked not to
 use the method, that is.

 A suggestion for a better alternative is of course very much
 appreciated.

 I do not want to keep pushing integration results that do not pass
 tests even for myself for too many integration cycles, so I'll keep
 this patch at the tip of the topic, at least for now.

 t/lib-cvs.sh | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index b55e861..4e890ea 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -27,6 +27,21 @@ case "$cvsps_version" in
;;
 esac
 
+if ! test_have_prereq PYTHON
+then
+   skipall='skipping cvsimport tests, no python'
+   test_done
+fi
+
+python -c '
+import sys
+if sys.hexversion < 0x0207:
+   sys.exit(1)
+' || {
+   skip_all='skipping cvsimport tests, python too old (< 2.7)'
+   test_done
+}
+
 setup_cvs_test_repository () {
CVSROOT="$(pwd)/.cvsroot" &&
cp -r "$TEST_DIRECTORY/$1/cvsroot" "$CVSROOT" &&
-- 
1.8.1.421.g6236851

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Possible bug in `remote set-url --add --push`

2013-01-12 Thread Junio C Hamano
Jardel Weyrich  writes:

> Step 1:
>
> jweyrich@pharao:test_clone1 [* master]$ git remote -v
> origin /Volumes/sandbox/test (fetch)
> origin /Volumes/sandbox/test (push)
>
> jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
> remote.origin.url=/Volumes/sandbox/test
> remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
>
> Step 3:
>
> jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add
> --push origin /Volumes/sandbox/test_clone2
> origin /Volumes/sandbox/test (fetch)
> origin /Volumes/sandbox/test_clone2/ (push)
>
> jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
> remote.origin.url=/Volumes/sandbox/test
> remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
> remote.origin.pushurl=/Volumes/sandbox/test_clone2/

So "remote -v" is not lying (we only see one pushurl after Step 3
above) and "set-url" is not working correctly on your box in a way
that I cannot reproduce X-<.

> ...
> Is `remote..pushurl` required for the primary URL as
> well?

What do you mean by "primary URL"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Possible bug in `remote set-url --add --push`

2013-01-12 Thread Jardel Weyrich
Step 1:

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test (push)

jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
remote.origin.url=/Volumes/sandbox/test
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*

Step 3:

jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add
--push origin /Volumes/sandbox/test_clone2
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test_clone2/ (push)

jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
remote.origin.url=/Volumes/sandbox/test
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
remote.origin.pushurl=/Volumes/sandbox/test_clone2/


After that, if I do a commit in test_clone1 and try to push to origin,
it pushes only to the test_clone2 rather than pushing to both test and
test_clone2 (it's a bare repo, sorry for using a misleading name).

Is `remote..pushurl` required for the primary URL as
well? If not, then git-push is not handling that information as it
should.

On Sat, Jan 12, 2013 at 5:10 AM, Junio C Hamano  wrote:
> Jardel Weyrich  writes:
>
>> I believe `remote set-url --add --push` has a bug. Performed tests
>> with v1.8.0.1 and v1.8.1 (Mac OS X).
>>
>> Quoting the relevant part of the documentation:
>>
>>> set-url
>>> Changes URL remote points to. Sets first URL remote points to matching 
>>> regex  (first URL if no  is given) to . If  
>>> doesn’t match any URL, error occurs and nothing is changed.
>>>
>>> With --push, push URLs are manipulated instead of fetch URLs.
>>> With --add, instead of changing some URL, new URL is added.
>>> With --delete, instead of changing some URL, all URLs matching regex 
>>>  are deleted. Trying to delete all non-push URLs is an error.
>>
>> Here are some steps to reproduce:
>>
>> 1. Show the remote URLs
>>
>> jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> origin  /Volumes/sandbox/test (fetch)
>> origin  /Volumes/sandbox/test (push)
>>
>> 2. Add a new push URL for origin
>>
>> jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push 
>> origin \
>> /Volumes/sandbox/test_clone2
>>
>> 3. Check what happened
>>
>> jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> origin  /Volumes/sandbox/test (fetch)
>> origin  /Volumes/sandbox/test_clone2 (push)
>
> The original pushurl was replaced with the additional one, instead
> of being left and the new one getting added.  That looks certainly
> wrong.
>
> However, the result of applying the attached patch (either to
> v1.7.12 or v1.8.1) still passes the test and I do not think it is
> doing anything differently from what you described above.
>
> What do you get from
>
> git config -l | grep '^remote\.origin'
>
> in steps 1. and 3. in your procedure?  This question is trying to
> tell if your bug is in "git remote -v" or in "git remote set-url".
>
> -- >8 --
> From 0f6cbc67db926e97707ae732b02e790b4604508e Mon Sep 17 00:00:00 2001
> From: Junio C Hamano 
> Date: Fri, 11 Jan 2013 23:04:16 -0800
> Subject: [PATCH] t5505: adding one pushurl from jweyrich
>
> Signed-off-by: Junio C Hamano 
> ---
>  t/t5505-remote.sh | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index c03ffdd..b31c5bb 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -901,6 +901,25 @@ test_expect_success 'remote set-url --push --add aaa' '
> cmp expect actual
>  '
>
> +test_expect_success 'remote set-url --push --add' '
> +   git config remote.jweyrich.url /Volumes/sandbox/test &&
> +   git config remote.jweyrich.pushurl /Volumes/sandbox/test &&
> +   git config remote.jweyrich.fetch 
> "refs/heads/*:refs/remotes/jweyrich/*" &&
> +
> +   added=/Volumes/sandbox/test_clone2 &&
> +   {
> +   git config -l | grep "^remote\.jweyrich\." &&
> +   echo "remote.jweyrich.pushurl=$added"
> +   } | sort >expect &&
> +
> +   git remote set-url --add --push jweyrich "$added" &&
> +   git config -l | grep "^remote\.jweyrich\." | sort >actual &&
> +
> +   test_cmp expect actual &&
> +
> +   git remote -v | grep "^jweyrich" # this is just for debugging
> +'
> +
>  test_expect_success 'remote set-url --push bar aaa' '
> git remote set-url --push someremote bar aaa &&
> echo foo >expect &&
> --
> 1.8.1.421.g6236851
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html