# [PATCH 0/5] Fixes 8 messages from 2014-04-09 to 2014-04-09. Participants: Felipe Contreras, Junio C Hamano. Thread: https://gitlist.dev/t/36380 ## Felipe Contreras, 2014-04-09 18:49 Subject: [PATCH 0/5] Fixes Message-ID: <1397069404-7451-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1397069404-7451-1-git-send-email-felipe.contreras%40gmail.com ``` 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, 2014-04-09 18:50 Subject: [PATCH 1/5] remote-helpers: allow all tests running from any dir Message-ID: <1397069404-7451-2-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1397069404-7451-2-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1397069404-7451-1-git-send-email-felipe.contreras@gmail.com> ``` 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 --- contrib/remote-helpers/test-hg-bidi.sh | 3 ++- contrib/remote-helpers/test-hg-hg-git.sh | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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, 2014-04-09 18:50 Subject: [PATCH 2/5] remote-hg: always normalize paths Message-ID: <1397069404-7451-3-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1397069404-7451-3-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1397069404-7451-1-git-send-email-felipe.contreras@gmail.com> ``` Apparently Mercurial can have paths such as 'foo//bar', so normalize all paths. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-hg | 1 + 1 file changed, 1 insertion(+) 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, 2014-04-09 18:50 Subject: [PATCH 3/5] remote-bzr: add support for older versions Message-ID: <1397069404-7451-4-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1397069404-7451-4-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1397069404-7451-1-git-send-email-felipe.contreras@gmail.com> ``` Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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, 2014-04-09 18:50 Subject: [PATCH 4/5] remote-bzr: include authors field in pushed commits Message-ID: <1397069404-7451-5-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1397069404-7451-5-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1397069404-7451-1-git-send-email-felipe.contreras@gmail.com> ``` From: dequis Tests-by: Felipe Contreras Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 2 ++ contrib/remote-helpers/test-bzr.sh | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) 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 " && + git remote add bzr "bzr::../bzrrepo" && + git push bzr master + ) && + + ( + cd bzrrepo && + bzr log | grep "^author: " > ../actual + ) && + + echo "author: John Doe " > expected && + test_cmp expected actual +' + test_done -- 1.9.1+fc1 ``` ## Felipe Contreras, 2014-04-09 18:50 Subject: [PATCH 5/5] completion: fix completion of certain aliases Message-ID: <1397069404-7451-6-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1397069404-7451-6-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1397069404-7451-1-git-send-email-felipe.contreras@gmail.com> ``` 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 Signed-off-by: Felipe Contreras --- 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 -- 1.9.1+fc1 ``` ## Junio C Hamano, 2014-04-09 19:33 Subject: Re: [PATCH 5/5] completion: fix completion of certain aliases Message-ID: URL: https://gitlist.dev/e/xmqqa9bu1enk.fsf%40gitster.dls.corp.google.com In-Reply-To: <1397069404-7451-6-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras 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 > Signed-off-by: Felipe Contreras > --- 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]. > 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, 2014-04-09 20:36 Subject: Re: [PATCH 5/5] completion: fix completion of certain aliases Message-ID: <5345af56dec8c_537810af310b9@nysa.notmuch> URL: https://gitlist.dev/e/5345af56dec8c_537810af310b9%40nysa.notmuch In-Reply-To: ``` Junio C Hamano wrote: > Felipe Contreras 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 > > Signed-off-by: Felipe Contreras > > --- > > 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 -- Felipe Contreras ```