All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCHv2 09/21] completion: respect 'git --git-dir=<path>' when listing remote refs
Date: Fri,  3 Feb 2017 03:48:17 +0100	[thread overview]
Message-ID: <20170203024829.8071-10-szeder.dev@gmail.com> (raw)
In-Reply-To: <20170203024829.8071-1-szeder.dev@gmail.com>

In __git_refs() the git commands listing refs, both short and full,
from a given remote repository are run without giving them the path to
the git repository which might have been specified on the command line
via 'git --git-dir=<path>'.  This is bad, those git commands should
access the 'refs/remotes/<remote>/' hierarchy or the remote and
credentials configuration in that specified repository.

Use the __gitdir() helper only to find the path to the .git directory
and pass the resulting path to the 'git ls-remote' and 'for-each-ref'
executions that list remote refs.  While modifying that 'for-each-ref'
line, remove the superfluous disambiguating doubledash.

Don't use __gitdir() to check that the given remote is on the file
system: basically it performs only a single if statement for us at the
considerable cost of fork()ing a subshell for a command substitution.
We are better off to perform all the necessary checks of the remote in
__git_refs().

Though __git_refs() was the last remaining callsite that passed a
remote to __gitdir(), don't delete __gitdir()'s remote-handling part
yet, just in case some users' custom completion scriptlets depend on
it.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 22 +++++++++++++++++-----
 t/t9902-completion.sh                  |  4 ++--
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 295f6de24..7d7e8b9d9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -342,9 +342,21 @@ __git_tags ()
 #    'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
 {
-	local i hash dir="$(__gitdir "${1-}")" track="${2-}"
+	local i hash dir="$(__gitdir)" track="${2-}"
+	local list_refs_from=path remote="${1-}"
 	local format refs pfx
-	if [ -d "$dir" ]; then
+
+	if [ -n "$remote" ]; then
+		if [ -d "$remote/.git" ]; then
+			dir="$remote/.git"
+		elif [ -d "$remote" ]; then
+			dir="$remote"
+		else
+			list_refs_from=remote
+		fi
+	fi
+
+	if [ "$list_refs_from" = path ] && [ -d "$dir" ]; then
 		case "$cur" in
 		refs|refs/*)
 			format="refname"
@@ -381,7 +393,7 @@ __git_refs ()
 	fi
 	case "$cur" in
 	refs|refs/*)
-		git ls-remote "$dir" "$cur*" 2>/dev/null | \
+		git --git-dir="$dir" ls-remote "$remote" "$cur*" 2>/dev/null | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -391,8 +403,8 @@ __git_refs ()
 		;;
 	*)
 		echo "HEAD"
-		git for-each-ref --format="%(refname:short)" -- \
-			"refs/remotes/$dir/" 2>/dev/null | sed -e "s#^$dir/##"
+		git --git-dir="$dir" for-each-ref --format="%(refname:short)" \
+			"refs/remotes/$remote/" 2>/dev/null | sed -e "s#^$remote/##"
 		;;
 	esac
 }
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7667baabf..6e64cd6ba 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -486,7 +486,7 @@ test_expect_success '__git_refs - configured remote - full refs' '
 	test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - configured remote - repo given on the command line' '
+test_expect_success '__git_refs - configured remote - repo given on the command line' '
 	cat >expected <<-EOF &&
 	HEAD
 	branch-in-other
@@ -501,7 +501,7 @@ test_expect_failure '__git_refs - configured remote - repo given on the command
 	test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - configured remote - full refs - repo given on the command line' '
+test_expect_success '__git_refs - configured remote - full refs - repo given on the command line' '
 	cat >expected <<-EOF &&
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
-- 
2.11.0.555.g967c1bcb3


  parent reply	other threads:[~2017-02-03  2:50 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03  2:48 [PATCHv2 00/21] completion: various __gitdir()-related improvements SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 01/21] completion: improve __git_refs()'s in-code documentation SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 02/21] completion tests: don't add test cruft to the test repository SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 03/21] completion tests: make the $cur variable local to the test helper functions SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 04/21] completion tests: consolidate getting path of current working directory SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 05/21] completion tests: check __gitdir()'s output in the error cases SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 06/21] completion tests: add tests for the __git_refs() helper function SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 07/21] completion: ensure that the repository path given on the command line exists SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 08/21] completion: fix most spots not respecting 'git --git-dir=<path>' SZEDER Gábor
2017-02-03  2:48 ` SZEDER Gábor [this message]
2017-02-03  2:48 ` [PATCHv2 10/21] completion: list refs from remote when remote's name matches a directory SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 11/21] completion: don't list 'HEAD' when trying refs completion outside of a repo SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 12/21] completion: list short refs from a remote given as a URL SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 13/21] completion: don't offer commands when 'git --opt' needs an argument SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 14/21] completion: fix completion after 'git -C <path>' SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 15/21] rev-parse: add '--absolute-git-dir' option SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 16/21] completion: respect 'git -C <path>' SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 17/21] completion: don't use __gitdir() for git commands SZEDER Gábor
2017-02-13 19:20   ` [PATCH] completion: restore removed line continuating backslash SZEDER Gábor
2017-02-13 20:45     ` Junio C Hamano
2017-02-15  1:35       ` SZEDER Gábor
2017-02-15  2:41         ` Junio C Hamano
2017-02-15 13:06           ` SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 18/21] completion: consolidate silencing errors from git commands SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 19/21] completion: don't guard git executions with __gitdir() SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 20/21] completion: extract repository discovery from __gitdir() SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 21/21] completion: cache the path to the repository SZEDER Gábor
2017-02-06 21:29 ` [PATCHv2 00/21] completion: various __gitdir()-related improvements Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170203024829.8071-10-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.