[PATCH v3 1/3] t4202: refactor test

[Mehul Jain]

Subsequent patches will want to reuse the 'signed' branch that the
'log --graph --show-signature' test creates and uses.

Split the set-up part into a test of its own, and make the existing
test into a separate one that only inspects the history on the 'signed'
branch. This way, it becomes clearer that tests added by subsequent
patches reuse the 'signed' branch in the same way.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t4202-log.sh | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 128ba93..ab66ee0 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -860,12 +860,15 @@ test_expect_success 'dotdot is a parent directory' '
test_cmp expect actual
 '
 
-test_expect_success GPG 'log --graph --show-signature' '
+test_expect_success GPG 'setup signed branch' '
test_when_finished "git reset --hard && git checkout master" &&
git checkout -b signed master &&
echo foo >foo &&
git add foo &&
-   git commit -S -m signed_commit &&
+   git commit -S -m signed_commit
+'
+
+test_expect_success GPG 'log --graph --show-signature' '
git log --graph --show-signature -n1 signed >actual &&
grep "^| gpg: Signature made" actual &&
grep "^| gpg: Good signature" actual
-- 
2.9.0.rc0.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: [PATCH v3 0/3] Introduce log.showSignature config variable

[Mehul Jain]

On Thu, Jun 23, 2016 at 12:02 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>
>> In patch 2/3 and 3/3, there are many tests which requires a branch
>> similar to that of "signed" branch, i.e. a branch with a commit having
>> GPG signature. So previously in v2, I created two new branches,
>> "test_sign" and "no_sign", which are identical to that of "signed"
>> branch. And with these branches, I wrote the tests in patch 2/3
>> and 3/3.
>>
>> As suggested by Eric [1], rather than creating new branches, I
>> can take advantage of "signed" branch which already exists.
>
> Yeah, I understand that part.  But you do not _need_ to do the split
> you do in 1/3 in order to reuse "signed".

If it's fine, then I think it would be OK to drop this 1/3. Without splitting
the 'log --graph --show-signature' in two test will also serve the
purpose for the new test to use the signed branch.

Should I send a new patch series with 1/3 dropped or you can do
it manually at your end?

Thanks,
Mehul
--
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 v3 0/3] Introduce log.showSignature config variable

[Mehul Jain]

On Thu, Jun 23, 2016 at 2:01 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>
>> Add a new configuratation variable "log.showSignature" for git-log
>> and related commands. "log.showSignature=true" will enable user to
>> see GPG signature by default for git-log and related commands.
>>
>> Changes compared to v2:
>>   * A preparatory patch 1/3 has been introduced so that tests
>> in patches 2/3 and 3/3 can take advantage of it.
>
> It is unclear how this change allows the remainder to "take
> advanrage" to me.  Earlier, "signed" branch was created only when
> the GPG prerequisite is met and with this change the branch is
> always created, which is the only change as far as I can see.  But
> the tests that are added in 2 and 3 are all protected with the GPG
> prerequiste.
>
> Besides, the invocation of "git commit -S" after this change is no
> longer protected by the GPG prerequisite and it may even cause the
> 'setup' step to fail on a host without GPG.

I overlooked the GPG prerequisite when I created the "setup signed
branch" test in patch 1/3. I will send a patch to  rectify it ones
everyone agree with the idea behind this patch.

In patch 2/3 and 3/3, there are many tests which requires a branch
similar to that of "signed" branch, i.e. a branch with a commit having
GPG signature. So previously in v2, I created two new branches,
"test_sign" and "no_sign", which are identical to that of "signed"
branch. And with these branches, I wrote the tests in patch 2/3
and 3/3.

As suggested by Eric [1], rather than creating new branches, I
can take advantage of "signed" branch which already exists.
So, I created a new test to separate the creation of "signed" branch
from existing test "log --graph --show-signature". This was done
because I do not want new tests to depend on this test. If in future
someone changes this test then it will affect new tests introduced
in 2/3 and 3/3.

Now the new tests and existing one ("log --graph ... ") are using a
single branch "signed" to do there work.

If changing an existing test is not well justified here, then I can create
setup test for new tests only, without affecting the existing test.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/297648

Thanks,
Mehul
--
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 v3 2/3] log: add "--no-show-signature" command line option

[Mehul Jain]

If an user creates an alias with "--show-signature" early in command
line, e.g.
[alias] logss = log --show-signature

then there is no way to countermand it through command line.

Teach git-log and related commands about "--no-show-signature" command
line option. This will make "git logss --no-show-signature" run
without showing GPG signature.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 revision.c | 2 ++
 t/t4202-log.sh | 5 +
 2 files changed, 7 insertions(+)

diff --git a/revision.c b/revision.c
index d30d1c4..3546ff9 100644
--- a/revision.c
+++ b/revision.c
@@ -1871,6 +1871,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->notes_opt.use_default_notes = 1;
} else if (!strcmp(arg, "--show-signature")) {
revs->show_signature = 1;
+   } else if (!strcmp(arg, "--no-show-signature")) {
+   revs->show_signature = 0;
} else if (!strcmp(arg, "--show-linear-break") ||
   starts_with(arg, "--show-linear-break=")) {
if (starts_with(arg, "--show-linear-break="))
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index ab66ee0..93a82e9 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -893,6 +893,11 @@ test_expect_success GPG 'log --graph --show-signature for 
merged tag' '
grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPG '--no-show-signature overrides --show-signature' '
+   git log -1 --show-signature --no-show-signature signed >actual &&
+   ! grep "^gpg:" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
-- 
2.9.0.rc0.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

[PATCH v3 3/3] log: add log.showSignature configuration variable

[Mehul Jain]

Users may want to always use "--show-signature" while using git-log and
related commands.

When log.showSignature is set to true, git-log and related commands will
behave as if "--show-signature" was given to them.

Note that this config variable is meant to affect git-log, git-show,
git-whatchanged and git-reflog. Other commands like git-format-patch,
git-rev-list are not to be affected by this config variable.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 t/t4202-log.sh| 20 
 t/t7510-signed-commit.sh  |  7 +++
 4 files changed, 37 insertions(+)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 03f9580..bbb5adc 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -196,6 +196,10 @@ log.showRoot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
 
+log.showSignature::
+   If `true`, `git log` and related commands will act as if the
+   `--show-signature` option was passed to them.
+
 mailmap.*::
See linkgit:git-shortlog[1].
 
diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..7103217 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -33,6 +33,7 @@ static const char *default_date_mode = NULL;
 static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
+static int default_show_signature;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -119,6 +120,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev->abbrev_commit = default_abbrev_commit;
rev->show_root_diff = default_show_root;
rev->subject_prefix = fmt_patch_subject_prefix;
+   rev->show_signature = default_show_signature;
DIFF_OPT_SET(>diffopt, ALLOW_TEXTCONV);
 
if (default_date_mode)
@@ -409,6 +411,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
use_mailmap_config = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.showsignature")) {
+   default_show_signature = git_config_bool(var, value);
+   return 0;
+   }
 
if (grep_config(var, value, cb) < 0)
return -1;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 93a82e9..ecac186 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -898,6 +898,26 @@ test_expect_success GPG '--no-show-signature overrides 
--show-signature' '
! grep "^gpg:" actual
 '
 
+test_expect_success GPG 'log.showsignature=true behaves like --show-signature' 
'
+   test_config log.showsignature true &&
+   git log -1 signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
+test_expect_success GPG '--no-show-signature overrides log.showsignature=true' 
'
+   test_config log.showsignature true &&
+   git log -1 --no-show-signature signed >actual &&
+   ! grep "^gpg:" actual
+'
+
+test_expect_success GPG '--show-signature overrides log.showsignature=false' '
+   test_config log.showsignature false &&
+   git log -1 --show-signature signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 4177a86..6e839f5 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -210,4 +210,11 @@ test_expect_success GPG 'show lack of signature with 
custom format' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'log.showsignature behaves like --show-signature' '
+   test_config log.showsignature true &&
+   git show initial >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_done
-- 
2.9.0.rc0.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

[PATCH v3 1/3] t4202: refactor test

[Mehul Jain]

Separate the creation of 'signed' branch so that other tests can take
advantage of this branch.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t4202-log.sh | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 128ba93..ab66ee0 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -860,12 +860,15 @@ test_expect_success 'dotdot is a parent directory' '
test_cmp expect actual
 '
 
-test_expect_success GPG 'log --graph --show-signature' '
+test_expect_success 'setup signed branch' '
test_when_finished "git reset --hard && git checkout master" &&
git checkout -b signed master &&
echo foo >foo &&
git add foo &&
-   git commit -S -m signed_commit &&
+   git commit -S -m signed_commit
+'
+
+test_expect_success GPG 'log --graph --show-signature' '
git log --graph --show-signature -n1 signed >actual &&
grep "^| gpg: Signature made" actual &&
grep "^| gpg: Good signature" actual
-- 
2.9.0.rc0.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

[PATCH v3 0/3] Introduce log.showSignature config variable

[Mehul Jain]

Add a new configuratation variable "log.showSignature" for git-log
and related commands. "log.showSignature=true" will enable user to
see GPG signature by default for git-log and related commands.

Changes compared to v2:
* A preparatory patch 1/3 has been introduced so that tests
  in patches 2/3 and 3/3 can take advantage of it.
* Mistake regarding branch in [patch v2 2/2] has been
  corrected.
* Tight coupling between the tests in [patch v2 2/2] has
  been resovled.

I would like to thanks Eric Sunshine for his feedback on previous
series [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/297648 

Mehul Jain (3):
  t4202: refactoring of test
  log: add "--no-show-signature" command line option
  log: add log.showSignature configuration variable

 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 revision.c|  2 ++
 t/t4202-log.sh| 32 ++--
 t/t7510-signed-commit.sh  |  7 +++
 5 files changed, 49 insertions(+), 2 deletions(-)

-- 
2.9.0.rc0.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: [PATCH v2 2/2] log: add log.showSignature configuration variable

[Mehul Jain]

Hi Eric,

Thanks for your review.

On Sun, Jun 19, 2016 at 12:29 PM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Sat, Jun 18, 2016 at 8:25 AM, Mehul Jain <mehul.jain2...@gmail.com> wrote:
>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
>> @@ -900,6 +900,31 @@ test_expect_success GPG '--no-show-signature overrides 
>> --show-signature' '
>> +test_expect_success GPG 'log.showsignature=true behaves like 
>> --show-signature' '
>> +   git checkout -b test_sign master &&
>
> It appears that you copied the bulk of this test from the 'log --graph
> --show-signature' test and changed it to create a new branch named
> 'test_sign' rather than 'signed', however...
>
>> +   echo foo >foo &&
>> +   git add foo &&
>> +   git commit -S -m signed_commit &&
>> +   test_config log.showsignature true &&
>> +   git log -1 signed >actual &&
>
> ... you're incorrectly testing against the 'signed' branch rather than
> the 'test_sign' created specifically for this test.

Yes, I made a mistake here.

>> +   grep "gpg: Signature made" actual &&
>> +   grep "gpg: Good signature" actual
>> +'
>> +
>> +test_expect_success GPG '--no-show-signature overrides 
>> log.showsignature=true' '
>> +   test_config log.showsignature true &&
>> +   git log -1 --no-show-signature signed >actual &&
>> +   ! grep "^gpg:" actual
>> +'
>> +
>> +test_expect_success GPG '--show-signature overrides 
>> log.showsignature=false' '
>> +   test_when_finished "git reset --hard && git checkout master" &&
>
> So, in the first of these three new tests, you're setting up some
> state by creating and checking out a new branch named 'test_sign', and
> leaving that branch checked out while these three tests run, and
> finally use test_when_finished() in the last of the three tests to
> restore sanity (by returning to the 'master' branch) when that test
> exits.
>
> This is ugly and couples these three tests too tightly. It would be
> better to make each test more self-contained, not relying upon state
> left over from previous tests.
>
>> +   test_config log.showsignature false &&
>> +   git log -1 --show-signature signed >actual &&
>> +   grep "gpg: Signature made" actual &&
>> +   grep "gpg: Good signature" actual
>> +'
>
> In fact, the original 'log --graph --show-signature' test which
> created the 'signed' branch, the new --no-show-signature test added in
> patch 1/2, and the three new tests here could all just work against
> the same 'signed' branch. There is no need to create a new 'test_sign'
> branch for these three tests, or a 'nosign' branch for the patch 1/2
> test.
>
> Therefore, it probably would make more sense to add a new distinct
> 'setup signed' test (or just enhance the existing 'setup' test) which
> creates the 'signed' branch and update the original 'log --graph
> --show-signature' to take advantage of it, as well as each of the new
> tests introduced by this patch series. And since each test would
> mention 'signed' explicitly in its git-log invocation, there's no need
> to leave that branch checked out, so the setup test itself only needs
> test_when_finished() to ensure that the current branch is restored to
> 'master'.

Adding a new test 'setup signed' will work, where I will create a new
'signed' branch and use that branch in new --no-show-signature test
introduced in patch 1/2, and the three tests in current patch 2/2. Also by
creating a preparatory patch for this series, I will modify the 'log --graph
--show-signature' test, so that it can also take advantage of new 'setup
signed' test.

Though I'm wondering if whether there is a need to create the new 'setup
signed' test. In 'log --graph --show-signature' test, we already have the
'signed' branch, which could be used in the test introduced here. But this
will couple the tests, 'log --graph ...' and new ones, tightly. Because if in
future someone changes the 'log --graph ...' test, then there is a possibility
of new tests (introduced in patch 1/2 and 2/2) to fail. So creating a new test
for creation of 'signed' branch seems fair.

Thanks,
Mehul
--
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/2] log: add log.showSignature configuration variable

[Mehul Jain]

Users may want to always use "--show-signature" while using git-log and
related commands.

When log.showSignature is set to true, git-log and related commands will
behave as if "--show-signature" was given to them.

Note that this config variable is meant to affect git-log, git-show,
git-whatchanged and git-reflog. Other commands like git-format-patch,
git-rev-list are not to be affected by this config variable.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 t/t4202-log.sh| 25 +
 t/t7510-signed-commit.sh  |  7 +++
 4 files changed, 42 insertions(+)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 03f9580..bbb5adc 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -196,6 +196,10 @@ log.showRoot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
 
+log.showSignature::
+   If `true`, `git log` and related commands will act as if the
+   `--show-signature` option was passed to them.
+
 mailmap.*::
See linkgit:git-shortlog[1].
 
diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..7103217 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -33,6 +33,7 @@ static const char *default_date_mode = NULL;
 static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
+static int default_show_signature;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -119,6 +120,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev->abbrev_commit = default_abbrev_commit;
rev->show_root_diff = default_show_root;
rev->subject_prefix = fmt_patch_subject_prefix;
+   rev->show_signature = default_show_signature;
DIFF_OPT_SET(>diffopt, ALLOW_TEXTCONV);
 
if (default_date_mode)
@@ -409,6 +411,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
use_mailmap_config = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.showsignature")) {
+   default_show_signature = git_config_bool(var, value);
+   return 0;
+   }
 
if (grep_config(var, value, cb) < 0)
return -1;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 02384a3..63ed863 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -900,6 +900,31 @@ test_expect_success GPG '--no-show-signature overrides 
--show-signature' '
! grep "^gpg:" actual
 '
 
+test_expect_success GPG 'log.showsignature=true behaves like --show-signature' 
'
+   git checkout -b test_sign master &&
+   echo foo >foo &&
+   git add foo &&
+   git commit -S -m signed_commit &&
+   test_config log.showsignature true &&
+   git log -1 signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
+test_expect_success GPG '--no-show-signature overrides log.showsignature=true' 
'
+   test_config log.showsignature true &&
+   git log -1 --no-show-signature signed >actual &&
+   ! grep "^gpg:" actual
+'
+
+test_expect_success GPG '--show-signature overrides log.showsignature=false' '
+   test_when_finished "git reset --hard && git checkout master" &&
+   test_config log.showsignature false &&
+   git log -1 --show-signature signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 4177a86..6e839f5 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -210,4 +210,11 @@ test_expect_success GPG 'show lack of signature with 
custom format' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'log.showsignature behaves like --show-signature' '
+   test_config log.showsignature true &&
+   git show initial >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_done
-- 
2.9.0.rc0.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

[PATCH v2 1/2] log: add "--no-show-signature" command line option

[Mehul Jain]

If an user creates an alias with "--show-signature" in command line,
e.g.
[alias] logss = log --show-signature

then there is no way to countermand it through command line.

Teach git-log and related commands about "--no-show-signature" command
line option. This will make "git logss --no-show-signature" run
without showing GPG signature.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 revision.c |  2 ++
 t/t4202-log.sh | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/revision.c b/revision.c
index d30d1c4..3546ff9 100644
--- a/revision.c
+++ b/revision.c
@@ -1871,6 +1871,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->notes_opt.use_default_notes = 1;
} else if (!strcmp(arg, "--show-signature")) {
revs->show_signature = 1;
+   } else if (!strcmp(arg, "--no-show-signature")) {
+   revs->show_signature = 0;
} else if (!strcmp(arg, "--show-linear-break") ||
   starts_with(arg, "--show-linear-break=")) {
if (starts_with(arg, "--show-linear-break="))
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 128ba93..02384a3 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -890,6 +890,16 @@ test_expect_success GPG 'log --graph --show-signature for 
merged tag' '
grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPG '--no-show-signature overrides --show-signature' '
+   test_when_finished "git reset --hard && git checkout master" &&
+   git checkout -b nosign master &&
+   echo foo >foo &&
+   git add foo &&
+   git commit -S -m signed_commit &&
+   git log -1 --show-signature --no-show-signature nosign >actual &&
+   ! grep "^gpg:" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
-- 
2.9.0.rc0.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

[PATCH v2 0/2] Introduce log.showSignature config variable

[Mehul Jain]

Add a new configuratation variable "log.showSignature" for git-log and
related commands. "log.showSignature=true" will enable user to see GPG signature
by default for git-log and related commands.

Changes:
* Order of patches is reversed as suggested by Junio[1].
* A new test has been introduced for "--no-show-signature"
  option.

Previous patch: http://thread.gmane.org/gmane.comp.version-control.git/296474

[1]: http://thread.gmane.org/gmane.comp.version-control.git/296474/focus=296476

Mehul Jain (2):
  log: add "--no-show-signature" command line option
  log: add log.showSignature configuration variable

 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 revision.c|  2 ++
 t/t4202-log.sh| 35 +++
 t/t7510-signed-commit.sh  |  7 +++
 5 files changed, 54 insertions(+)

-- 
2.9.0.rc0.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: [PATCH 2/2] log: "--no-show-signature" commmand-line option

[Mehul Jain]

On Tue, Jun 7, 2016 at 12:20 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>
>> If "log.showSignature=true", then there is no way to override it using
>> command line switch.
>>
>> Teach git-log and related commands about "--no-showSignature" command
>> line option.
>
> Doesn't that suggest that 1/2 alone will cause users problems?  The
> users can by mistake set the configuration variable and there is no
> way for them to override it from the command line.
>
> If you swap the order of the two patches, the topic makes more
> sense.  I.e.
>
> [1/2] log: add "--no-show-signature" command line option
>
> makes "git log --show-signature --no-show-signature" to run without
> GPG checks, which by itself is a worthy change.  And then
>
> [2/2] log: add log.showSignature configuration variable
>
> makes revs->show_signature default to the configured value, instead
> of always initializing it to false.

Yes, it does make sense to swap the order of the patches.
I will do a re-roll soon.

Thanks,
Mehul
--
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/2] log: add "log.showsignature" configuration variable

[Mehul Jain]

People may want to always use "--show-signature" while using "git log"
and related commands.

When log.showSignature is set to true, "git log" and related commands
will behave as if "--show-signature" was given to them.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 t/t4202-log.sh| 19 +++
 t/t7510-signed-commit.sh  |  7 +++
 4 files changed, 36 insertions(+)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 03f9580..bbb5adc 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -196,6 +196,10 @@ log.showRoot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
 
+log.showSignature::
+   If `true`, `git log` and related commands will act as if the
+   `--show-signature` option was passed to them.
+
 mailmap.*::
See linkgit:git-shortlog[1].
 
diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..7103217 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -33,6 +33,7 @@ static const char *default_date_mode = NULL;
 static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
+static int default_show_signature;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -119,6 +120,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev->abbrev_commit = default_abbrev_commit;
rev->show_root_diff = default_show_root;
rev->subject_prefix = fmt_patch_subject_prefix;
+   rev->show_signature = default_show_signature;
DIFF_OPT_SET(>diffopt, ALLOW_TEXTCONV);
 
if (default_date_mode)
@@ -409,6 +411,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
use_mailmap_config = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.showsignature")) {
+   default_show_signature = git_config_bool(var, value);
+   return 0;
+   }
 
if (grep_config(var, value, cb) < 0)
return -1;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 128ba93..3e4a4ac 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -890,6 +890,25 @@ test_expect_success GPG 'log --graph --show-signature for 
merged tag' '
grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPG 'log.showsignature=true behaves like --show-signature' 
'
+   git checkout -b test_sign master &&
+   echo foo >foo &&
+   git add foo &&
+   git commit -S -m signed_commit &&
+   test_config log.showsignature true &&
+   git log -1 signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
+test_expect_success GPG '--show-signature overrides log.showsignature=false' '
+   test_when_finished "git reset --hard && git checkout master" &&
+   test_config log.showsignature false &&
+   git log -1 --show-signature signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 4177a86..6e839f5 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -210,4 +210,11 @@ test_expect_success GPG 'show lack of signature with 
custom format' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'log.showsignature behaves like --show-signature' '
+   test_config log.showsignature true &&
+   git show initial >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_done
-- 
2.9.0.rc0.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

[PATCH 2/2] log: "--no-show-signature" commmand-line option

[Mehul Jain]

If "log.showSignature=true", then there is no way to override it using
command line switch.

Teach git-log and related commands about "--no-showSignature" command
line option.

Note that introduction of "--no-show-signature" is meant to tackle
the above mentioned problem for the following commands: git-log,
git-show, git-whatchanged and git-reflog. It does not suggest that
we need "log.showSignature" config variable to affect other git
commands, as currently "log.showSignature" is only meant to affect
git-log, git-show, git-whatchanged and git-reflog.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 revision.c | 2 ++
 t/t4202-log.sh | 6 ++
 2 files changed, 8 insertions(+)

diff --git a/revision.c b/revision.c
index d30d1c4..3546ff9 100644
--- a/revision.c
+++ b/revision.c
@@ -1871,6 +1871,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->notes_opt.use_default_notes = 1;
} else if (!strcmp(arg, "--show-signature")) {
revs->show_signature = 1;
+   } else if (!strcmp(arg, "--no-show-signature")) {
+   revs->show_signature = 0;
} else if (!strcmp(arg, "--show-linear-break") ||
   starts_with(arg, "--show-linear-break=")) {
if (starts_with(arg, "--show-linear-break="))
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 3e4a4ac..026808e 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -901,6 +901,12 @@ test_expect_success GPG 'log.showsignature=true behaves 
like --show-signature' '
grep "gpg: Good signature" actual
 '
 
+test_expect_success GPG '--no-show-signature overrides log.showsignature=true' 
'
+   test_config log.showsignature true &&
+   git log -1 --no-show-signature signed >actual &&
+   ! grep "^gpg:" actual
+'
+
 test_expect_success GPG '--show-signature overrides log.showsignature=false' '
test_when_finished "git reset --hard && git checkout master" &&
test_config log.showsignature false &&
-- 
2.9.0.rc0.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

[PATCH 0/2] Introduce "log.showSignature" config variable

[Mehul Jain]

Add a new configuratation variable "log.showSignature" for git-log and
related commands. This variable will enable user to see GPG signature
by default.

[Patch 1/2] 
Introduce the config variable along with some tests.

[Patch 2/2] 
Tackles the problem: what if user wants to disable the
setting of "log.showSignature=true" using a command line
switch.

* Thanks Junio, Jeff and Remi for helping in reference patch.

Previous reference patch: 
http://thread.gmane.org/gmane.comp.version-control.git/295649

Mehul Jain (2):
  log: add "log.showsignature" configuration variable
  log: "--no-show-signature" commmand-line option

 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 revision.c|  2 ++
 t/t4202-log.sh| 25 +
 t/t7510-signed-commit.sh  |  7 +++
 5 files changed, 44 insertions(+)

-- 
2.9.0.rc0.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: [RFC/PATCH 2/2] log: add "--no-show-signature" command line option

[Mehul Jain]

On Thu, May 26, 2016 at 10:52 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Jeff King <p...@peff.net> writes:
>
>> On Thu, May 26, 2016 at 06:36:47PM +0530, Mehul Jain wrote:
>>
>>> If "log.showsignature=true", then there is no way to override it using
>>> command line switch.
>>>
>>> Teach "git log" and "git show" about "--no-show-signature" command line
>>> option.
>>
>> I think this is teaching all of the revision machinery about it (which
>> is a good thing).
>
> I agree that the proposed commit log message should be updated to
> say so.
>
> Because we do not want .showsignature configuration to affect
> rev-list nor format-patch, and we will not make "--show-sig" the
> default for them either.  From that point of view, there is no
> reason for them to know about the "--no-show-signature" option.
>
> The only reason why teaching the "--no-show-signature" option to
> these commands is a good idea is because it would help people who
> create an alias with "--show-sig" in early part of the command line,
> e.g.
>
> [alias] fp = format-patch --show-signature
>
> by allowing them to countermand with --no-show-signature, i.e.
>
> $ git fp --no-show-signature ...
>
> If we are updating the log message in the final submission of this
> patch, we'd want it to be clear that the presence of this option is
> not an excuse to introduce .showsignature that affects rev-list
> later to make sure we do not have to waste our time rejecting such a
> patch in the future.

Currently, with the [patch 1/2], only git-show, git-log, git-whatchanged
and git-reflog are able to learn about log.showsignature config variable.
But commands which will learn about "--no-show-signature" with
[patch 2/2] are notably a super-set of above mentioned commands.
Introduction of this option should not give an impression that we might
need log.showSignature for commands like git-format-patch etc, and
it will definitely be a wise decision to convey the same in the commit
message of this patch. I will do the necessary change.

Just out of curiosity, I was thinking that we might be able to teach
"--no-show-signature" option only to git-show, git-log, git-whatchanged
and git-reflog. To do this we can introduce a new member
"no_show_signature" in struct rev_info, and use this variable further
to modify the value of value of "rev.show_signature" after init_revision()
is called. This way we can selectively decide which commands should
learn about "--no-show-signature". This may be a bad idea because
we will have two variables in rev_info, for option --[no]-show-signature.
Any thoughts?

Thanks,
Mehul
--
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: [RFC/PATCH 1/2] log: add "log.showsignature" configuration variable

[Mehul Jain]

On Thu, May 26, 2016 at 10:29 PM, Jeff King <p...@peff.net> wrote:
> On Thu, May 26, 2016 at 06:36:46PM +0530, Mehul Jain wrote:
> The documentation here mentions "log" and "show". But I think this will
> affect other programs, too, including "whatchanged" and "reflog". Those
> ones are probably good, but the documentation is a little misleading (I
> think other options just say "git-log and related commands" or
> something).

Yes, the documentation is misleading. As you have mentioned, this
config variable will affect git-log, git-show, git-whatchanged and git-reflog.
I will mention them in the documentation.

>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
>> index 128ba93..36be9a1 100755
>> --- a/t/t4202-log.sh
>> +++ b/t/t4202-log.sh
>> @@ -890,6 +890,25 @@ test_expect_success GPG 'log --graph --show-signature 
>> for merged tag' '
>>   grep "^| | gpg: Good signature" actual
>>  '
>>
>> +test_expect_success GPG 'log.showsignature=true behaves like 
>> --show-signature' '
>> + git checkout -b test_sign master &&
>> + echo foo >foo &&
>> + git add foo &&
>> + git commit -S -m signed_commit &&
>> + test_config log.showsignature true &&
>> + git log -1 signed >actual &&
>> + test_i18ngrep "gpg: Signature made" actual &&
>> + test_i18ngrep "gpg: Good signature" actual
>> +'
>
> You can see in the context that we do not use test_i18ngrep for finding
> gpg output in existing tests. I'm not sure if the new tests should be
> consistent, or if they should be changed to use test_i18ngrep. I don't
> think it's actually doing anything here, though. It's used with a
> git-specific GETTEXT_POISON flag that tweaks the output generated by
> git, but not by sub-programs like gpg.

There was no real motivation behind usage of test_i18ngrep. Certainly,
usage of grep will fit in the context.

>> +test_expect_success GPG '--show-signature overrides 
>> log.showsignature=false' '
>> + test_when_finished "git reset --hard && git checkout master" &&
>> + git config log.showsignature false &&
>
> Should this be test_config?

Noted.

Thanks,
Mehul
--
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: [RFC/PATCH 2/2] log: add "--no-show-signature" command line option

[Mehul Jain]

Hi,

Thanks for your input.

On Thu, May 26, 2016 at 10:02 PM, Jeff King <p...@peff.net> wrote:
> On Thu, May 26, 2016 at 06:36:47PM +0530, Mehul Jain wrote:
>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
>> index 36be9a1..ea24259 100755
>> --- a/t/t4202-log.sh
>> +++ b/t/t4202-log.sh
>> @@ -901,6 +901,13 @@ test_expect_success GPG 'log.showsignature=true behaves 
>> like --show-signature' '
>>   test_i18ngrep "gpg: Good signature" actual
>>  '
>>
>> +test_expect_success GPG '--no-show-signature overrides 
>> log.showsignature=true' '
>> + git config log.showsignature true &&
>> + git log -1 --no-show-signature signed >actual &&
>> + test "$(test_i18ngrep "gpg: Signature made" actual)" = "" &&
>> + test "$(test_i18ngrep "gpg: Good signature" actual)" = ""
>> +'
>
> Perhaps it would be more robust to simply grep for "gpg:". We should not
> be seeing any gpg-related lines in the output. It probably isn't that
> big a deal in practice, though. If the output from gpg changes, this
> test could report a false success, but all of the other nearby tests
> would show a breakage, so somebody would probably notice.

That's a very good point. I will make the changes accordingly.

Thanks,
Mehul
--
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: [RFC/PATCH 1/2] log: add "log.showsignature" configuration variable

[Mehul Jain]

On Thu, May 26, 2016 at 9:13 PM, Remi Galan Alfonso
 wrote:
> Sorry, I should have made explicit what went through my mind.
> "When log.showsignature set true" doesn't sound right to me, while
> "When log.showsignature is set to true" sounds better, however not
> being a native english speaker maybe it's just me being wrong.

I think "When log.showsignature is set to true" is better.
The one that I phrased does sound bit strange. I also
not being a native English speaker, have a history of making
grammatical mistakes. :)

Thanks,
Mehul
--
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: [RFC/PATCH 1/2] log: add "log.showsignature" configuration variable

[Mehul Jain]

Hi Remi,

Thanks for your input.

On Thu, May 26, 2016 at 7:12 PM, Remi Galan Alfonso
<remi.galan-alfo...@ensimag.grenoble-inp.fr> wrote:
> Hi Mehul,
>
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>> When log.showsignature set true, "git log" and "git show" will behave
>
> 'When log.showsignature is set to true' ?

Pardon me, but I don't understand your question.
I think you are suggesting me to write
"When log.showsignature is set to true"
instead of
"When log.showsignature set true".

>> +test_expect_success GPG '--show-signature overrides 
>> log.showsignature=false' '
>> +test_when_finished "git reset --hard && git checkout master" &&
>> +git config log.showsignature false &&
>
> Any specific reason as to why you don't use test_config like in the
> first test?

None, actually. It was just that I forgot to use test_config while
writing the tests. I will make changes  accordingly as test_config
automatically unset the config variable, which is necessary.

Thanks,
Mehul
--
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

[RFC/PATCH 2/2] log: add "--no-show-signature" command line option

[Mehul Jain]

If "log.showsignature=true", then there is no way to override it using
command line switch.

Teach "git log" and "git show" about "--no-show-signature" command line
option.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 revision.c | 2 ++
 t/t4202-log.sh | 7 +++
 2 files changed, 9 insertions(+)

diff --git a/revision.c b/revision.c
index d30d1c4..3546ff9 100644
--- a/revision.c
+++ b/revision.c
@@ -1871,6 +1871,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->notes_opt.use_default_notes = 1;
} else if (!strcmp(arg, "--show-signature")) {
revs->show_signature = 1;
+   } else if (!strcmp(arg, "--no-show-signature")) {
+   revs->show_signature = 0;
} else if (!strcmp(arg, "--show-linear-break") ||
   starts_with(arg, "--show-linear-break=")) {
if (starts_with(arg, "--show-linear-break="))
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 36be9a1..ea24259 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -901,6 +901,13 @@ test_expect_success GPG 'log.showsignature=true behaves 
like --show-signature' '
test_i18ngrep "gpg: Good signature" actual
 '
 
+test_expect_success GPG '--no-show-signature overrides log.showsignature=true' 
'
+   git config log.showsignature true &&
+   git log -1 --no-show-signature signed >actual &&
+   test "$(test_i18ngrep "gpg: Signature made" actual)" = "" &&
+   test "$(test_i18ngrep "gpg: Good signature" actual)" = ""
+'
+
 test_expect_success GPG '--show-signature overrides log.showsignature=false' '
test_when_finished "git reset --hard && git checkout master" &&
git config log.showsignature false &&
-- 
2.9.0.rc0.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

[RFC/PATCH 0/2] Introduce "log.showSignature" config variable

[Mehul Jain]

Add a new configuratation variable "log.showSignature" for git-log and
git-show. "log.showSignature=true" will enable user to see GPG signature
by default while using git-log and git-show.

[Patch 1/2] introduce the config variable along with some tests.
[Patch 2/2] tackles the problem: what if user wants to disable the
setting of "log.showSignature=true" using a command line
switch.

Previous discussion on this: 
http://thread.gmane.org/gmane.comp.version-control.git/295405

Mehul Jain (2):
  log: add "log.showsignature" configuration variable
  log: add "--no-show-signature" command line option

 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 revision.c|  2 ++
 t/t4202-log.sh| 26 ++
 t/t7510-signed-commit.sh  |  7 +++
 5 files changed, 45 insertions(+)

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

[RFC/PATCH 1/2] log: add "log.showsignature" configuration variable

[Mehul Jain]

People may want to always use "--show-signature" while using "git log"
or "git show".

When log.showsignature set true, "git log" and "git show" will behave
as "--show-signature" was given to them.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 t/t4202-log.sh| 19 +++
 t/t7510-signed-commit.sh  |  7 +++
 4 files changed, 36 insertions(+)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 03f9580..f39f800 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -196,6 +196,10 @@ log.showRoot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
 
+log.showSignature::
+   If `true`, `git log` and `git show` will act as if `--show-signature`
+   option was passed to them.
+
 mailmap.*::
See linkgit:git-shortlog[1].
 
diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..7103217 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -33,6 +33,7 @@ static const char *default_date_mode = NULL;
 static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
+static int default_show_signature;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -119,6 +120,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev->abbrev_commit = default_abbrev_commit;
rev->show_root_diff = default_show_root;
rev->subject_prefix = fmt_patch_subject_prefix;
+   rev->show_signature = default_show_signature;
DIFF_OPT_SET(>diffopt, ALLOW_TEXTCONV);
 
if (default_date_mode)
@@ -409,6 +411,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
use_mailmap_config = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.showsignature")) {
+   default_show_signature = git_config_bool(var, value);
+   return 0;
+   }
 
if (grep_config(var, value, cb) < 0)
return -1;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 128ba93..36be9a1 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -890,6 +890,25 @@ test_expect_success GPG 'log --graph --show-signature for 
merged tag' '
grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPG 'log.showsignature=true behaves like --show-signature' 
'
+   git checkout -b test_sign master &&
+   echo foo >foo &&
+   git add foo &&
+   git commit -S -m signed_commit &&
+   test_config log.showsignature true &&
+   git log -1 signed >actual &&
+   test_i18ngrep "gpg: Signature made" actual &&
+   test_i18ngrep "gpg: Good signature" actual
+'
+
+test_expect_success GPG '--show-signature overrides log.showsignature=false' '
+   test_when_finished "git reset --hard && git checkout master" &&
+   git config log.showsignature false &&
+   git log -1 --show-signature signed >actual &&
+   test_i18ngrep "gpg: Signature made" actual &&
+   test_i18ngrep "gpg: Good signature" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 4177a86..326dcc8 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -210,4 +210,11 @@ test_expect_success GPG 'show lack of signature with 
custom format' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'log.showsignature behaves like --show-signature' '
+   git config log.showsignature true &&
+   git show initial > actual &&
+   test_i18ngrep "gpg: Signature made" actual &&
+   test_i18ngrep "gpg: Good signature" actual
+'
+
 test_done
-- 
2.9.0.rc0.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: Please add a git config option to make --show-signature the default

[Mehul Jain]

Hi,

On Wed, May 25, 2016 at 9:28 AM, Austin English  wrote:
> I'll try
> to submit my own patch. In the meantime, it seems appropriate to file
> a bug so that others can have the opportunity to solve the problem if
> they're interested.

If you haven't started working on it and if no one else has picked it up
then I would like to try it out and submit a patch.

Thanks,
Mehul
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

[Mehul Jain]

On Mon, Apr 4, 2016 at 10:22 PM, Matthieu Moy
 wrote:
> I think it would be much simpler to drop the loop, and write instead
> something like (untested):

I tested it (with few minor changes), and worked fine.

test_autostash () {
OLDIFS=$IFS
IFS='='
set -- $*
IFS=$OLDIFS
expect=$1
cmd=$2
config_variable=$3
value=$4
test_expect_success "$cmd, $config_variable=$value" '
if [ "$value" = "" ]; then
test_unconfig $config_variable
else
test_config $config_variable $value
fi &&

git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&

if [ $expect = "ok" ]; then
git pull $cmd . copy &&
test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
else
test_must_fail git pull $cmd . copy 2>err &&
test_i18ngrep "uncommitted changes." err
fi
'
}

test_autostash ok '--rebase' rebase.autostash=true
test_autostash ok '--rebase --autostash' rebase.autostash=true
test_autostash ok '--rebase --autostash' rebase.autostash=false
test_autostash ok '--rebase --autostash' rebase.autostash=
test_autostash err '--rebase --no-autostash' rebase.autostash=true
test_autostash err '--rebase --no-autostash' rebase.autostash=false
test_autostash err '--rebase --no-autostash' rebase.autostash=
test_autostash ok '--autostash' pull.rebase=true
test_autostash err '--no-autostash' pull.rebase=true

Perhaps this looks better than the one with the loop. Even better than
the implementation in v2[1].

I think it would be wise to go with the above script for v3 (as I will
be doing a re-roll of the series[1]).

[1]: http://thread.gmane.org/gmane.comp.version-control.git/290596

Thanks,
Mehul
--
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 0/7] t5520: tests for --[no-]autostash option

[Mehul Jain]

On Mon, Apr 4, 2016 at 10:30 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Matthieu Moy <matthieu@grenoble-inp.fr> writes:
>
>> Mehul Jain <mehul.jain2...@gmail.com> writes:
>>
>>> -test_rebase_autostash () {
>>> +test_pull_autostash () {
>>>  git reset --hard before-rebase &&
>>>  echo dirty >new_file &&
>>>  git add new_file &&
>>> -git pull --rebase --autostash . copy &&
>>> +git pull $@ . copy &&
>>
>> Not strictly needed here, but I'd write "$@" (with the double-quotes)
>> which is the robust way to say "transmit all my arguments without
>> whitespace interpretation".
>
> Yes, these should be "$@" (with the double-quotes).

I will do a re-roll then.

Thanks,
Mehul
--
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 0/7] t5520: tests for --[no-]autostash option

[Mehul Jain]

On Mon, Apr 4, 2016 at 1:01 PM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>
>> -test_rebase_autostash () {
>> +test_pull_autostash () {
>>   git reset --hard before-rebase &&
>>   echo dirty >new_file &&
>>   git add new_file &&
>> - git pull --rebase --autostash . copy &&
>> + git pull $@ . copy &&
>
> Not strictly needed here, but I'd write "$@" (with the double-quotes)
> which is the robust way to say "transmit all my arguments without
> whitespace interpretation".
>
> I don't mind for this patch since there's no whitespace to interpret,
> but some people (sysadmins ;-) ) have the bad habit of writting $@, $*
> or "$*" in wrapper scripts and it breaks when you call them with spaces
> so it's better to take good habits IHMO.

Thanks for the suggestion, I will remember it. I'm relatively new to
shell and therefore didn't know much about the difference
between "$@" and $@, $*, "$*".

Now that I have read[1][2] about it, it won't be repeated.

[1]: 
http://unix.stackexchange.com/questions/41571/what-is-the-difference-between-and/94200#94200
[2]: 
http://unix.stackexchange.com/questions/131766/why-does-my-shell-script-choke-on-whitespace-or-other-special-characters

Thanks,
Mehul
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

[Mehul Jain]

On Mon, Apr 4, 2016 at 12:58 AM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Fri, Apr 1, 2016 at 6:27 AM, Mehul Jain <mehul.jain2...@gmail.com> wrote:
>> In test_autostash() there's a line
>>
>> echo test_cmp_rev HEAD^ copy &&
>>
>> Originally it should have been
>>
>> test_cmp_rev HEAD^ copy &&
>>
>> but this raise following error while testing
>>
>> ./t5520-pull.sh: 684: eval: diff -u: not found
>
> This is caused by the custom IFS=',\t=' which is still in effect when
> test_cmp_rev() is invoked. You need to restore IFS within the loop
> itself.

Thanks for pointing it out. I made a mistake by not considering
the consequences of setting IFS=',\t='. I tried it out again and
this time all tests passed perfectly.

I should  been more careful in the first place while playing
with IFS, but instead of that, I kept on thinking that there is some
other problem with the script which lead to me making foolish
changes in the script like putting an echo before "test_cmp_rev ...".

It was nice of you to take out some time and point it out :)

Also now that I have sent v2[1] of this series, which goes
in different direction as far as implementation of these tests
are concerned. I think the script now is useless (but I
learned a bit about shell while writing it).

Thanks,
Mehul
--
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 6/7] t5520: reduce commom lines of code

[Mehul Jain]

These two tests are almost similar and thus can be folded in a for-loop.

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index fb9f845..e12af96 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -298,15 +298,13 @@ test_expect_success 'pull --rebase --no-autostash & 
rebase.autostash unset' '
test_pull_autostash_fail --rebase --no-autostash
 '
 
-test_expect_success 'pull --autostash (without --rebase) should error out' '
-   test_must_fail git pull --autostash . copy 2>err &&
-   test_i18ngrep "only valid with --rebase" err
-'
-
-test_expect_success 'pull --no-autostash (without --rebase) should error out' '
-   test_must_fail git pull --no-autostash . copy 2>err &&
-   test_i18ngrep "only valid with --rebase" err
-'
+for autostash_flag in --autostash --no-autostash
+do
+   test_expect_success "pull $autostash_flag (without --rebase) is 
illegal" '
+   test_must_fail git pull $autostash_flag . copy 2>err &&
+   test_i18ngrep "only valid with --rebase" err
+   '
+done
 
 test_expect_success 'pull.rebase' '
git reset --hard before-rebase &&
-- 
2.7.1.340.g69eb491.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: [PATCH v2 6/7] t5520: reduce commom lines of code

[Mehul Jain]

On Sun, Apr 3, 2016 at 12:20 AM, Johannes Sixt <j...@kdbg.org> wrote:
> Am 02.04.2016 um 19:58 schrieb Mehul Jain:
>> +for i in --autostash --no-autostash
>> +do
>> +   test_expect_success "pull $i (without --rebase) is illegal" '
>> +   test_must_fail git pull $i . copy 2>err &&
>> +   test_i18ngrep "only valid with --rebase" err
>> +   '
>> +done
>
>
> Hm. If the implementation of test_expect_success uses the variable, too, its
> value is lost when the test snippet runs. Fortunately, it does not.
>
> You can make this code a bit more robust by using double-quotes around the
> test code so that $i is expanded before test_expect_success is evaluated.

I think that the current format is preferred over the one you suggest.
Here[1] Junio
has given a descriptive explanation.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/283350/focus=284769

Thanks,
Mehul
--
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 5/7] t5520: factor out common code

[Mehul Jain]

Three tests contains repetitive lines of code.

Factor out common code into test_pull_autostash_fail() and then call it in
these tests.

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 26 +++---
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index ac063c2..fb9f845 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -19,6 +19,14 @@ test_pull_autostash () {
test "$(cat file)" = "modified again"
 }
 
+test_pull_autostash_fail () {
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   test_must_fail git pull $@ . copy 2>err &&
+   test_i18ngrep "uncommitted changes." err
+}
+
 test_expect_success setup '
echo file >file &&
git add file &&
@@ -277,29 +285,17 @@ test_expect_success 'pull --rebase --autostash & 
rebase.autostash unset' '
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
test_config rebase.autostash true &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
-   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
+   test_pull_autostash_fail --rebase --no-autostash
 '
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '
test_config rebase.autostash false &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
-   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
+   test_pull_autostash_fail --rebase --no-autostash
 '
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
-   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
+   test_pull_autostash_fail --rebase --no-autostash
 '
 
 test_expect_success 'pull --autostash (without --rebase) should error out' '
-- 
2.7.1.340.g69eb491.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

[PATCH v2 7/7] t5520: test --[no-]autostash with pull.rebase=true

[Mehul Jain]

"--[no-]autostash" option for git-pull is only valid in rebase mode(
i.e. either --rebase should be used or pull.rebase=true). Existing
tests already check the cases when --rebase is used but fails to check
for pull.rebase=true case.

Add two new tests to check that --[no-]autostash option works with
pull.rebase=true.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index e12af96..bed75f5 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -314,6 +314,16 @@ test_expect_success 'pull.rebase' '
test new = "$(git show HEAD:file2)"
 '
 
+test_expect_success 'pull --autostash & pull.rebase=true' '
+   test_config pull.rebase true &&
+   test_pull_autostash --autostash
+'
+
+test_expect_success 'pull --no-autostash & pull.rebase=true' '
+   test_config pull.rebase true &&
+   test_pull_autostash_fail --no-autostash
+'
+
 test_expect_success 'branch.to-rebase.rebase' '
git reset --hard before-rebase &&
test_config branch.to-rebase.rebase true &&
-- 
2.7.1.340.g69eb491.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

[PATCH v2 6/7] t5520: reduce commom lines of code

[Mehul Jain]

These two tests are almost similar and thus can be folded in a for-loop.

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index fb9f845..e12af96 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -298,15 +298,13 @@ test_expect_success 'pull --rebase --no-autostash & 
rebase.autostash unset' '
test_pull_autostash_fail --rebase --no-autostash
 '
 
-test_expect_success 'pull --autostash (without --rebase) should error out' '
-   test_must_fail git pull --autostash . copy 2>err &&
-   test_i18ngrep "only valid with --rebase" err
-'
-
-test_expect_success 'pull --no-autostash (without --rebase) should error out' '
-   test_must_fail git pull --no-autostash . copy 2>err &&
-   test_i18ngrep "only valid with --rebase" err
-'
+for i in --autostash --no-autostash
+do
+   test_expect_success "pull $i (without --rebase) is illegal" '
+   test_must_fail git pull $i . copy 2>err &&
+   test_i18ngrep "only valid with --rebase" err
+   '
+done
 
 test_expect_success 'pull.rebase' '
git reset --hard before-rebase &&
-- 
2.7.1.340.g69eb491.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

[PATCH v2 4/7] t5520: factor out common code

[Mehul Jain]

Four tests contains repetitive lines of code.

Factor out common code into test_pull_autostash() and then call it in
these tests.

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 44 +++-
 1 file changed, 15 insertions(+), 29 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index d03cb84..ac063c2 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -9,6 +9,16 @@ modify () {
mv "$2.x" "$2"
 }
 
+test_pull_autostash () {
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull $@ . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+}
+
 test_expect_success setup '
echo file >file &&
git add file &&
@@ -247,46 +257,22 @@ test_expect_success '--rebase fails with multiple 
branches' '
 
 test_expect_success 'pull --rebase succeeds with dirty working directory and 
rebase.autostash set' '
test_config rebase.autostash true &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --rebase . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
+   test_pull_autostash --rebase
 '
 
 test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '
test_config rebase.autostash true &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --rebase --autostash . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
+   test_pull_autostash --rebase --autostash
 '
 
 test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '
test_config rebase.autostash false &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --rebase --autostash . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
+   test_pull_autostash --rebase --autostash
 '
 
-test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
+test_expect_success 'pull --rebase --autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --rebase --autostash . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
+   test_pull_autostash --rebase --autostash
 '
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
-- 
2.7.1.340.g69eb491.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

[PATCH v2 3/7] t5520: use better test to check stderr output

[Mehul Jain]

Checking stderr output using test_i18ncmp may lead to test failure as
some shells write trace output to stderr when run under 'set -x'.

Use test_i18ngrep instead of test_i18ncmp.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 9ee2218..d03cb84 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -317,15 +317,13 @@ test_expect_success 'pull --rebase --no-autostash & 
rebase.autostash unset' '
 '
 
 test_expect_success 'pull --autostash (without --rebase) should error out' '
-   test_must_fail git pull --autostash . copy 2>actual &&
-   echo "fatal: --[no-]autostash option is only valid with --rebase." 
>expect &&
-   test_i18ncmp actual expect
+   test_must_fail git pull --autostash . copy 2>err &&
+   test_i18ngrep "only valid with --rebase" err
 '
 
 test_expect_success 'pull --no-autostash (without --rebase) should error out' '
-   test_must_fail git pull --no-autostash . copy 2>actual &&
-   echo "fatal: --[no-]autostash option is only valid with --rebase." 
>expect &&
-   test_i18ncmp actual expect
+   test_must_fail git pull --no-autostash . copy 2>err &&
+   test_i18ngrep "only valid with --rebase" err
 '
 
 test_expect_success 'pull.rebase' '
-- 
2.7.1.340.g69eb491.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

[PATCH v2 1/7] t5520: use consistent capitalization in test titles

[Mehul Jain]

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 745e59e..5be39df 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -267,7 +267,7 @@ test_expect_success 'pull --rebase --autostash & 
rebase.autostash=true' '
test "$(cat file)" = "modified again"
 '
 
-test_expect_success 'pull --rebase --autostash & rebase.autoStash=false' '
+test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '
test_config rebase.autostash false &&
git reset --hard before-rebase &&
echo dirty >new_file &&
@@ -278,7 +278,7 @@ test_expect_success 'pull --rebase --autostash & 
rebase.autoStash=false' '
test "$(cat file)" = "modified again"
 '
 
-test_expect_success 'pull --rebase: --autostash & rebase.autoStash unset' '
+test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
-- 
2.7.1.340.g69eb491.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

[PATCH v2 2/7] t5520: ensure consistent test conditions

[Mehul Jain]

Test title says that tests are done with rebase.autostash unset,
but does not take any action to make sure that it is indeed unset.
This may lead to test failure if future changes somehow pollutes
the configuration globally.

Ensure consistent test conditions by explicitly unsetting
rebase.autostash.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 5be39df..9ee2218 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -279,6 +279,7 @@ test_expect_success 'pull --rebase --autostash & 
rebase.autostash=false' '
 '
 
 test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
+   test_unconfig rebase.autostash &&
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
@@ -307,6 +308,7 @@ test_expect_success 'pull --rebase --no-autostash & 
rebase.autostash=false' '
 '
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
+   test_unconfig rebase.autostash &&
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
-- 
2.7.1.340.g69eb491.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

[PATCH v2 0/7] t5520: tests for --[no-]autostash option

[Mehul Jain]

out --rebase) is illegal" '
+   test_must_fail git pull $i . copy 2>err &&
+   test_i18ngrep "only valid with --rebase" err
+   '
+done
 
 test_expect_success 'pull.rebase' '
git reset --hard before-rebase &&
@@ -318,22 +316,12 @@ test_expect_success 'pull.rebase' '
 
 test_expect_success 'pull --autostash & pull.rebase=true' '
test_config pull.rebase true &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --autostash . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
+   test_pull_autostash --autostash
 '
 
 test_expect_success 'pull --no-autostash & pull.rebase=true' '
test_config pull.rebase true &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   test_must_fail git pull --no-autostash . copy 2>err &&
-   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
+   test_pull_autostash_fail --no-autostash
 '
 
 test_expect_success 'branch.to-rebase.rebase' '


Mehul Jain (7):
  t5520: use consistent capitalization in test titles
  t5520: ensure consistent test conditions
  t5520: use better test to check stderr output
  t5520: factor out common code
  t5520: factor out common code
  t5520: reduce commom lines of code
  t5520: test --[no-]autostash with pull.rebase=true

 t/t5520-pull.sh | 102 +---
 1 file changed, 46 insertions(+), 56 deletions(-)

-- 
2.7.1.340.g69eb491.dirty

[1]:http://thread.gmane.org/gmane.comp.version-control.git/290134

Thanks,
Mehul
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

[Mehul Jain]

Hi Eric,

On Thu, Mar 31, 2016 at 2:01 AM, Eric Sunshine  wrote:
> One other possibility would be to make this all table-driven by
> collecting all of the above state information into a table and then
> feeding that into a function (either as its argument list or via
> stdin). For instance:
>
> test_autostash <<\-EOF
> ok,--rebase,rebase.autostash=true
> ok,--rebase --autostash,rebase.autostash=true
> ok,--rebase --autostash,rebase.autostash=false
> ok,--rebase --autostash,rebase.autostash=
> err,--rebase --no-autostash,rebase.autostash=true
> err,--rebase --no-autostash,rebase.autostash=false
> err,--rebase --no-autostash,rebase.autostash=
> ok,--autostash,pull.rebase=true
> err,--no-autostash,pull.rebase=true
>EOF
>
> The function would loop over the input, split each line apart by
> setting IFS=, and then run the test based upon the state information.
> "ok" means autostash is expected to succeed, and err means it is
> expected to fail. The function would want to specially recognize the
> "foo.bar=" in the last argument in order to invoke test_unconfig()
> rather than test_config().

I tried out this method also. Below is the script that I wrote for this:

---

test_autostash () {
OLDIFS=$IFS
IFS=',='
while read -r expect cmd config_variable value
do
test_expect_success "$cmd, $config_variable=$value" '
if [ "$value" = "" ]; then
test_unconfig $config_variable
else
test_config $config_variable $value
fi &&

git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&

if [ $expect = "ok" ]; then
git pull '$cmd' . copy &&
echo test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
else
test_must_fail git pull '$cmd' . copy 2>err &&
test_i18ngrep "uncommitted changes." err
fi
'
done
IFS=$OLDIFS
}


test_autostash <<-\EOF
ok,--rebase,rebase.autostash=true
ok,--rebase --autostash,rebase.autostash=true
ok,--rebase --autostash,rebase.autostash=false
ok,--rebase --autostash,rebase.autostash=
err,--rebase --no-autostash,rebase.autostash=true
err,--rebase --no-autostash,rebase.autostash=false
err,--rebase --no-autostash,rebase.autostash=
ok,--autostash,pull.rebase=true
err,--no-autostash,pull.rebase=true
EOF


---

Things worked out perfectly.

Unfortunately there was a strange behaviour that I noticed
and frankly I don't understand why it happened.

In test_autostash() there's a line

echo test_cmp_rev HEAD^ copy &&

Originally it should have been

test_cmp_rev HEAD^ copy &&

but this raise following error while testing

./t5520-pull.sh: 684: eval: diff -u: not found

I'm not able to understand why putting an "echo" before
test_cmp didn't raise the above error. This looks quite
strange. Any thoughts?

Though the above code works perfectly and can be used in
place of previous tests. Only problem remains is tests titles.
Currently with this script, test titles will be:

ok 21 - --rebase, rebase.autostash=true
ok 22 - --rebase --autostash, rebase.autostash=true
ok 23 - --rebase --autostash, rebase.autostash=false
ok 24 - --rebase --autostash, rebase.autostash=
ok 25 - --rebase --no-autostash, rebase.autostash=true
ok 26 - --rebase --no-autostash, rebase.autostash=false
ok 27 - --rebase --no-autostash, rebase.autostash=
ok 28 - --autostash, pull.rebase=true
ok 29 - --no-autostash, pull.rebase=true

Any thoughts/suggestions on them?

Thanks,
Mehul
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

[Mehul Jain]

Hi Eric,

Thanks for the reviews on this series.

On Wed, Mar 30, 2016 at 2:46 AM, Eric Sunshine  wrote:
> With the exception of the missing --rebase argument, this is exactly
> the same code as in test_rebase_autostash(), right? Rather than
> repeating this code yet again, it might be nice to augment that
> function to accept a (possibly) optional argument controlling whether
> --rebase is used.

Thanks for the idea. I have come up with something like this:

* Introduce two function test_pull() and test_pull_fail() in
the place of
  test_rebase_autostash() and test_rebase_no_autostash.()

  Using these functions we can easily re-write all the 6 tests which
  deals with combination of autostash and rebase.autostash. Plus
  these functions helped in writing two new tests which deals with
  combination of pull.rebase and autostash. Thus reducing the code
  base to simpler and fewer lines of code. Also I could re-write one
  of the old test to reduce the repetition with them.

Here are the functions and there implementations:

---

test_pull () {
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
git pull $@ . copy &&
test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
}

test_pull_fail () {
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
test_must_fail git pull $@ . copy 2>err &&
test_i18ngrep "uncommitted changes." err
}

test_expect_success 'pull --rebase succeeds with dirty working
directory and rebase.autostash set' '
test_config rebase.autostash true &&
test_pull --rebase
'

test_expect_success "pull --rebase --autostash & rebase.autostash=true" '
test_config rebase.autostash true &&
test_pull --rebase --autostash
'

test_expect_success "pull --rebase --autostash & rebase.autostash=false" '
test_config rebase.autostash false &&
test_pull --rebase --autostash
'

test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
test_pull --rebase --autostash
'

test_expect_success "pull --rebase --no-autostash & rebase.autostash=true" '
test_config rebase.autostash true &&
test_pull_fail --rebase --no-autostash
'

test_expect_success "pull --rebase --no-autostash & rebase.autostash=false" '
test_config rebase.autostash false &&
test_pull_fail --rebase --no-autostash
'

test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
test_pull_fail --rebase --no-autostash
'

test_expect_success 'pull --autostash & pull.rebase=true' '
test_config pull.rebase true &&
test_pull --autostash
'

test_expect_success 'pull --no-autostash & pull.rebase=true' '
test_config pull.rebase true &&
test_pull_fail --no-autostash
'
---

I'm sorry if this is bit difficult to digest without diff output. I
just wanted to
know if the above mention functions looks suitable to you.

Also I've read your comments on other patches of this series, I will make
changes accordingly ones above mention functions, tests looks fit for a
re-roll.

Thanks,
Mehul
--
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/5] t/t5520: test --[no-]autostash with pull.rebase=true

[Mehul Jain]

"--[no-]autostash" option for git-pull is only valid in rebase mode.
That is, either --rebase is used or pull.rebase=true. Existing tests
already check the cases when --rebase is used but fails to check for
pull.rebase=true case.

Add two new tests to check that --[no-]autostash option works with
pull.rebase=true.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 20 
 1 file changed, 20 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 2611170..4da9e52 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -316,6 +316,26 @@ test_expect_success 'pull.rebase' '
test new = "$(git show HEAD:file2)"
 '
 
+test_expect_success 'pull --autostash & pull.rebase=true' '
+   test_config pull.rebase true &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
+test_expect_success 'pull --no-autostash & pull.rebase=true' '
+   test_config pull.rebase true &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   test_must_fail git pull --no-autostash . copy 2>err &&
+   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
+'
+
 test_expect_success 'branch.to-rebase.rebase' '
git reset --hard before-rebase &&
test_config branch.to-rebase.rebase true &&
-- 
2.7.1.340.g69eb491.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

[PATCH 4/5] t/t5520: modify tests to reduce common code

[Mehul Jain]

There exist three groups of tests which have repetitive lines of code.

Introduce two functions test_rebase_autostash() and
test_rebase_no_autostash() to reduce the number of lines. Also introduce
loops to futher reduce the current implementation.

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 100 +++-
 1 file changed, 41 insertions(+), 59 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index d03cb84..2611170 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -9,6 +9,24 @@ modify () {
mv "$2.x" "$2"
 }
 
+test_rebase_autostash () {
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+}
+
+test_rebase_no_autostash () {
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
+   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
+}
+
 test_expect_success setup '
echo file >file &&
git add file &&
@@ -256,75 +274,39 @@ test_expect_success 'pull --rebase succeeds with dirty 
working directory and reb
test "$(cat file)" = "modified again"
 '
 
-test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '
-   test_config rebase.autostash true &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --rebase --autostash . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
-'
-
-test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '
-   test_config rebase.autostash false &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --rebase --autostash . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
-'
+for i in true false
+   do
+   test_expect_success "pull --rebase --autostash & 
rebase.autostash=$i" '
+   test_config rebase.autostash $i &&
+   test_rebase_autostash
+   '
+   done
 
 test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   git pull --rebase --autostash . copy &&
-   test_cmp_rev HEAD^ copy &&
-   test "$(cat new_file)" = dirty &&
-   test "$(cat file)" = "modified again"
+   test_rebase_autostash
 '
 
-test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
-   test_config rebase.autostash true &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
-   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
-'
-
-test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '
-   test_config rebase.autostash false &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
-   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
-'
+for i in true false
+   do
+   test_expect_success "pull --rebase --no-autostash & 
rebase.autostash=$i" '
+   test_config rebase.autostash $i &&
+   test_rebase_no_autostash
+   '
+   done
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
-   git reset --hard before-rebase &&
-   echo dirty >new_file &&
-   git add new_file &&
-   test_must_fail git pu

[PATCH 1/5] t/t5520: change rebase.autoStash to rebase.autostash

[Mehul Jain]

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 745e59e..5be39df 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -267,7 +267,7 @@ test_expect_success 'pull --rebase --autostash & 
rebase.autostash=true' '
test "$(cat file)" = "modified again"
 '
 
-test_expect_success 'pull --rebase --autostash & rebase.autoStash=false' '
+test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '
test_config rebase.autostash false &&
git reset --hard before-rebase &&
echo dirty >new_file &&
@@ -278,7 +278,7 @@ test_expect_success 'pull --rebase --autostash & 
rebase.autoStash=false' '
test "$(cat file)" = "modified again"
 '
 
-test_expect_success 'pull --rebase: --autostash & rebase.autoStash unset' '
+test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
-- 
2.7.1.340.g69eb491.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

[PATCH 0/5] modify tests for --[no-]autostash option

[Mehul Jain]

The following patch series is applicable on mj/pull-rebase-autostash.

This series contain changes suggested by Eric and Matthieu on following
series

http://thread.gmane.org/gmane.comp.version-control.git/289434

Changes made:
* [Patch 4/5] reduces the code needed to test possible
  combinations of --autostash and rebase.autostash by introducing
  two functions.

  Also introduce a loop to tackle the repetitive code used to
  test the usage of --[no-]autostash without --rebase.

* [Patch 5/5] introduces two new tests to check the cases when
  "git pull --[no-]autostash" is called with pull.rebase=true.


Mehul Jain (5):
  t/t5520: change rebase.autoStash to rebase.autostash
  t/t5520: explicitly unset rebase.autostash
  t/t5520: use test_i18ngrep instead of test_cmp
  t/t5520: modify tests to reduce common code
  t/t5520: test --[no-]autostash with pull.rebase=true

 t/t5520-pull.sh | 120 
 1 file changed, 61 insertions(+), 59 deletions(-)

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

[PATCH 3/5] t/t5520: use test_i18ngrep instead of test_cmp

[Mehul Jain]

test_cmp is used for error checking when test_i18ngrep could be used.

Use test_i18ngrep to check for the valid error.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 9ee2218..d03cb84 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -317,15 +317,13 @@ test_expect_success 'pull --rebase --no-autostash & 
rebase.autostash unset' '
 '
 
 test_expect_success 'pull --autostash (without --rebase) should error out' '
-   test_must_fail git pull --autostash . copy 2>actual &&
-   echo "fatal: --[no-]autostash option is only valid with --rebase." 
>expect &&
-   test_i18ncmp actual expect
+   test_must_fail git pull --autostash . copy 2>err &&
+   test_i18ngrep "only valid with --rebase" err
 '
 
 test_expect_success 'pull --no-autostash (without --rebase) should error out' '
-   test_must_fail git pull --no-autostash . copy 2>actual &&
-   echo "fatal: --[no-]autostash option is only valid with --rebase." 
>expect &&
-   test_i18ncmp actual expect
+   test_must_fail git pull --no-autostash . copy 2>err &&
+   test_i18ngrep "only valid with --rebase" err
 '
 
 test_expect_success 'pull.rebase' '
-- 
2.7.1.340.g69eb491.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

[PATCH 2/5] t/t5520: explicitly unset rebase.autostash

[Mehul Jain]

Tests title suggest that tests are done with rebase.autostash unset,
but doesn not take any action to make sure that it is indeed unset.

Make sure that rebase.autostash is unset by explicitly setting it.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 5be39df..9ee2218 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -279,6 +279,7 @@ test_expect_success 'pull --rebase --autostash & 
rebase.autostash=false' '
 '
 
 test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
+   test_unconfig rebase.autostash &&
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
@@ -307,6 +308,7 @@ test_expect_success 'pull --rebase --no-autostash & 
rebase.autostash=false' '
 '
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
+   test_unconfig rebase.autostash &&
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
-- 
2.7.1.340.g69eb491.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: [PATCH v10 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Sat, Mar 26, 2016 at 12:07 AM, Matthieu Moy
 wrote:
> I think you should also change one of the tests to use pull.resbase=true
> so that this behavior is properly tested.

Sure. I will add this test in the re-roll.

Thanks,
Mehul
--
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 v10 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 25, 2016 at 2:35 PM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>
>> +--autostash::
>> +--no-autostash::
>> + Before starting rebase, stash local modifications away (see
>> + linkgit:git-stash[1]) if needed, and apply the stash when
>> + done. `--no-autostash` is useful to override the `rebase.autoStash`
>> + configuration variable (see linkgit:git-config[1]).
>> ++
>> +This option is only valid when "--rebase" is used.
>
> This does not have to be added to this series (I don't want to break
> everything at v10 ...), but I think it would be nice to allow "git pull
> --autostash" even without --rebase if pull.rebase=true.

This is a nice observation. As current patch allow "git pull --autostash"
to be run without --rebase if pull.rebase=true, hence correct
documentation should be something like this

This option is only valid when "--rebase" is used or pull.rebase=true.

But OTOH users who knows about pull.rebase understands that
pull.rebase=true means "git pull --rebase ..." will be executed whenever
"git pull ..." is called, thus for those users it might be easy to deduce that
need of "--rebase" for validity of "--autostash" is not necessary if
pull.rebase=true.

I will correct it in the re-roll.

Thanks,
Mehul
--
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 v10 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 25, 2016 at 2:01 PM, Eric Sunshine  wrote:
>> +test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '
>
> Nit: Some of the test titles spell this as "rebase.autostash" while
> others use "rebase.autoStash".

That's a mistake. All test titles must spell "rebase.autoStash".

>> +test_expect_success 'pull --rebase: --autostash & rebase.autoStash unset' '
>
> The title says that this is testing with rebase.autoStash unset,
> however, the test itself doesn't take any action to ensure that it is
> indeed unset.

Actually test_config unset the config variable once the test is complete.
Thus I felt that test_unconfig might not be needed.

>As with the two above tests which explicitly set
> rebase.autoStash, this test should explicitly unset rebase.autoStash
> to ensure consistent results even if some future change somehow
> pollutes the configuration globally. Therefore:
>
> test_unconfig rebase.autostash &&
>

But considering this point, I'm convinced that indeed test_unconfig
should have been used.

>> +   git reset --hard before-rebase &&
>> +   echo dirty >new_file &&
>> +   git add new_file &&
>> +   git pull --rebase --autostash . copy &&
>> +   test_cmp_rev HEAD^ copy &&
>> +   test "$(cat new_file)" = dirty &&
>> +   test "$(cat file)" = "modified again"
>> +'
>
> With the addition of these three new tests, aside from the
> introductory 'test_{un}config', this exact sequence of commands is now
> repeated four times in the script. Such repetition suggests that the
> common code should be moved to a function. For instance:
>
> test_rebase_autostash () {
> git reset --hard before-rebase &&
> echo dirty >new_file &&
> git add new_file &&
> git pull --rebase . copy &&
> test_cmp_rev HEAD^ copy &&
> test "$(cat new_file)" = dirty &&
> test "$(cat file)" = "modified again"
> }
>
> And, a caller would look like this:
>
> test_expect_success 'pull ... rebase.autostash=true' '
> test_config rebase.autostash true &&
> test_rebase_autostash
> '
>
> Of course, you'd also update the original test, from which this code
> was copied, to also call the new function. Factoring out the common
> code into a function should probably be done as a separate preparatory
> patch.
>
> This suggestion isn't mandatory and doesn't demand a re-roll, but, if
> you're feeling ambitious, it would make the code easier to digest and
> review.

Nice. This will increase fluency of the code and also lead to significant
reduction in number of new lines introduced by this patch.

>> +test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
>> +   test_config rebase.autostash true &&
>> +   git reset --hard before-rebase &&
>> +   echo dirty >new_file &&
>> +   git add new_file &&
>> +   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
>> +   test_i18ngrep "Cannot pull with rebase: Your index contains 
>> uncommitted changes." err
>
> I don't care strongly, but many tests consider test_must_fail() alone
> sufficient to verify proper behavior and don't bother being more exact
> by checking the precise error message (since error messages sometimes
> get refined, thus requiring adjustments to the tests). If you do

Main reason to use test_i18ngrep here and check for this specific
error is that in future if some developer make changes which might
trigger git-pull not to die at die_on_unclean_work_tree() check (if
work tree is dirty) but leads git-pull to die somewhere else then
basically he/she will not understand the bug introduced by him/her as
test "pull --rebase --no-autostash & rebase.autostash=true" might pass.
test_i18ngrep will make sure that this does not happen.

> retain the error message check, it's often sufficient to check for
> just a fragment of the error string rather than the full message. For
> instance, it might be fine to grep merely for "uncommitted changes".

Yes, that will work too as no other error messages for git-pull contain these
words.

>> +'
>> +
>> +test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' 
>> '
>> +   test_config rebase.autostash false &&
>> +   git reset --hard before-rebase &&
>> +   echo dirty >new_file &&
>> +   git add new_file &&
>> +   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
>> +   test_i18ngrep "Cannot pull with rebase: Your index contains 
>> uncommitted changes." err
>> +'
>> +
>> +test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' 
>> '
>
> Same comment as above:
>
> test_unconfig rebase.autostash &&
>
>> +   git reset --hard before-rebase &&
>> +   echo dirty >new_file &&
>> +   git add new_file &&
>> +   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
>> +   test_i18ngrep "Cannot pull with rebase: Your index contains 
>> 

Re: [PATCH v10 0/2] introduce --[no-]autostash command line flag

[Mehul Jain]

On Fri, Mar 25, 2016 at 12:53 PM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Mon, Mar 21, 2016 at 2:18 PM, Mehul Jain <mehul.jain2...@gmail.com> wrote:
>> Changes introduced w.r.t. previous patch:
>> [...]
>> * Two more tests are added to checkout for error when "git pull
>>   --[no-]autostash" is called. Here I'm forced to use "test_i18ncmp"
>>   instead of "test_i18ngrep" to compare the expected error message with
>>   the actual because grep was, unfortunately, reading "--[no-]autostash"
>>   as an option and thus leading to test failure.
>
> Pass -e to grep to treat the next argument as an expression (even if
> it happens to look like an option):
>
> test_i18ngrep -e "--[no-]-autostash ..."
>
> You may also need to escape the [ and ] with backslash (\) to force
> grep to treat them as literal characters rather than as the character
> set "[no-]". Alternately, rather than escaping, also pass the -F flag
> to make it treat all characters as literals.

Thanks for this. I tried it out

test_i18ngrep -F -e "--[no-]autostash ..." err

and worked fine.

Thanks,
Mehul
--
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: Regarding GSoC 2016

[Mehul Jain]

On Thu, Mar 24, 2016 at 11:13 PM, Saurabh Jain
 wrote:
> The proposal can be accessed here.
>  5mWF_cqoZS6IUvGr6CVu5SmkbZk/edit?usp=sharing>

It's broken I guess.

Also Cc  Carlos Martín Nieto . He is one of
the possible mentor on this one.

> --
> 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
--
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: git-apply does not work in a sub-directory of a Git repository

[Mehul Jain]

On Wed, Mar 23, 2016 at 8:51 PM, Junio C Hamano  wrote:
> I think we do have --no-index (which is why I am largely ignoring
> the rest of your message as uninformed speculation for now).

--no-index command line flag is there for git-apply but unfortunately not
documented.

Also *auto-completion* for "git apply --no-index" doesn't work.

That is

git apply --no-i

should be auto completed and give

   git apply --no-index.

Probably following change will remove this problem.

diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 45ec47f..860dae0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -886,7 +886,7 @@ _git_apply ()
;;
--*)
__gitcomp "
-   --stat --numstat --summary --check --index
+   --stat --numstat --summary --check --index --no-index
--cached --index-info --reverse --reject --unidiff-zero
--apply --no-add --exclude=
--ignore-whitespace --ignore-space-change
--
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-apply does not work in a sub-directory of a Git repository

[Mehul Jain]

Hello everyone,

Recently while using git-apply, I observed that if git-apply is used in a
sub-directory of a Git repository then it silently dies without doing
anything.

Here's what I did

~ $mkdir example
~ $cd example
example $git init
Initialized empty Git repository in /home/mj/example/.git/
example $echo main >main
example $git add main
example $git commit -m 'main'
[master (root-commit) 97aeda3] main
 1 file changed, 1 insertion(+)
 create mode 100644 main
example $git checkout -b feature
Switched to a new branch 'feature'
example $echo modified >main
example $git add main
example $git commit -m 'modified'
[feature 2551fa2] modified
 1 file changed, 1 insertion(+), 1 deletion(-)
example $mkdir outgoing
example $git diff master >outgoing/feature.patch
example $git checkout master
Switched to branch 'master'
example $cd outgoing/
outgoing $git apply feature.patch
outgoing $cd ..
example $cat main
main

As you observed, patch wasn't applied. Is it intended behaviour of
git-apply? Usually to apply the patch I have to copy it to top directory
and then use git-apply.

I tried out git-am to apply the patch ("git format-patch" was used to
make patch) while being in the "outgoing" sub-directory and it worked
fine. So why does git-apply show this kind of behaviour?

Thanks,
Mehul
--
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 v10 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-pull.txt |  9 ++
 builtin/pull.c | 12 
 t/t5520-pull.sh| 70 ++
 3 files changed, 91 insertions(+)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..3914507 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash[1]) if needed, and apply the stash when
+   done. `--no-autostash` is useful to override the `rebase.autoStash`
+   configuration variable (see linkgit:git-config[1]).
++
+This option is only valid when "--rebase" is used.
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index c21897d..d98f481 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int opt_autostash = -1;
 static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -802,6 +805,10 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
+   else if (opt_autostash == 1)
+   argv_array_push(, "--autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -847,8 +854,13 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (get_sha1("HEAD", orig_head))
hashclr(orig_head);
 
+   if (!opt_rebase && opt_autostash != -1)
+   die(_("--[no-]autostash option is only valid with --rebase."));
+
if (opt_rebase) {
int autostash = config_autostash;
+   if (opt_autostash != -1)
+   autostash = opt_autostash;
 
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..745e59e 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -256,6 +256,76 @@ test_expect_success 'pull --rebase succeeds with dirty 
working directory and reb
test "$(cat file)" = "modified again"
 '
 
+test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '
+   test_config rebase.autostash true &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
+test_expect_success 'pull --rebase --autostash & rebase.autoStash=false' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test

[PATCH v10 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-pull.txt |  9 ++
 builtin/pull.c | 12 
 t/t5520-pull.sh| 70 ++
 3 files changed, 91 insertions(+)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..3914507 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash.txt[1]) if needed, and apply the stash when
+   done. `--no-autostash` is useful to override the `rebase.autoStash`
+   configuration variable (see linkgit:git-config[1]).
++
+This option is only valid when "--rebase" is used.
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index c21897d..d98f481 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int opt_autostash = -1;
 static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -802,6 +805,10 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
+   else if (opt_autostash == 1)
+   argv_array_push(, "--autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -847,8 +854,13 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (get_sha1("HEAD", orig_head))
hashclr(orig_head);
 
+   if (!opt_rebase && opt_autostash != -1)
+   die(_("--[no-]autostash option is only valid with --rebase."));
+
if (opt_rebase) {
int autostash = config_autostash;
+   if (opt_autostash != -1)
+   autostash = opt_autostash;
 
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..745e59e 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -256,6 +256,76 @@ test_expect_success 'pull --rebase succeeds with dirty 
working directory and reb
test "$(cat file)" = "modified again"
 '
 
+test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '
+   test_config rebase.autostash true &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
+test_expect_success 'pull --rebase --autostash & rebase.autoStash=false' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+

[PATCH v10 0/2] introduce --[no-]autostash command line flag

[Mehul Jain]

h' 
'
+test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
test_config rebase.autostash true &&
git reset --hard before-rebase &&
echo dirty >new_file &&
@@ -286,7 +297,7 @@ test_expect_success 'pull --rebase: --no-autostash 
overrides rebase.autostash' '
test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
 '
 
-test_expect_success 'pull --rebase --no-autostash works with rebase.autostash 
set false' '
+test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '
test_config rebase.autostash false &&
git reset --hard before-rebase &&
echo dirty >new_file &&
@@ -295,6 +306,26 @@ test_expect_success 'pull --rebase --no-autostash works 
with rebase.autostash se
test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
 '
 
+test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   test_must_fail git pull --rebase --no-autostash . copy 2>err &&
+   test_i18ngrep "Cannot pull with rebase: Your index contains uncommitted 
changes." err
+'
+
+test_expect_success 'pull --autostash (without --rebase) should error out' '
+   test_must_fail git pull --autostash . copy 2>actual &&
+   echo "fatal: --[no-]autostash option is only valid with --rebase." 
>expect &&
+   test_i18ncmp actual expect
+'
+
+test_expect_success 'pull --no-autostash (without --rebase) should error out' '
+   test_must_fail git pull --no-autostash . copy 2>actual &&
+   echo "fatal: --[no-]autostash option is only valid with --rebase." 
>expect &&
+   test_i18ncmp actual expect
+'
+
 test_expect_success 'pull.rebase' '
git reset --hard before-rebase &&
test_config pull.rebase true &&


Mehul Jain (2):
  git-pull.c: introduce git_pull_config()
  pull --rebase: add --[no-]autostash flag

 Documentation/git-pull.txt |  9 ++
 builtin/pull.c | 30 ++--
 t/t5520-pull.sh| 70 ++
 3 files changed, 106 insertions(+), 3 deletions(-)

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

[PATCH v10 1/2] git-pull.c: introduce git_pull_config()

[Mehul Jain]

git-pull makes a seperate call to git_config_get_bool() to read the value
of "rebase.autostash". This can be reduced as a call to git_config() is
already there in the code.

Introduce a callback function git_pull_config() to read "rebase.autostash"
along with other variables.

Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 builtin/pull.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..c21897d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
@@ -306,6 +307,18 @@ static enum rebase_type config_get_rebase(void)
 }
 
 /**
+ * Read config variables.
+ */
+static int git_pull_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "rebase.autostash")) {
+   config_autostash = git_config_bool(var, value);
+   return 0;
+   }
+   return git_default_config(var, value, cb);
+}
+
+/**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
 static int has_unstaged_changes(const char *prefix)
@@ -823,7 +836,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (opt_rebase < 0)
opt_rebase = config_get_rebase();
 
-   git_config(git_default_config, NULL);
+   git_config(git_pull_config, NULL);
 
if (read_cache_unmerged())
die_resolve_conflict("Pull");
@@ -835,12 +848,11 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
+   int autostash = config_autostash;
 
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
if (!autostash)
die_on_unclean_work_tree(prefix);
 
-- 
2.7.1.340.g69eb491.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: [PATCH v9 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 18, 2016 at 9:54 AM, Eric Sunshine  wrote:
> Since this is now a patch series rather than a single patch, another
> way to help reviewers is to use a cover letter (see git-format-patch
> --cover-letter) where you'd explain the changes, and, importantly,
> include an interdiff between the previous and current versions.

Point noted for the future patches.

>> +test_expect_success 'pull --rebase: --autostash overrides rebase.autostash' 
>> '
>
> Why do titles of some of the new test titles have a ":" after "rebase"
> while other don't?
>
> Also, how about normalizing the titles so that the reader knows in
> which tests rebase.autostash is 'true' and in which it is 'false'?
> Presently, it's difficult to decipher what's being tested based only
> on the titles.

If it's so then how about the tests titles to be the following:

* pull --rebase: --autostash works with rebase.autoStash set true

* pull --rebase: --autostash works with rebase.autoStash set false

* pull --rebase: --no-autostash works with rebase.autoStash set true

* pull --rebase: --no-autostash works with rebase.autoStash set false

Earlier I tried to keep it as less verbose as possible (and probably
made it hard to decipher). Does the above titles seems short and
informative to you? If so then I will use them instead of earlier ones.

> Finally, shouldn't you also be testing --autostash and --no-autostash
> when rebase.autostash is not set?

If rebase.autoStash is not set then config.autostash will remain zero
through out the process. What I want to point out is that rebase.autoStash
, if not set, is equivalent to being set false. So adding tests regarding
"--[no-]autostash with rebase.autoStash unset" seems equivalent to tests
" pull --rebase: --autostash works with rebase.autoStash set false" and
"pull --rebase: --no-autostash works with rebase.autoStash set false".

Thanks,
Mehul
--
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 v9 1/2] git-pull.c: introduce git_pull_config()

[Mehul Jain]

git-pull makes a seperate call to git_config_get_bool() to read the value
of "rebase.autostash". This can be reduced as a call to git_config() is
already there in the code.

Introduce a callback function git_pull_config() to read "rebase.autostash"
along with other variables.

Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
previous patches: http://thread.gmane.org/gmane.comp.version-control.git/287709
 builtin/pull.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..72f4475 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
@@ -304,6 +305,17 @@ static enum rebase_type config_get_rebase(void)
 
return REBASE_FALSE;
 }
+/**
+ * Read config variables.
+ */
+static int git_pull_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "rebase.autostash")) {
+   config_autostash = git_config_bool(var, value);
+   return 0;
+   }
+   return git_default_config(var, value, cb);
+}
 
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
@@ -823,7 +835,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (opt_rebase < 0)
opt_rebase = config_get_rebase();
 
-   git_config(git_default_config, NULL);
+   git_config(git_pull_config, NULL);
 
if (read_cache_unmerged())
die_resolve_conflict("Pull");
@@ -835,13 +847,10 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
-
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if (!config_autostash)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
-- 
2.7.1.340.g69eb491.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

[PATCH v9 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
previous patches: http://thread.gmane.org/gmane.comp.version-control.git/287709

Changes:
* Modified documentation
* "git pull --[no-]autostash" case is handled bit early then before

 Documentation/git-pull.txt |  9 +
 builtin/pull.c | 12 +++-
 t/t5520-pull.sh| 39 +++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..3914507 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash.txt[1]) if needed, and apply the stash when
+   done. `--no-autostash` is useful to override the `rebase.autoStash`
+   configuration variable (see linkgit:git-config[1]).
++
+This option is only valid when "--rebase" is used.
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 72f4475..671179b 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int opt_autostash = -1;
 static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -801,6 +804,7 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   argv_array_push(, opt_autostash ? "--autostash" : 
"--no-autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -846,11 +850,17 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (get_sha1("HEAD", orig_head))
hashclr(orig_head);
 
+   if (!opt_rebase && opt_autostash != -1)
+   die(_("--[no-]autostash option is only valid with --rebase."));
+
if (opt_rebase) {
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   if (!config_autostash)
+   if (opt_autostash == -1)
+   opt_autostash = config_autostash;
+
+   if (!opt_autostash)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..85d9bea 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -255,6 +255,45 @@ test_expect_success 'pull --rebase succeeds with dirty 
working directory and reb
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
 '
+test_expect_success 'pull --rebase: --autostash overrides rebase.autostash' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
+test_e

Re: [PATCH v9 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 18, 2016 at 10:09 AM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Fri, Mar 18, 2016 at 12:24 AM, Eric Sunshine <sunsh...@sunshineco.com> 
> wrote:
>> On Thu, Mar 17, 2016 at 12:49 PM, Mehul Jain <mehul.jain2...@gmail.com> 
>> wrote:
>>> @@ -801,6 +804,7 @@ static int run_rebase(const unsigned char *curr_head,
>>> argv_array_pushv(, opt_strategy_opts.argv);
>>> if (opt_gpg_sign)
>>> argv_array_push(, opt_gpg_sign);
>>> +   argv_array_push(, opt_autostash ? "--autostash" : 
>>> "--no-autostash");
>>
>> At this point, we know that opt_autostash can't be -1 (thus
>> incorrectly triggering use of --autostash) because the conditional in
>> cmd_pull() set it to the value of config_autostash (either 0 or 1) if
>> the user did not specify it on the command-line. Okay. Makes sense.
>
> Actually, this is going to pass --autostash or --no-autostash to
> git-rebase unconditionally won't it? This seems kind of undesirable
> due to the unnecessarily tight coupling it creates between the two
> commands. I wasn't paying close attention to the earlier discussion,
> but wasn't the idea that you should pass one of these two options
> along to git-rebase only if the user explicitly asked to do by saying
> so on the command line?

This is interesting. I checked out git-rebase.sh and found that it reads
rebase.autoStash if nothing is specified by user. So if user is not
specifying anything about stashing then it is the job of git-rebase
to decide whether or not to do stashing by reading rebase.autoStash.

Similarly if user doesn't specify the --[no-]autostash option to git-pull
then neither of --autostash and --no-autstash should be passed to the
git-rebase as it will decide on his own about what needs to be done.
Agreed. I made a unnecessary tight coupling between git-pull and
git-rebase. Instead of that the following changes can be done to
remove it.

...
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
-   argv_array_push(, opt_autostash ? "--autostash" :
"--no-autostash");
+   if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
+   else if (opt_autostash == 1)
+   argv_array_push(, "--autostash");

...

...
if (opt_rebase) {
+   int autostash = config_autostash;
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("... with changes added to the index."));

-   if (opt_autostash == -1)
-   opt_autostash = config_autostash;
+   if (opt_autostash != -1)
+   autostash = opt_autostash;

-   if (!opt_autostash)
+   if (!autostash)
die_on_unclean_work_tree(prefix);

if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
}
...

Note that the above changes are suggest with respect to my patch, not
the current
code base.

This way there's no need to remove "autostash" from the current code
base and instead use it to write a much cleaner patch.  Something like
this (this is w.r.t. current code base)

...
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
+   else if (opt_autostash == 1)
+   argv_array_push(, "--autostash");

...

...

if (opt_rebase) {
 int autostash = config_autostash;
+   if (opt_autostash != -1)
+   autostash = opt_autostash;

if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("... with changes added to the index."));

if (!autostash)
die_on_unclean_work_tree(prefix);

if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
}
...


What are your views on this? Also this way I will not touch changes
introduced in patch 1/2.

> In other words:
>
> * invoke "git-rebase --autostash" only if the user typed "git pull
> --rebase --autostash"
>
> * invoke "git-rebase --no-autostash" only if the user typed "git pull
> --rebase --no-autostash"
>
> * invoke "git rebase" if the user typed bare "git pull --rebase"


Thanks,
Mehul
--
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/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

Hello Eric,

I tried out this approach and here's the result.

---

diff --git a/builtin/pull.c b/builtin/pull.c
index b5b0255..0ce007d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int opt_autostash = -1;
 static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -801,6 +804,7 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   argv_array_push(, opt_autostash ? "--autostash" :
"--no-autostash");

argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -846,12 +850,20 @@ int cmd_pull(int argc, const char **argv, const
char *prefix)
if (get_sha1("HEAD", orig_head))
hashclr(orig_head);

+   if(!opt_rebase && opt_autostash != -1)
+   die(_("--[no-]autostash option is only valid with --rebase."));
+
if (opt_rebase) {
int autostash = config_autostash;

if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes
added to the index."));

+   if (opt_autostash != -1)
+   autostash = opt_autostash;
+   else
+   opt_autostash = config_autostash;
+
if (!autostash)
die_on_unclean_work_tree(prefix);
---

This way of implementation looks a bit less clean to me than
the previous one because we are using "opt_autostash" to pass
the "--[no-]autostash"  flag to git-rebase, thus if user does not
specify anything about stashing in command line then  config_autostash
value has to be used ( i.e. opt_autostash = config_autostash).
To do this an "else" case has to be introduced in the code. This
might effect the readability of the code because the reader might
wonder why "opt_autostash" is used to assign value to "autostash"
in one case, and opt_autostash = config_autostash in other case.

But I agree that this way I won't be touching the changes I made
in patch 1/2.

I would like to know your view on above mentioned issue.

Also I made a mistake in patch 1/2 which I will correct in the next
version along with other changes suggested by you.

Thanks,
Mehul
--
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/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Wed, Mar 16, 2016 at 3:13 AM, Eric Sunshine  wrote:
>> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
>> @@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
>> +--autostash::
>> +--no-autostash::
>> +   Before starting rebase, stash local modifications away (see
>> +   linkgit:git-stash.txt[1]) if needed, and apply the stash when
>> +   done (this option is only valid when "--rebase" is used).
>> ++
>> +`--no-autostash` is useful to override the `rebase.autoStash`
>> +configuration variable (see linkgit:git-config[1]).
>
> The last couple sentences seem reversed. It feels odd to have the bit
> about --rebase dropped dead in the middle of the description of
> --autostash and --no-autostash. I'd have expected to see --autostash
> and --no-autostash discussed together, and then, as a follow-on,
> mention them being valid only with --rebase.

So you are suggesting something like this:

--autostash::
--no-autostash::
Before starting rebase, stash local modifications away (see
linkgit:git-stash.txt[1]) if needed, and apply the stash when
done. `--no-autostash` is useful to override the `rebase.autoStash`
configuration variable (see linkgit:git-config[1]).
+
This option is only valid when "--rebase" is used.

Can be done and it make more sense to talk about the validity of the
option in a seperate line.

>> diff --git a/builtin/pull.c b/builtin/pull.c
>> @@ -851,12 +855,17 @@ int cmd_pull(int argc, const char **argv, const char 
>> *prefix)
>> if (is_null_sha1(orig_head) && !is_cache_unborn())
>> die(_("Updating an unborn branch with changes added 
>> to the index."));
>>
>> -   if (config_autostash)
>> +   if (opt_autostash == -1)
>
> In patch 1/2, this changed from 'if (autostash)' to 'if
> (config_autostash)'; it's a bit sad that patch 2/2 then has to touch
> the same code to change it yet again, this time to 'if
> (opt_autostash)'. Have you tried just keeping the original 'autostash'
> variable and modifying its value based upon config_autostash and
> opt_autostash instead? (Not saying that this would be better, but
> interested in knowing if the result is as clean or cleaner or worse.)

Yes, I tried that. Things looked something like this:

In patch 1/2
...

-int autostash = 0;
+int autostash = config_autoshash;

if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating ..."));

-git_config_get_bool("rebase.autostash", );
if (!autostash)
die_on_unclean_work_tree(prefix);

...

In patch 2/2
...
int autostash = config_autoshash;

if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating ..."));

+if (opt_autostash != -1)
+autostash = opt_autostash;

if (!autostash)
die_on_unclean_work_tree(prefix);
...

This implementation looks much more cleaner but we are using some
extra space (autostash) to do the task. If everyone is fine with this
trade off then I can re-roll a new patch with this method. Comments please.

>> +   opt_autostash = config_autostash;
>> +
>> +   if (!opt_autostash)
>> die_on_unclean_work_tree(prefix);
>>
>> if (get_rebase_fork_point(rebase_fork_point, repo, 
>> *refspecs))
>> hashclr(rebase_fork_point);
>> -   }
>> +   } else
>> +   if (opt_autostash != -1)
>> +die(_("--[no-]autostash option is only valid with 
>> --rebase."));
>
> How about formatting this as a normal 'else if'?
>
> } else if (opt_autostash != -1)

I thought of it earlier but voted against it as it may reduce the readability of
the code.

> On the other hand, this error case hanging off the 'rebase'
> conditional is somewhat more difficult to reason about than perhaps it
> ought to be. It might be easier to see what's going on if you get the
> error case out of the way early, and then handle the normal case. That
> is, something like this:
>
> if (!opt_rebase && opt_autostash != -1)
> die(_("... is only valid with --rebase"));
>
> if (opt_rebase) {
> ...
> }

This is good. I'll make the changes accordingly.

Thanks for the comments.

Mehul
--
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/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
previous patches: http://thread.gmane.org/gmane.comp.version-control.git/287709

 Documentation/git-pull.txt |  9 +
 builtin/pull.c | 13 +++--
 t/t5520-pull.sh| 39 +++
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..a070ec9 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash.txt[1]) if needed, and apply the stash when
+   done (this option is only valid when "--rebase" is used).
++
+`--no-autostash` is useful to override the `rebase.autoStash`
+configuration variable (see linkgit:git-config[1]).
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 43353f9..c48e28a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int opt_autostash = -1;
 static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -801,6 +804,7 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   argv_array_push(, opt_autostash ? "--autostash" : 
"--no-autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -851,12 +855,17 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   if (config_autostash)
+   if (opt_autostash == -1)
+   opt_autostash = config_autostash;
+
+   if (!opt_autostash)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-   }
+   } else
+   if (opt_autostash != -1)
+die(_("--[no-]autostash option is only valid with 
--rebase."));
 
if (run_fetch(repo, refspecs))
return 1;
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..85d9bea 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -255,6 +255,45 @@ test_expect_success 'pull --rebase succeeds with dirty 
working directory and reb
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
 '
+test_expect_success 'pull --rebase: --autostash overrides rebase.autostash' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
+test_expect_success 'pull --rebase --autostash works with rebase.autostash set 
true' '
+   test_config rebase.au

[PATCH 1/2] git-pull.c: introduce git_pull_config()

[Mehul Jain]

git-pull makes a seperate call to git_config_get_bool() to read the value
of "rebase.autostash", this can be reduced as a call to git_config() is
already there in the code.

Introduce a callback function git_pull_config() to read "rebase.autostash"
along with other variables.

Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
Previous patches: http://thread.gmane.org/gmane.comp.version-control.git/287709

Change: config_autostash initialized with 0 instead of -1

 builtin/pull.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..43353f9 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int config_autostash;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
@@ -304,6 +305,17 @@ static enum rebase_type config_get_rebase(void)
 
return REBASE_FALSE;
 }
+/**
+ * Read config variables.
+ */
+static int git_pull_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "rebase.autostash")) {
+   config_autostash = git_config_bool(var, value);
+   return 0;
+   }
+   return git_default_config(var, value, cb);
+}
 
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
@@ -823,7 +835,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (opt_rebase < 0)
opt_rebase = config_get_rebase();
 
-   git_config(git_default_config, NULL);
+   git_config(git_pull_config, NULL);
 
if (read_cache_unmerged())
die_resolve_conflict("Pull");
@@ -835,13 +847,11 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
 
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if (config_autostash)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
-- 
2.7.1.340.g69eb491.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: [PATCH v7 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 11, 2016 at 10:21 AM, Paul Tan  wrote:
> Stepping back a bit, the only reason why we introduced opt_autostash =
> -1 as a possible value is because we need to detect if
> --[no-]autostash is being used with git-merge. I consider that a
> kludge, because if git-merge supports --autostash as well (possibly in
> the future), then we will not need this -1 value.

No, there is one more reason for which opt_autostash = -1 is required.
When user calls "git pull --rebase" then config_autostash value will
be used to perform --[no-]autostash task but if user calls "git pull
--rebase --[no-]autostash" then config_autostash value should not be
read at all as this option is supposed to override config_autostash
value. So if opt_autostash defaults to 0 then how will the code
understand if "--[no-]autostash" flag is passed or not?

As per the current patch, the value opt_autostash = 0 or 1  tells us
that the user has explicitly asked for --no-autostash or --autostash
respectively, and -1 value tells us that user has not specified
anything and thus we should read config_autostash value to perform
--[no-]autostash.

One way to do this was to read rebase.autoStash before parse_options(),
but now  as we have introduced a callback function git_pull_config(),
reading this config variable before parse_option() will now require
calling git_config(git_pull_config, NULL) before parse_option() and
doing opt_autostash = config_autostash there only.This may lead to
some problems (I'm not sure of that), as git_config() reads many other
config variables too.

Thanks,
Mehul
--
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 v7 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 11, 2016 at 7:00 PM, Matthieu Moy
 wrote:
> What Junio says is that you don't need to write
>
> static int config_autostash = 0;
>
> since it is equivalent to
>
> static int config_autostash;
>
> But there's nothing wrong with having a static variable defaulting to 0.

My bad. I should have read Junio's comment more carefully.

config_autostash can be default to 0. And thus
 if (opt_autostash != 1)
die_on_unclean_work_tree(prefix);

can be replaced by
   if (!opt_autostash)
die_on_unclean_work_tree(prefix);

and thus opt_autostash will be either 0 or 1 and we don't
have to worry about it being -1 (whenever --rebase is passed).

I will make the changes accordingly.

Thanks,
Mehul
--
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 v7 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 11, 2016 at 10:21 AM, Paul Tan  wrote:
>>  static int config_autostash = -1;
>
> Hmm, why can't config_autostash just default to 0?

Previously Junio recommended not to explicitly initialize a
static to 0 (or NULL).
http://thread.gmane.org/gmane.comp.version-control.git/287709/focus=287726

Defaulting config_autostash = 0 will work too as you explained.

>> +   if (opt_autostash == 1)
>> +   argv_array_push(, "--autostash");
>> +   else if (opt_autostash == 0)
>> +   argv_array_push(, "--no-autostash");
>
> The precise testing for specific values of -1, 0 and 1 throughout the
> code makes me uncomfortable. Ordinarily, I would expect a simple
>
> argv_array_push(, opt_autostash ? "--autostash" : "--no-autostash");

This looks good. I will change accordingly.

> Stepping back a bit, the only reason why we introduced opt_autostash =
> -1 as a possible value is because we need to detect if
> --[no-]autostash is being used with git-merge. I consider that a
> kludge, because if git-merge supports --autostash as well (possibly in
> the future), then we will not need this -1 value.
>
> So, from that point of view, a -1 value is okay as a workaround, but
> kludges, and hence the -1 value, should be gotten rid off as soon as
> possible.

That right! But until git-merge doesn't support --autostash, it's necessary to
have opt_autostash = -1 as default.

I wonder if it will be a good thing to add the following line to the
commit message
"Changes needed to be introduced:
Change opt_autostash = 0 as default as soon as git-merge supports
--autostash option, also display no error when "git pull --autostash"
is called."

Possibly it would be better to add gmane link of your review in the
commit message
for further clarification.

Thanks,
Mehul
--
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 v7 1/2] git-pull.c: introduce git_pull_config()

[Mehul Jain]

git-pull makes a seperate call to git_config_get_bool() to read the value
of "rebase.autostash", this can be reduced as a call to git_config() is
already there in the code.

Introduce a callback function git_pull_config() to read "rebase.autostash"
along with other variables.

Helped-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
previous patches: $gname287709

This is a clean-up patch to add --[no-]autostash option to 
"git pull --rebase".
 
 builtin/pull.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..8a318e9 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int config_autostash = -1;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
@@ -304,6 +305,17 @@ static enum rebase_type config_get_rebase(void)
 
return REBASE_FALSE;
 }
+/**
+ * Read config variables.
+ */
+static int git_pull_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "rebase.autostash")) {
+   config_autostash = git_config_bool(var, value);
+   return 0;
+   }
+   return git_default_config(var, value, cb);
+}
 
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
@@ -823,7 +835,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (opt_rebase < 0)
opt_rebase = config_get_rebase();
 
-   git_config(git_default_config, NULL);
+   git_config(git_pull_config, NULL);
 
if (read_cache_unmerged())
die_resolve_conflict("Pull");
@@ -835,13 +847,11 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
 
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if (config_autostash != 1)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
-- 
2.7.1.340.g69eb491.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

[PATCH v7 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
Previous patches: $gname287709

Changes:
- Slight change is documentation.

 Documentation/git-pull.txt |  9 +
 builtin/pull.c | 16 ++--
 t/t5520-pull.sh| 39 +++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..da89be6 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash.txt[1]) if needed, and apply the stash when
+   done (this option is only valid when "--rebase" is used).
++
+`--no-autostash` is useful to override the `rebase.autoStash`
+configuration variable (see linkgit:git-config[1]).
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 8a318e9..a01058a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int opt_autostash = -1;
 static int config_autostash = -1;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -801,6 +804,10 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   if (opt_autostash == 1)
+   argv_array_push(, "--autostash");
+   else if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -851,12 +858,17 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   if (config_autostash != 1)
+   if (opt_autostash == -1)
+   opt_autostash = config_autostash;
+
+   if (opt_autostash != 1)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-   }
+   } else
+   if (opt_autostash != -1)
+die(_("--[no-]autostash option is only valid with 
--rebase."));
 
if (run_fetch(repo, refspecs))
return 1;
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..85d9bea 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -255,6 +255,45 @@ test_expect_success 'pull --rebase succeeds with dirty 
working directory and reb
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
 '
+test_expect_success 'pull --rebase: --autostash overrides rebase.autostash' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
+t

[PATCH v6 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
Previous patches: $gname287709

Changes:
- Slight change is documentation.

 Documentation/git-pull.txt |  9 +
 builtin/pull.c | 16 ++--
 t/t5520-pull.sh| 39 +++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..da89be6 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash.txt[1]) if needed, and apply the stash when
+   done (this option is only valid when "--rebase" is used).
++
+'--no-autostash' is useful to override the 'rebase.autoStash'
+configuration variable (see linkgit:git-config[1]).
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 8a318e9..a01058a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int opt_autostash = -1;
 static int config_autostash = -1;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -801,6 +804,10 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   if (opt_autostash == 1)
+   argv_array_push(, "--autostash");
+   else if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -851,12 +858,17 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   if (config_autostash != 1)
+   if (opt_autostash == -1)
+   opt_autostash = config_autostash;
+
+   if (opt_autostash != 1)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-   }
+   } else
+   if (opt_autostash != -1)
+die(_("--[no-]autostash option is only valid with 
--rebase."));
 
if (run_fetch(repo, refspecs))
return 1;
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..85d9bea 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -255,6 +255,45 @@ test_expect_success 'pull --rebase succeeds with dirty 
working directory and reb
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
 '
+test_expect_success 'pull --rebase: --autostash overrides rebase.autostash' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
+t

[PATCH v6 1/2] git-pull.c: introduce git_pull_config()

[Mehul Jain]

git-pull makes a seperate call to git_config_get_bool() to read the value
of "rebase.autostash", this can be reduced as a call to git_config() is
already there in the code.

Introduce a callback function git_pull_config() to read "rebase.autostash"
along with other variables.

Helped-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
previous patches: $gname287709

This is a clean-up patch to add --[no-]autostash option to 
"git pull --rebase".
 
 builtin/pull.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..8a318e9 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
 static char *opt_verify_signatures;
+static int config_autostash = -1;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
@@ -304,6 +305,17 @@ static enum rebase_type config_get_rebase(void)
 
return REBASE_FALSE;
 }
+/**
+ * Read config variables.
+ */
+static int git_pull_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "rebase.autostash")) {
+   config_autostash = git_config_bool(var, value);
+   return 0;
+   }
+   return git_default_config(var, value, cb);
+}
 
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
@@ -823,7 +835,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (opt_rebase < 0)
opt_rebase = config_get_rebase();
 
-   git_config(git_default_config, NULL);
+   git_config(git_pull_config, NULL);
 
if (read_cache_unmerged())
die_resolve_conflict("Pull");
@@ -835,13 +847,11 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
 
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if (config_autostash != 1)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
-- 
2.7.1.340.g69eb491.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

[PATCH v5] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Also introduce a callback function to read config variables.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
Previous patches: $gname287709

Changes: 
* Documentation modified
* Introduced a callback function to read config variables.
* Introduce two new tests
* Modified previous tests because:

if worktree is dirty and --no-autostash is passed then git-pull
should die before git-fetch is called, hence the error message 
must be: 

"Cannot pull with rebase: Your index contains uncommitted changes."

Whereas if git-fetch is invoked and --no-autostash is passed to
git-rebase then error message is:

"Cannot rebase: Your index contains uncommitted changes."

This test will prevent_future_changes by other people to break the
code which may cause git-fetch being called even if worktree is dirty
with --no-autostash option.

 Documentation/git-pull.txt |  9 +
 builtin/pull.c | 34 --
 t/t5520-pull.sh| 40 
 3 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..393ac7d 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash.txt[1]) if needed, and apply the stash when
+   done (this option is only valid when '--rebase' is used).
++
+'--no-autostash' is useful to override the 'rebase.autoStash'
+configuration variable (see linkgit:git-config[1]).
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..b25c53d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -85,6 +85,8 @@ static char *opt_squash;
 static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
+static int opt_autostash = -1;
+static int config_autostash = -1;
 static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -146,6 +148,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "ff-only", _ff, NULL,
N_("abort if fast-forward is not possible"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
@@ -306,6 +310,18 @@ static enum rebase_type config_get_rebase(void)
 }
 
 /**
+ * Read config variables.
+ */
+static int git_pull_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var,"rebase.autostash")) {
+   config_autostash = git_config_bool(var,value);
+   return 0;
+   }
+   return git_default_config(var, value, cb);
+}
+
+/**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
 static int has_unstaged_changes(const char *prefix)
@@ -789,6 +805,10 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   if (opt_autostash == 1)
+   argv_array_push(, "--autostash");
+   else if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -823,7 +843,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (opt_rebase < 0)
opt_rebase = config_get_rebase();
 
-   git_config(git_default_config, NULL);
+   git_config(git_pull_config, NULL);
 
if (read_cache_unmerged())
die_resol

Re: [PATCH v4] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

Hi Junio,

Thanks for the thorough review.

On Sat, Mar 5, 2016 at 10:34 PM, Junio C Hamano  wrote:
> Is it worth checking the case where autostash kicks in, rebase
> itself is completed successfully, but the final "stash pop" fails in
> conflict?  I am thinking aloud and just wondering, not suggesting
> to add such a test, or even suggesting that having such a test is
> worthwhile. I didn't even check if there is already an existing test
> to cover that case.

I think this case has already been checked in t3420-rebase.autostash.sh:
"rebase$type: non-conflicting rebase, conflicting stash". Though in that
test rebase.autoStash has been used to trigger git-stash.

Thanks,
Mehul
--
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 v4] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Sat, Mar 5, 2016 at 3:22 PM, Mehul Jain <mehul.jain2...@gmail.com> wrote:

> Changes:
> * --no-autostash is passed to git-rebase (suggested by Paul)
>
> * Error message changed when "git pull --[no-]autostash" is called.
>
> * If rebase.autoStash is unset and user don't pass --[no-]autostash
>   flag then nothing is passed to git-rebase (i.e. if rebase.autoStash 
> = -1),
>   as it should be left to git-rebase to decide what to do.

I'm sorry. Here instead of " ... (i.e. if rebase.autoStash = -1), ...
" it should be " ... (i.e. if opt_autostash = -1), ... " .

Thanks,
Mehul
--
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 v4] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is set, there is no way to
override it for "git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no-]autostash command line flag which
overrides the current value of rebase.autoStash, if set. As "git rebase"
understands the --[no-]autostash option, it's just a matter of passing
the option to underlying "git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---

Previous patches: $gname287709

Changes:
* --no-autostash is passed to git-rebase (suggested by Paul)

* Error message changed when "git pull --[no-]autostash" is called.

* If rebase.autoStash is unset and user don't pass --[no-]autostash
  flag then nothing is passed to git-rebase (i.e. if rebase.autoStash = 
-1),
  as it should be left to git-rebase to decide what to do.

 Documentation/git-pull.txt | 15 +++
 builtin/pull.c | 18 ++
 t/t5520-pull.sh| 19 +++
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..06e5ddd 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,21 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Before starting rebase, stash local modifications away (see
+   linkgit:git-stash.txt[1]) if needed, and apply the stash when
+   done.
++
+This option is only valid when '--rebase' is used.
++
+'--no-autostash' is useful to override the 'rebase.autoStash'
+configuration variable (see linkgit:git-config[1]).
++
+[NOTE]
+Use with care: the final stash application after a successful
+rebase might result in non-trivial conflicts.
+
 Options related to fetching
 ~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..e3f5fbf 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -85,6 +85,7 @@ static char *opt_squash;
 static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
+static int opt_autostash = -1;
 static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -146,6 +147,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "ff-only", _ff, NULL,
N_("abort if fast-forward is not possible"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
@@ -789,6 +792,10 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   if (opt_autostash == 1)
+   argv_array_push(, "--autostash");
+   else if (opt_autostash == 0)
+   argv_array_push(, "--no-autostash");
 
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
@@ -835,17 +842,20 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
-
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if (opt_autostash == -1)
+   git_config_get_bool("rebase.autostash", _autostash);
+
+   if (opt_autostash == 0 || opt_autostash == -1)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
+   } else {
+   if (opt_autostash != -1)
+   die(_("--[no-]autostash option is only valid with 
--rebase."));
}
 
if (run_fetch(repo, refspecs))
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..f5d1d31 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -245,6 +245,25 @@ test_expect_success '--rebase fails with multiple 
branches' '

Re: [PATCH v3 2/3] test: add test for --[no-]autostash flag

[Mehul Jain]

On Thu, Mar 3, 2016 at 11:01 PM, Matthieu Moy
 wrote:
> There's no need to split code/test/doc into separate patches, except if
> your patches are getting really huge. As a reviewer, I like having tests
> and doc in the same patch because they describe the intention of the
> programmer.
>
> We try to have a history where each commit is equally good, and with
> your version there are two commits where --autostash exists and is
> undocumented (which is not catastrophic, though).

Alright, I will make a single patch for the next version.

>> --- a/t/t5521-pull-options.sh
>> +++ b/t/t5521-pull-options.sh
>> @@ -62,6 +62,22 @@ test_expect_success 'git pull -v --rebase' '
>>   test_must_be_empty out)
>>  '
>>
>> +test_expect_success 'git pull --rebase --autostash' '
>> + mkdir clonedrbas &&
>> + (cd clonedrbas  && git init &&
>> + git pull --rebase --autostash "../parent" >out 2>err &&
>> + test -s err &&
>> + test_must_be_empty out)
>> +'
>> +
>> +test_expect_success 'git pull --rebase --no-autostash' '
>> + mkdir clonedrbnas &&
>> + (cd clonedrbnas  && git init &&
>> + git pull --rebase --no-autostash "../parent" >out 2>err &&
>> + test -s err &&
>> + test_must_be_empty out)
>> +'
>
> Not sure these tests are needed if you have the ones in t/t5520-pull.sh.
> More tests means more time to run so testing twice the same thing has a
> cost.

I agree with you. This test may not be needed as t/t5520-pull.sh already test
this option.

Thanks,
Mehul
--
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 v3 1/3] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Thu, Mar 3, 2016 at 10:54 PM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>
>> If rebase.autoStash configuration variable is
>> set, there is no way to override it for
>> "git pull --rebase" from the command line.
>>
>> Teach "git pull --rebase" the --[no]autostash
>> command line flag which overrides the current
>> value of rebase.autostash, if set. As "git rebase"
>> understands the --[no]autostash option, it's
>> just a matter of passing the option to underlying
>> "git rebase" when "git pull --rebase" is called.
>
> We normally wrap text with a bit less than 80 columns. Yours is wrappet
> at 50 columns which makes it look weird.

OK. I will change it.

>> + else {
>> + /* If --[no-]autostash option is called without --rebase */
>> + if (opt_autostash == 0)
>> + die(_("--no-autostash option is only valid with 
>> --rebase."));
>> + else if (opt_autostash == 1)
>
> The else is not needed since the other branch dies.

I'm bit confused here. Which "else" you are talking about. I think both the
"else" and "else if" are needed here because:

- for the first "else", it is necessary that the case is only executed
when --rebase option is not given. If "else" is removed then in some case
where user calls "git pull --rebase --autostash" will lead to the execution of
"else if (opt_autostash == 1)"  case.

- Also removal of  "else if (opt_autostash == 1)" is not the right thing. As
the possibility of opt_autostash = -1 is there and this change may lead to
the execution of "die(_("--no-autostash ... "));" in case user calls "git pull".

Though I agree with Eric on combining the "if and else if" cases.

Thanks,
Mehul
--
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 v3 3/3] Documentation/git-pull: document --[no-]autostash option

[Mehul Jain]

On Thu, Mar 3, 2016 at 10:44 PM, Junio C Hamano  wrote:
> Should this entry this verbose?
>
>  - Is there a non-temporary stash?
>
>  - I think "This means that ..." is totally unnecessary.
>
>  - It probably makes sense to have "This option is only valid..." as
>a separate second paragraph as you did.
>
>  - "The default is..." is misleading.  Even if rebase.autostash is
>set to false, we won't autostash, but that is different from the
>default being "--no-autostash".
>
>Think of "--[no-]autostash" option as *ONE* way to affect the
>auto-stashing behaviour, and treat "options" and "behaviours" two
>different things.
>
> There is no default "option" for this.  It is that "autostash"
> behaviour defaults to what is given to rebase.autostash if
> exists, and can be explicitly set by --[no-]autostash if given.
>
> But that is the norm for any configuration and option that overrides
> the configuration, so it probably is a better use of the ink to say
> something like this perhaps?
>
> --autostash::
> --no-autostash::
> Before starting "pull --rebase", create a stash to save
> local modifications, and apply the stash when done (this
> option is only valid when "--rebase" is used).

OK, but according to the definition of --[no-]autostash given in
git-rebase documentation (https://git-scm.com/docs/git-rebase),
temporary stash is created. So shouldn't we be consistent with this
definition by writing "... , create a temporary stash ..." instead of
"... , create a stash ...".

> +
> '--no-autostash' is useful to override the 'rebase.autoStash'
> configuration variable (see linkgit:git-config[1]).

Thanks,
Mehul
--
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 v3 3/3] Documentation/git-pull: document --[no-]autostash option

[Mehul Jain]

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-pull.txt | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..a593972 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,21 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+--no-autostash::
+   Automatically create a temporary stash before the operation
+   begins, and apply it after the operation ends. This means
+   that you can run rebase on a dirty worktree.
++
+This option is only valid when '--rebase' option is used.
++
+The default is --no-autostash, unless rebase.autoStash configuration 
+is set.
++
+[NOTE]
+Use with care: the final stash application after a successful
+rebase might result in non-trivial conflicts.
+
 Options related to fetching
 ~~~
 
-- 
2.7.1.340.g69eb491.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

[PATCH v3 2/3] test: add test for --[no-]autostash flag

[Mehul Jain]

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 t/t5520-pull.sh | 19 +++
 t/t5521-pull-options.sh | 16 
 2 files changed, 35 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..f5d1d31 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -245,6 +245,25 @@ test_expect_success '--rebase fails with multiple 
branches' '
test modified = "$(git show HEAD:file)"
 '
 
+test_expect_success 'pull --rebase --no-autostash fails with dirty working 
directory and rebase.autstash set true' '
+   test_config rebase.autostash true &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   test_must_fail git pull --rebase --no-autostash . copy
+'
+
+test_expect_success 'pull --rebase --autostash succeeds with dirty working 
directory and rebase.autstash set false' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
 test_expect_success 'pull --rebase succeeds with dirty working directory and 
rebase.autostash set' '
test_config rebase.autostash true &&
git reset --hard before-rebase &&
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 18372ca..3930e45 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -62,6 +62,22 @@ test_expect_success 'git pull -v --rebase' '
test_must_be_empty out)
 '
 
+test_expect_success 'git pull --rebase --autostash' '
+   mkdir clonedrbas &&
+   (cd clonedrbas  && git init &&
+   git pull --rebase --autostash "../parent" >out 2>err &&
+   test -s err &&
+   test_must_be_empty out)
+'
+
+test_expect_success 'git pull --rebase --no-autostash' '
+   mkdir clonedrbnas &&
+   (cd clonedrbnas  && git init &&
+   git pull --rebase --no-autostash "../parent" >out 2>err &&
+   test -s err &&
+   test_must_be_empty out)
+'
+
 test_expect_success 'git pull -v -q' '
mkdir clonedvq &&
(cd clonedvq && git init &&
-- 
2.7.1.340.g69eb491.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

[PATCH v3 1/3] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

If rebase.autoStash configuration variable is 
set, there is no way to override it for 
"git pull --rebase" from the command line.

Teach "git pull --rebase" the --[no]autostash
command line flag which overrides the current
value of rebase.autostash, if set. As "git rebase"
understands the --[no]autostash option, it's 
just a matter of passing the option to underlying 
"git rebase" when "git pull --rebase" is called.

Helped-by: Matthieu Moy <matthieu@grenoble-inp.fr>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Paul Tan <pyoka...@gmail.com>
Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
Sorry for a late follow up. I had my mid-semester
examination going on.

Previous patch: $gname287709

Changes:
* error message is produced when "git pull
 --[no]autostash" is called.
* opt_autostash intialized with -1.

 builtin/pull.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..b338b83 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -85,6 +85,7 @@ static char *opt_squash;
 static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
+static int opt_autostash = -1;
 static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -146,6 +147,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "ff-only", _ff, NULL,
N_("abort if fast-forward is not possible"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+   OPT_BOOL(0, "autostash", _autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
@@ -789,7 +792,8 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
-
+   if (opt_autostash)
+   argv_array_push(, "--autostash");
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
 
@@ -835,18 +839,25 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
-
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if (opt_autostash == -1)
+   git_config_get_bool("rebase.autostash", _autostash);
+
+   if (opt_autostash == 0 || opt_autostash == -1)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
}
+   else {
+   /* If --[no-]autostash option is called without --rebase */
+   if (opt_autostash == 0)
+   die(_("--no-autostash option is only valid with 
--rebase."));
+   else if (opt_autostash == 1)
+   die(_("--autostash option is only valid with 
--rebase."));
+   }
 
if (run_fetch(repo, refspecs))
return 1;
-- 
2.7.1.340.g69eb491.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: [PATCH v2 1/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Sun, Feb 28, 2016 at 12:56 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Mehul Jain <mehul.jain2...@gmail.com> writes:
>
>> git pull --rebase understands --[no-]autostash flag.
>>
>> This flag overrides config variable "rebase.autoStash"
>> if set (default is false).
>
> Is that a statement of a fact?  If so, is it true before this patch
> is applied, or after?
>
> Each project has local convention for log messages, and we do too.
> A long log message typically start by explaining how the world is,
> why that is not desirable, present a description of a possible world
> that is better, and then give commands to somebody who is updating
> the code telling him what to do to make that better world a reality
> (and optionally how).
>
> So perhaps (I am totally making this up; you need to fact check and
> adjust):
>
> If you enable rebase.autoStash option in your repository, there
> is no way to override it for "git pull --rebase" from the
> command line.
>
> Teach "git pull" a new "--[no-]autostash" option so that a
> rebase.autoStash configuration can be overridden.  As "git
> rebase" already knows "--[no-]autostash" option, it is just the
> matter of passing one when we spawn the command as necessary.
>
> or something.  The first one gives the readers how the current world
> works, and why it is not ideal.  The proposed better world in this
> case is too simple--the first paragraph complained that "we cannot
> do X" and X is something reader would likely to agree is a good
> thing to do, so it can be left unsaid that a better world is one in
> which X can be done.
>
>> When calling "git pull --rebase" with "--autostash",
>> pull passes the "--autostash" option to rebase,
>> which then runs rebase on a dirty worktree.
>>
>> With "--no-autostash" option, the command will die
>> if the worktree is dirty, before calling rebase.
>
> These two paragraphs are too obvious and probably are better left
> unsaid.  Especially the latter--you are changing "pull" and do not
> control what "rebase" would do in the future.  It could be that a
> better rebase in the future may be able to do its job in a dirty
> worktree without doing an autostash.

OK. I will follow up with a better commit message.

>> Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
>> ---
>>
>> Thanks to Paul and Matthieu for comments on previous round.
>> Changes:
>>   - --autostash flag added
>>   - OPT_COLOR_FLAG replaced by OPT_BOOL
>>   - Default value of opt_rebase changed
>>   - Few changes in code
>>   - Commit message improvements
>>   - Documentation added
>>   - Few tests removed as suggested by Paul
>>   - Added test for --autostash flag
>> All tests passed: https://travis-ci.org/mehul2029/git
>>
>>  builtin/pull.c  | 13 -
>>  t/t5520-pull.sh | 19 +++
>>  t/t5521-pull-options.sh | 16 
>>  3 files changed, 43 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/pull.c b/builtin/pull.c
>> index 10eff03..60b320e 100644
>> --- a/builtin/pull.c
>> +++ b/builtin/pull.c
>> @@ -85,6 +85,7 @@ static char *opt_squash;
>>  static char *opt_commit;
>>  static char *opt_edit;
>>  static char *opt_ff;
>> +static int opt_autostash = 0;
>
> Do not explicitly initialize a static to 0 (or NULL).
>
>>  static char *opt_verify_signatures;
>>  static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
>>  static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
>> @@ -146,6 +147,8 @@ static struct option pull_options[] = {
>>   OPT_PASSTHRU(0, "ff-only", _ff, NULL,
>>   N_("abort if fast-forward is not possible"),
>>   PARSE_OPT_NOARG | PARSE_OPT_NONEG),
>> + OPT_BOOL(0,"autostash",_autostash,
>> + N_("automatically stash/stash pop before and after rebase")),
>>   OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
>>   N_("verify that the named commit has a valid GPG signature"),
>>   PARSE_OPT_NOARG),
>> @@ -789,7 +792,8 @@ static int run_rebase(const unsigned char *curr_head,
>>   argv_array_pushv(, opt_strategy_opts.argv);
>>   if (opt_gpg_sign)
>>   argv_array_push(, opt_gpg_sign);
>> -
>> + if(opt_autostash)
>
> Style: control keywords are followed 

[PATCH v2 2/2] Documentation/git-pull.txt: teach 'git pull --rebase' the --[no-]autostash option.

[Mehul Jain]

git pull --rebase understands --[no-]autostash flag.

This flag overrides config variable "rebase.autoStash" 
if set (default is false).

When calling "git pull --rebase" with "--autostash",
pull passes the "--autostash" option to rebase, 
which then runs rebase on a dirty worktree.

With "--no-autostash" option, the command will die
if the worktree is dirty, before calling rebase.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 Documentation/git-pull.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index a62a2a6..ce22c52 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -128,6 +128,22 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
Override earlier --rebase.
 
+--autostash::
+   Automatically create a temporary stash before the operation
+   begins, and apply it after the operation ends. This means
+   that you can run rebase on a dirty worktree.
++
+This option is only valid when '--rebase' option is used.
++
+[NOTE]
+Use with care: the final stash application after a successful
+rebase might result in non-trivial conflicts.
+
+--no-autostash::
+   If the '--autostash' option is enabled by default using the
+   configuration variable `rebase.autoStash`, this option can be
+   used to override this setting.
+
 Options related to fetching
 ~~~
 
-- 
2.7.1.340.g69eb491.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

[PATCH v2 1/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

git pull --rebase understands --[no-]autostash flag.

This flag overrides config variable "rebase.autoStash" 
if set (default is false).

When calling "git pull --rebase" with "--autostash",
pull passes the "--autostash" option to rebase, 
which then runs rebase on a dirty worktree.

With "--no-autostash" option, the command will die
if the worktree is dirty, before calling rebase.

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---

Thanks to Paul and Matthieu for comments on previous round.
Changes:
- --autostash flag added
- OPT_COLOR_FLAG replaced by OPT_BOOL
- Default value of opt_rebase changed
- Few changes in code
- Commit message improvements
- Documentation added
- Few tests removed as suggested by Paul
- Added test for --autostash flag
All tests passed: https://travis-ci.org/mehul2029/git 

 builtin/pull.c  | 13 -
 t/t5520-pull.sh | 19 +++
 t/t5521-pull-options.sh | 16 
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..60b320e 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -85,6 +85,7 @@ static char *opt_squash;
 static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
+static int opt_autostash = 0;
 static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -146,6 +147,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "ff-only", _ff, NULL,
N_("abort if fast-forward is not possible"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+   OPT_BOOL(0,"autostash",_autostash,
+   N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
@@ -789,7 +792,8 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
-
+   if(opt_autostash)
+   argv_array_push(,"--autostash");
argv_array_push(, "--onto");
argv_array_push(, sha1_to_hex(merge_head));
 
@@ -813,6 +817,8 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (!getenv("GIT_REFLOG_ACTION"))
set_reflog_message(argc, argv);
 
+   git_config_get_bool("rebase.autostash",_autostash);
+
argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
parse_repo_refspecs(argc, argv, , );
@@ -835,13 +841,10 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
-
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if (!opt_autostash)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..d80b6cc 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -245,6 +245,25 @@ test_expect_success '--rebase fails with multiple 
branches' '
test modified = "$(git show HEAD:file)"
 '
 
+test_expect_success 'pull --rebase --no-autostash fails with dirty working 
directory and rebase.autstash set false' '
+   test_config rebase.autostash true &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   test_must_fail git pull --rebase --no-autostash . copy
+'
+
+test_expect_success 'pull --rebase --autostash succeeds with dirty working 
directory and rebase.autstash set false' '
+   test_config rebase.autostash false &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   git pull --rebase --autostash . copy &&
+   test_cmp_rev HEAD^ copy &&
+   test "$(cat new_file)" = dirty &&
+   test "$(cat file)" = "modified again"
+'
+
 test_expect_success 'pull --rebase succeeds with dirty working directory and 
rebase.autostash set' '
test_config rebase.autostash true &&
git reset --hard before-rebase &&
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
inde

Re: [PATCH] Add --no-autostash flag to git pull --rebase

[Mehul Jain]

Hi,

Thank you for the suggestions.
On Fri, Feb 26, 2016 at 6:17 PM, Paul Tan <pyoka...@gmail.com> wrote:
> Some grammatical/spelling nits below:

Many apologies for my English.

> I think git-pull's documentation should be updated as well to talk
> about this new command-line switch.

OK.

>> diff --git a/builtin/pull.c b/builtin/pull.c
>> index 10eff03..9d1a3d0 100644
>> --- a/builtin/pull.c
>> +++ b/builtin/pull.c
>> @@ -85,6 +85,7 @@ static char *opt_squash;
>>  static char *opt_commit;
>>  static char *opt_edit;
>>  static char *opt_ff;
>> +static int opt_autostash = -1;
>>  static char *opt_verify_signatures;
>>  static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
>>  static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
>> @@ -146,6 +147,8 @@ static struct option pull_options[] = {
>> OPT_PASSTHRU(0, "ff-only", _ff, NULL,
>> N_("abort if fast-forward is not possible"),
>> PARSE_OPT_NOARG | PARSE_OPT_NONEG),
>> +   OPT_COLOR_FLAG(0,"autostash",_autostash,
>> +   N_("abort if tree is dirty")),
>
> Why OPT_COLOR_FLAG()? And --autostash is not just about aborting if
> the working tree is dirty. Why not just copy the help message from
> git-rebase? Something like:
> "automatically stash/stash pop before and after rebase"

Using OPT_COLOR_FLAG() is wrong, I agree. OPT_BOOL will be a better option.
N_("automatically stash/stash pop before and after rebase") is better.

>> OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
>> N_("verify that the named commit has a valid GPG signature"),
>> PARSE_OPT_NOARG),
>> @@ -835,13 +838,14 @@ int cmd_pull(int argc, const char **argv, const char 
>> *prefix)
>> hashclr(orig_head);
>>
>> if (opt_rebase) {
>> -   int autostash = 0;
>> -
>> if (is_null_sha1(orig_head) && !is_cache_unborn())
>> die(_("Updating an unborn branch with changes added 
>> to the index."));
>>
>> -   git_config_get_bool("rebase.autostash", );
>> -   if (!autostash)
>> +   if(opt_autostash < 0)
>> +   
>> if(git_config_get_bool("rebase.autostash",_autostash))
>> +   opt_autostash = 0;
>
> I wonder if this code could be shortened if we simply just called
> git_config_get_bool() just before parse_options(). That way, we don't
> need to check for the "-1" special value.

Definitely. This way opt_autostash can be initialized with 0, thus default
will be false.

>>
>> +   if (!opt_autostash)
>> die_on_unclean_work_tree(prefix);
>
> OK.
>
>>
>> if (get_rebase_fork_point(rebase_fork_point, repo, 
>> *refspecs))
>> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
>> index c952d5e..512d3bf 100755
>> --- a/t/t5520-pull.sh
>> +++ b/t/t5520-pull.sh
>> @@ -245,6 +245,14 @@ test_expect_success '--rebase fails with multiple 
>> branches' '
>> test modified = "$(git show HEAD:file)"
>>  '
>>
>> +test_expect_success '--rebase --no-autostash fails with dirty working 
>> directory' '
>
> Maybe add ..."and rebase.autostash set" to the test name? Describes
> the test better, and is consistent with the name of the test below.

Can be done. But which one of these will be more appropriate:
 "rebase.autostash set" or "rebase.autostash set true".
I prefer latter, as it will maintain consistence with the test name of
"--rebase --autostash", which will be
'--rebase --autostash succeeds with dirty working directory and
rebase.autostash set false.'


>> +test_expect_success 'git pull -q --rebase --no-autostash' '
>> +   mkdir clonedqrbnas &&
>> +   (cd clonedqrbnas  && git init &&
>> +   git pull -q --rebase --no-autostash "../parent" >out 2>err &&
>> +   test_must_be_empty err &&
>> +   test_must_be_empty out)
>> +'
>> +
>> +test_expect_success 'git pull -v --rebase --no-autostash' '
>> +   mkdir clonedvrbnas &&
>> +   (cd clonedvrbnas && git init &&
>> +   git pull -v --rebase --no-autostash "../parent" >out 2>err &&
>> +   test -s err &&
>> +   test_must_be_empty out)
>> +'
>
> While more tests are always good, I don't think we need to test for
> "-q" and "-v" with --no-autostash, because it's already covered by the
> test for "git pull -q --rebase". Perhaps with --autostash, but even
> then I don't think we need a test for "-v".

OK then. I will only add tests for "git pull --rebase --no-autostash",
"git pull --rebase --autostash" and
"git pull -q --rebase --autostash" in t5521-pull-options.sh

Thanks,
Mehul Jain
--
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: [GSoC] Microproject :- Teaching git pull --rebase the --no-autostash flag

[Mehul Jain]

On Fri, Feb 26, 2016 at 5:21 PM, Paul Tan  wrote:
> That was the point of the microproject ;-). --[no-]autostash means
> both --autostash and --no-autostash.

Oops, my bad. I will add the necessary changes :-).

Thanks,
Mehul
--
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] Add --no-autostash flag to git pull --rebase

[Mehul Jain]

git pull --rebase now understand --no-autostash flag. If directory is found
to be dirty then command will die. This flag override "rebase.autostash" 
configuration(if set). If this flag is not passed in command line then
default behaviour is choosen, given by "rebase.autostash"(if "rebase.autostash"
is not set then git pull --rebase will die if directory is dirty).

Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
---
 builtin/pull.c  | 12 
 t/t5520-pull.sh |  8 
 t/t5521-pull-options.sh | 24 
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..9d1a3d0 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -85,6 +85,7 @@ static char *opt_squash;
 static char *opt_commit;
 static char *opt_edit;
 static char *opt_ff;
+static int opt_autostash = -1;
 static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -146,6 +147,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "ff-only", _ff, NULL,
N_("abort if fast-forward is not possible"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+   OPT_COLOR_FLAG(0,"autostash",_autostash,
+   N_("abort if tree is dirty")),
OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
@@ -835,13 +838,14 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
hashclr(orig_head);
 
if (opt_rebase) {
-   int autostash = 0;
-
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to 
the index."));
 
-   git_config_get_bool("rebase.autostash", );
-   if (!autostash)
+   if(opt_autostash < 0)
+   
if(git_config_get_bool("rebase.autostash",_autostash))
+   opt_autostash = 0;
+
+   if (!opt_autostash)
die_on_unclean_work_tree(prefix);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index c952d5e..512d3bf 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -245,6 +245,14 @@ test_expect_success '--rebase fails with multiple 
branches' '
test modified = "$(git show HEAD:file)"
 '
 
+test_expect_success '--rebase --no-autostash fails with dirty working 
directory' '
+   test_config rebase.autostash true &&
+   git reset --hard before-rebase &&
+   echo dirty >new_file &&
+   git add new_file &&
+   test_must_fail git pull --rebase --no-autostash . copy
+'
+
 test_expect_success 'pull --rebase succeeds with dirty working directory and 
rebase.autostash set' '
test_config rebase.autostash true &&
git reset --hard before-rebase &&
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 18372ca..22ff5d7 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -62,6 +62,30 @@ test_expect_success 'git pull -v --rebase' '
test_must_be_empty out)
 '
 
+test_expect_success 'git pull --rebase --no-autostash' '
+   mkdir clonedrbnas &&
+   (cd clonedrbnas  && git init &&
+   git pull --rebase --no-autostash "../parent" >out 2>err &&
+   test -s err &&
+   test_must_be_empty out)
+'
+
+test_expect_success 'git pull -q --rebase --no-autostash' '
+   mkdir clonedqrbnas &&
+   (cd clonedqrbnas  && git init &&
+   git pull -q --rebase --no-autostash "../parent" >out 2>err &&
+   test_must_be_empty err &&
+   test_must_be_empty out)
+'
+
+test_expect_success 'git pull -v --rebase --no-autostash' '
+   mkdir clonedvrbnas &&
+   (cd clonedvrbnas && git init &&
+   git pull -v --rebase --no-autostash "../parent" >out 2>err &&
+   test -s err &&
+   test_must_be_empty out)
+'
+
 test_expect_success 'git pull -v -q' '
mkdir clonedvq &&
(cd clonedvq && git init &&
-- 
2.7.1.340.g69eb491.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

[GSoC] Microproject :- Teaching git pull --rebase the --no-autostash flag

[Mehul Jain]

With this patch, git pull --rebase will understand --no-autostash command line 
flag.
This flag will override "rebase.autostash" configuration(if set) and leads to a
failure if current working directory is dirty. If "rebase.autostash" is not 
configured
and no flag is passed then by default command will die if directory is to be 
dirty, before
even calling rebase.

I am also thinking of adding a "--autostash" flag for git pull --rebase, which 
will
override "rebase.autostash" configuration(if set false) and will pass 
--autostash to
git rebase. I would like to know your opinion on this option. 

Thanks! 

Mehul Jain (1):
  Add --no-autostash flag to git pull --rebase

 builtin/pull.c  | 12 
 t/t5520-pull.sh |  8 
 t/t5521-pull-options.sh | 24 
 3 files changed, 40 insertions(+), 4 deletions(-)

-- 
2.7.1.340.g69eb491.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: GSoC 2016: Microproject

[Mehul Jain]

On Mon, Feb 22, 2016 at 4:51 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> OK server is up but very likely misconfigured. If you have experience
> with http server before (I think this is apache), then you can dig in
> t/lib-httpd.sh, study how the server is configured and try to fix it.

Thanks Duy. :)  All tests passed. I configured apache server to listen
to the ports tests were trying to access.

Thanks Lars. I tested on Traivs-CI, worked fine. :)

Mehul Jain
--
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: GSoC 2016: Microproject

[Mehul Jain]

On Mon, Feb 22, 2016 at 3:50 PM, Duy Nguyen  wrote:
> You may have an http server installed but not suitable for these
> tests. Try running one test file alone with -v -i, e.g.
> ./t5539-fetch-http-shallow.sh -v -i and post the output.

Here's the output :-

expecting success:
git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
git clone $HTTPD_URL/smart/repo.git clone &&
(
cd clone &&
git fsck &&
git log --format=%s origin/master >actual &&
cat actual &&
#cat 

Re: GSoC 2016: Microproject

[Mehul Jain]

On Mon, Feb 22, 2016 at 12:22 AM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:
> The simplest way to get back on track for you is probably to start over
> with a fresh clone, or (warning: destructive operations): use git clean
> to remove untracked files.

Hello Matthieu,

I followed your advise and cloned a fresh copy of git source code.
After compiling it and running the test with " prove --timer --jobs 15
./t[0-9]*.sh" command, I received tests failure. All these tests are
regarding HTTP protocol being invoked like
t5539-fetch-http-shallow.sh.

I'm behind a proxy server which blocks all ports except 80 and 443.
Also my .gitconfig file is properly configured for proxy. Can these
tests failure be triggered because of proxy server?

Thanks,
Mehul Jain
--
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: GSoC 2016: Microproject

[Mehul Jain]

On Sun, Feb 21, 2016 at 10:25 AM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:
> Please, don't top-post on this list.

I apologize for top-posting on the list.

> Are you sure that 1) you have no uncommitted change and 2) you have
> compiled what you have in your tree?

1) Yes, I have no uncommited change in the branch(master).
2) Yes, I compiled before testing.

Earlier when I was testing the master branch on my pc, I used "make"
in \t directory, which lead to failure of test #2, #3 in
t5539-fetch-http-shallow.sh .
Afterwards I switched to sudo mode and ran the make command again.
This time all test of  t5539-fetch-http-shallow.sh passed, but test
#32 of file t7300-clean.sh failed. To crosscheck, I ran " sudo sh
./t7300-clean.sh -v --run='1-32' " which gave the following error
message -

Skipping repository baz/boo
Skipping repository foo/
Removing possible_sub1/
Skipping repository repo/
Skipping repository sub2/
Removing to_clean/
File possible_sub1/.git doesn't exist.
not ok 32 - should avoid cleaning possible submodules
#
# rm -fr to_clean possible_sub1 &&
# mkdir to_clean possible_sub1 &&
# test_when_finished "rm -rf possible_sub*" &&
# echo "gitdir: foo" >possible_sub1/.git &&
# >possible_sub1/hello.world &&
# chmod 0 possible_sub1/.git &&
# >to_clean/should_clean.this &&
# git clean -f -d &&
# test_path_is_file possible_sub1/.git &&
# test_path_is_file possible_sub1/hello.world &&
# test_path_is_missing to_clean
#

I haven't made any commits/changes in the master branch. Can you
please suggest where things are going wrong.

Thanks
Mehul Jain
--
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: GSoC 2016: Microproject

[Mehul Jain]

On Fri, Feb 19, 2016 at 11:20 PM, Stefan Beller <sbel...@google.com> wrote:
> Yes, most likely t/t5521-pull-options.sh or t/t5520-pull.sh would be the right
> place as judging from the file name of the tests.

I checked out both of the files and made following observations -

1) As I'm going to add an option for "git pull --rebase"; tests should
be added for following commands in t/t5521-pull-options.sh
  a)  git pull --rebase --[no-]autostash
  b)  git pull -v --rebase --[no-]autostash
  c)  git pull -q --rebase --[no-]autostash

2) Also test for "pull --rebase --[no-]autostash fails with dirty
working tree and rebase.autostash set" should be added to
t/t5520-pull.sh.

Have I made the right observation?

Thanks,
Mehul Jain
--
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: GSoC 2016: Microproject

[Mehul Jain]

Hello,

I faced the following problem when I tested master branch. Here I have
made no commits to master branch.

*** t5539-fetch-http-shallow.sh ***
ok 1 - setup shallow clone
not ok 2 - clone http repository
#
# git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
# git clone $HTTPD_URL/smart/repo.git clone &&
# (
# cd clone &&
# git fsck &&
# git log --format=%s origin/master >actual &&
# cat <expect &&
# 7
# 6
# 5
# 4
# 3
# EOF
# test_cmp expect actual
# )
#
not ok 3 - no shallow lines after receiving ACK ready
#
# (
# cd shallow &&
# test_tick &&
# for i in $(test_seq 15)
# do
# git checkout --orphan unrelated$i &&
# test_commit unrelated$i &&
# git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
# refs/heads/unrelated$i:refs/heads/unrelated$i &&
# git push -q ../clone/.git \
# refs/heads/unrelated$i:refs/heads/unrelated$i ||
# exit 1
# done &&
# git checkout master &&
# test_commit new &&
# git push  "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master
# ) &&
# (
# cd clone &&
# git checkout --orphan newnew &&
# test_commit new-too &&
# GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 &&
# grep "fetch-pack< ACK .* ready" ../trace &&
# ! grep "fetch-pack> done" ../trace
# )
#
# failed 2 among 3 test(s)
1..3
make[1]: *** [t5539-fetch-http-shallow.sh] Error 1
make[1]: Leaving directory `/home/mj/git/t'
make: *** [test] Error 2

Is this test failure usual with linux based system or just happened with me.
I'm running Ubuntu 14.04.

thanks,
Mehul Jain

On Fri, Feb 19, 2016 at 11:20 PM, Stefan Beller <sbel...@google.com> wrote:
> On Fri, Feb 19, 2016 at 9:39 AM, Mehul Jain <mehul.jain2...@gmail.com> wrote:
>> On Fri, Feb 19, 2016 at 6:33 PM, Matthieu Moy
>> <matthieu@grenoble-inp.fr> wrote:
>>> Hi,
>>>
>>> This is a double-post. You've posted almost the same message under the
>>> title "GSoC 2016". Nothing serious, but attention to details is
>>> important if you want to give a good image of yourself.
>>
>> I'm sorry of this kind of behavior. It was due to a misunderstanding by my 
>> side.
>>
>>> I don't have all the code in mind, but I think it is. In this situation,
>>> you always end up with a variable telling Git what to do (I guess,
>>> autostash here), and this variable is set according to the configuration
>>> and the command-line.
>>>
>>> You should be careful about the precedence: command-line should override
>>> the configuration. And the default behavior should be used if neither
>>> the command-line nor the configuration specified otherwise.
>>
>> Thanks for the suggestion.
>> I've started the work on patch and did the change in the code which
>> were necessary for Microproject. I have run the test by using "make",
>> and there was no failure of any test.
>> I've a doubt regarding tests. Here as "git pull" will now understand
>> the argument that  "--[no-]autostash" means "rebase.autostash" should
>> be set false for current execution of command "git pull --rebase". So
>> do I have to write a test for this new option?
>>
>
> Yes, most likely t/t5521-pull-options.sh or t/t5520-pull.sh would be the right
> place as judging from the file name of the tests.
>
> Thanks,
> Stefan
>
>> Mehul Jain
>> --
>> 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
--
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: GSoC 2016: Microproject

[Mehul Jain]

On Fri, Feb 19, 2016 at 11:20 PM, Stefan Beller <sbel...@google.com> wrote:
> Yes, most likely t/t5521-pull-options.sh or t/t5520-pull.sh would be the right
> place as judging from the file name of the tests.

Thanks for specifying the files. I think t/t5520-pull.sh line 258 will
be the best place to write test for this case.

Mehul Jain
--
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: GSoC 2016: Microproject

[Mehul Jain]

On Fri, Feb 19, 2016 at 6:33 PM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:
> Hi,
>
> This is a double-post. You've posted almost the same message under the
> title "GSoC 2016". Nothing serious, but attention to details is
> important if you want to give a good image of yourself.

I'm sorry of this kind of behavior. It was due to a misunderstanding by my side.

> I don't have all the code in mind, but I think it is. In this situation,
> you always end up with a variable telling Git what to do (I guess,
> autostash here), and this variable is set according to the configuration
> and the command-line.
>
> You should be careful about the precedence: command-line should override
> the configuration. And the default behavior should be used if neither
> the command-line nor the configuration specified otherwise.

Thanks for the suggestion.
I've started the work on patch and did the change in the code which
were necessary for Microproject. I have run the test by using "make",
and there was no failure of any test.
I've a doubt regarding tests. Here as "git pull" will now understand
the argument that  "--[no-]autostash" means "rebase.autostash" should
be set false for current execution of command "git pull --rebase". So
do I have to write a test for this new option?

Mehul Jain
--
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

GSoC 2016: Microproject

[Mehul Jain]

Hello everyone,

I'm Mehul Jain. I'm looking for participating in GSoC 2016.

I've started work on a Microproject" Teach git pull --rebase the
--no-autostash" option. While looking at Git's source code I have made
following observation: In the pull.c file
1.  git_config_get_bool( , ) search in the configuration key for the
value of rebase.autostash, if found true then modify autostash's value
to a non-zero number and thus making a stash to encounter the problem
of dirty tree.
2. Here if in command line a flag "--no-autostash" is given then we
can easily set the value of autostash = 0 and thus killing the process
by die_on_unclean_work_tree(prefix).
Is my observation is right?

I'm new to open source projects and not much experienced at it. So
please correct/comment on any mistake that I made while trying to put
my point. I will also appreciate any comment/suggestion/criticism on
my observation.

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

GSoC 2016

[Mehul Jain]

Hello everyone,

I'm Mehul Jain. I'm looking for participating in GSoC 2016.

I've started work on a Microproject" Teach git pull --rebase the
--no-autostash" option. While looking at Git's source code I have made
following observation: In the pull.c file
1.  git_config_get_bool( , ) search in the configuration key for the
value of rebase.autostash, if found true then modify autostash's value
to a non-zero number and thus making a stash to encounter the problem
of dirty tree.
2. Here if in command line a flag "--no-autostash" is given then we
can easily set the value of autostash = 0 and thus killing the process
by die_on_unclean_work_tree(prefix).
Is my observation is right?

I'm new to open source projects and not much experienced at it. So
please correct/comment on any mistake that I made while trying to put
my point. I will also appreciate any comment/suggestion/criticism on
my observation.

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