Re: [PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:04:41 

Paul Tan [off-list ref] writes:

error_on_no_merge_candidates() does not consider the case where "$#"
includes command-line flags that are passed to git-fetch.

As such, when the current branch has no configured upstream, and there
are no merge candidates because of that, git-pull --all erroneously reports
that we are pulling from "--all", as it believes that the first argument
is the remote name.
Interesting.

I do not think "pull [origin] --all" makes much sense for the same
reason why we error out when you say "pull [origin] --tag", so I am
not sure "There is no tracking information" is the right diag we
would want to give the user, but I agree that "--all" is not a
remote name.

Does the same comment as 11/12 applies to this as well?
quoted hunk
Add a failing test that shows this case.

Reported-by: Stephen Robin <redacted>
Signed-off-by: Paul Tan <redacted>
---

Notes:
    * Added this test to the patch series.

 t/t5520-pull.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index d97a575..b93b735 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -153,6 +153,18 @@ test_expect_success 'fail if no configuration for current branch' '
 	test `cat file` = file
 '
 
+test_expect_failure 'pull --all: fail if no configuration for current branch' '
+	git remote add test_remote . &&
+	test_when_finished "git remote remove test_remote" &&
+	git checkout -b test copy^ &&
+	test_when_finished "git checkout -f copy && git branch -D test" &&
+	test_config branch.test.remote test_remote &&
+	test `cat file` = file &&
+	test_must_fail git pull --all 2>out &&
+	test_i18ngrep "There is no tracking information" out &&
+	test `cat file` = file
+'
+
 test_expect_success 'fail if upstream branch does not exist' '
 	git checkout -b test copy^ &&
 	test_when_finished "git checkout -f copy && git branch -D test" &&

Re: [PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream

From: Paul Tan <hidden>
Date: 2016-06-15 23:04:41 

Hi,

On Fri, May 8, 2015 at 1:38 AM, Junio C Hamano [off-list ref] wrote:
Paul Tan [off-list ref] writes:
quoted
error_on_no_merge_candidates() does not consider the case where "$#"
includes command-line flags that are passed to git-fetch.

As such, when the current branch has no configured upstream, and there
are no merge candidates because of that, git-pull --all erroneously reports
that we are pulling from "--all", as it believes that the first argument
is the remote name.
Interesting.

I do not think "pull [origin] --all" makes much sense for the same
reason why we error out when you say "pull [origin] --tag", so I am
not sure "There is no tracking information" is the right diag we
would want to give the user, but I agree that "--all" is not a
remote name.

Does the same comment as 11/12 applies to this as well?
This is actually only just one symptom of the problem of git-pull
assuming in many places that "$1" is the remote. There are other
possible ones, such as git pull --rebase failing silently at finding
the upstream branch to pass to git-rebase when git-fetch flags are
passed as well (see git-pull.sh:253), though crafting a test case for
that would be a bit more involved. Maybe if I have extra time to look
into it.

Then there is the problem where command line flag parsing stops when
an unidentified flag is encountered, assuming that it is for
git-fetch. E.g. git-pull --all --dry-run will pass --dry-run to
git-fetch, but will still run git-merge. Now, I haven't written this
up as a test case yet because I haven't the time yet, and it could be
argued that this is expected behavior, as the git-pull docs say that
options meant for git-fetch must come after git-pull.

However, this is ultimately really confusing for end-users, so I
propose that we just drop the distinction between git-pull's flags and
git-fetch's flags, and just parse them all at git-pull. This can be
done with the current git-pull.sh, but would be easier with the
superior C parse-options API.

Thanks,
Paul
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help