threads / patch / 36380

patch, 5 partsFixes

Subject: [PATCH 0/5] Fixes

## tl;dr

8 messages between Apr 9, 2014 and Apr 9, 2014. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Felipe Contreras· Apr 9, 2014, 18:49 UTC · lore
Felipe Contreras (4):
  remote-helpers: allow all tests running from any dir
  remote-hg: always normalize paths
  remote-bzr: add support for older versions
  completion: fix completion of certain aliases
dequis (1):
  remote-bzr: include authors field in pushed commits
 contrib/completion/git-completion.bash   |  1 +
 contrib/completion/git-completion.zsh    |  1 +
 contrib/remote-helpers/git-remote-bzr    |  6 ++++--
 contrib/remote-helpers/git-remote-hg     |  1 +
 contrib/remote-helpers/test-bzr.sh       | 24 ++++++++++++++++++++++++
 contrib/remote-helpers/test-hg-bidi.sh   |  3 ++-
 contrib/remote-helpers/test-hg-hg-git.sh |  3 ++-
 7 files changed, 35 insertions(+), 4 deletions(-)
-- 
1.9.1+fc1
Felipe Contreras· Apr 9, 2014, 18:50 UTC · re: Felipe Contreras · lore

[PATCH 1/5] remote-helpers: allow all tests running from any dir

Commit d3243d7 (test-bzr.sh, test-hg.sh: allow running from any dir) allowed the tests to run from any directory, however, it didn't update all the tests.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-hg-bidi.sh   | 3 ++-
 contrib/remote-helpers/test-hg-hg-git.sh | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
Show changes to 2 files +4 −2

contrib/remote-helpers/test-hg-bidi.sh, contrib/remote-helpers/test-hg-hg-git.sh

diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
index e24c51d..d86e147 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -8,7 +8,8 @@
 
 test_description='Test bidirectionality of remote-hg'
 
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 6dcd95d..b23909a 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -8,7 +8,8 @@
 
 test_description='Test remote-hg output compared to hg-git'
 
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
-- 
1.9.1+fc1
Felipe Contreras· Apr 9, 2014, 18:50 UTC · re: Felipe Contreras · lore

[PATCH 2/5] remote-hg: always normalize paths

Apparently Mercurial can have paths such as 'foo//bar', so normalize all paths.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 1 +
 1 file changed, 1 insertion(+)
Show changes to contrib/remote-helpers/git-remote-hg +1 −0
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index eb89ef6..84e3872 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -260,6 +260,7 @@ class Parser:
         return (user, int(date), -tz)
 
 def fix_file_path(path):
+    path = os.path.normpath(path)
     if not os.path.isabs(path):
         return path
     return os.path.relpath(path, '/')
-- 
1.9.1+fc1
Felipe Contreras· Apr 9, 2014, 18:50 UTC · re: Felipe Contreras · lore

[PATCH 3/5] remote-bzr: add support for older versions

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to contrib/remote-helpers/git-remote-bzr +2 −2
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 332aba7..7f354c8 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -759,7 +759,7 @@ def clone(path, remote_branch):
 def get_remote_branch(name):
     remote_branch = bzrlib.branch.Branch.open(branches[name],
                                               possible_transports=transports)
-    if isinstance(remote_branch.user_transport, bzrlib.transport.local.LocalTransport):
+    if isinstance(remote_branch.bzrdir.root_transport, bzrlib.transport.local.LocalTransport):
         return remote_branch
 
     branch_path = os.path.join(dirname, 'clone', name)
@@ -842,7 +842,7 @@ def get_repo(url, alias):
     if not wanted:
         try:
             repo = origin.open_repository()
-            if not repo.user_transport.listable():
+            if not repo.bzrdir.root_transport.listable():
                 # this repository is not usable for us
                 raise bzrlib.errors.NoRepositoryPresent(repo.bzrdir)
         except bzrlib.errors.NoRepositoryPresent:
-- 
1.9.1+fc1
Felipe Contreras· Apr 9, 2014, 18:50 UTC · re: Felipe Contreras · lore

[PATCH 4/5] remote-bzr: include authors field in pushed commits

From: dequis <dx@dxzone.com.ar>
Tests-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr |  2 ++
 contrib/remote-helpers/test-bzr.sh    | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
Show changes to 2 files +26 −0

contrib/remote-helpers/git-remote-bzr, contrib/remote-helpers/test-bzr.sh

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 7f354c8..6ca1e97 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -618,10 +618,12 @@ def parse_commit(parser):
         files[path] = f
 
     committer, date, tz = committer
+    author, _, _ = author
     parents = [mark_to_rev(p) for p in parents]
     revid = bzrlib.generate_ids.gen_revision_id(committer, date)
     props = {}
     props['branch-nick'] = branch.nick
+    props['authors'] = author
 
     mtree = CustomTree(branch, revid, parents, files)
     changes = mtree.iter_changes()
diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 1e53ff9..431de3b 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -391,4 +391,28 @@ test_expect_success 'export utf-8 authors' '
 	test_cmp expected actual
 '
 
+test_expect_success 'push different author' '
+	test_when_finished "rm -rf bzrrepo gitrepo" &&
+
+	bzr init bzrrepo &&
+
+	(
+	git init gitrepo &&
+	cd gitrepo &&
+	echo john >> content &&
+	git add content &&
+	git commit -m john --author "John Doe <jdoe@example.com>" &&
+	git remote add bzr "bzr::../bzrrepo" &&
+	git push bzr master
+	) &&
+
+	(
+	cd bzrrepo &&
+	bzr log | grep "^author: " > ../actual
+	) &&
+
+	echo "author: John Doe <jdoe@example.com>" > expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.9.1+fc1
Felipe Contreras· Apr 9, 2014, 18:50 UTC · re: Felipe Contreras · lore

[PATCH 5/5] completion: fix completion of certain aliases

Some commands need the first word to determine the actual action that is being executed, however, the command is wrong when we use an alias, for example 'alias.p=push', if we try to complete 'git p origin ', the result would be wrong because __git_complete_remote_or_refspec() doesn't know where it come from.

So let's override words[1], so the alias 'p' is override by the actual command, 'push'.

Reported-by: Aymeric Beaumet <aymeric.beaumet@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 1 +
 contrib/completion/git-completion.zsh  | 1 +
 2 files changed, 2 insertions(+)
Show changes to 2 files +2 −0

contrib/completion/git-completion.bash, contrib/completion/git-completion.zsh

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9525343..893ae5d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2547,6 +2547,7 @@ __git_main ()
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then
+		words[1]=$expansion
 		completion_func="_git_${expansion//-/_}"
 		declare -f $completion_func >/dev/null && $completion_func
 	fi
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 6b77968..9f6f0fa 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -104,6 +104,7 @@ __git_zsh_bash_func ()
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then
+		words[1]=$expansion
 		completion_func="_git_${expansion//-/_}"
 		declare -f $completion_func >/dev/null && $completion_func
 	fi
-- 
1.9.1+fc1
Junio C Hamano· Apr 9, 2014, 19:33 UTC · re: Felipe Contreras · lore

Re: [PATCH 5/5] completion: fix completion of certain aliases

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 12 quoted lines
> Some commands need the first word to determine the actual action that is
> being executed, however, the command is wrong when we use an alias, for
> example 'alias.p=push', if we try to complete 'git p origin ', the
> result would be wrong because __git_complete_remote_or_refspec() doesn't
> know where it come from.
>
> So let's override words[1], so the alias 'p' is override by the actual
> command, 'push'.
>
> Reported-by: Aymeric Beaumet <aymeric.beaumet@gmail.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

Does "some commands" above refer to anything that uses __git_complete_remote_or_refspec, or is the set of commands larger than that?

I am wondering if it is safer to introduce a new "local" variable that is set by the caller of __git_complete_remote_or_refspec and inspected by __git_complete_remote_or_refspec (instead of words[1]) to communiate the real name of the git subcommand being completed, without touching words[] in place.

That way, we wouldn't have to worry about all the other references of words[c], words[i], words[CURRENT] etc. in the script seeing the word that the end-user did not type and did not actually appear on the command line.

But perhaps we muck with the contents of words[] in a similar way in many different places in the existing completion code often enough that such an attempt not to touch the words[] array does not buy us much safety anyway. I didn't check (and that is why I am asking with "I am wondering...").

Thanks, will queue.

[Ram and Szeder CC'ed as they appear in shortlog for the past 12 months].

Show 28 quoted lines
>  contrib/completion/git-completion.bash | 1 +
>  contrib/completion/git-completion.zsh  | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 9525343..893ae5d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2547,6 +2547,7 @@ __git_main ()
>  
>  	local expansion=$(__git_aliased_command "$command")
>  	if [ -n "$expansion" ]; then
> +		words[1]=$expansion
>  		completion_func="_git_${expansion//-/_}"
>  		declare -f $completion_func >/dev/null && $completion_func
>  	fi
> diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
> index 6b77968..9f6f0fa 100644
> --- a/contrib/completion/git-completion.zsh
> +++ b/contrib/completion/git-completion.zsh
> @@ -104,6 +104,7 @@ __git_zsh_bash_func ()
>  
>  	local expansion=$(__git_aliased_command "$command")
>  	if [ -n "$expansion" ]; then
> +		words[1]=$expansion
>  		completion_func="_git_${expansion//-/_}"
>  		declare -f $completion_func >/dev/null && $completion_func
>  	fi
Felipe Contreras· Apr 9, 2014, 20:36 UTC · re: Junio C Hamano · lore

Re: [PATCH 5/5] completion: fix completion of certain aliases

Junio C Hamano wrote:
Show 17 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Some commands need the first word to determine the actual action that is
> > being executed, however, the command is wrong when we use an alias, for
> > example 'alias.p=push', if we try to complete 'git p origin ', the
> > result would be wrong because __git_complete_remote_or_refspec() doesn't
> > know where it come from.
> >
> > So let's override words[1], so the alias 'p' is override by the actual
> > command, 'push'.
> >
> > Reported-by: Aymeric Beaumet <aymeric.beaumet@gmail.com>
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> 
> Does "some commands" above refer to anything that uses
> __git_complete_remote_or_refspec, or is the set of commands larger than that?
For this particular issue, yes, the former.
> But perhaps we muck with the contents of words[] in a similar way in many
> different places in the existing completion code often enough that such an
> attempt not to touch the words[] array does not buy us much safety anyway.  I
> didn't check (and that is why I am asking with "I am wondering...").

The 'words' array is already messed up and not used correctly, so I wouldn't worry too much about this patch messing it more (I don't see how that can be).

For example:
  % git --git-dir=$PWD/.git fetch or<tab>
-- 
Felipe Contreras

← back to recent threads