[sort-of-BUG] merge-resolve cannot resolve "content/mode" conflict

2016-04-02 Thread Jeff King
Imagine a merge where one side changes the content of a path and the
other changes the mode. Here's a minimal reproduction:

  git init repo && cd repo &&

  echo base >file &&
  git add file &&
  git commit -m base &&

  echo changed >file &&
  git commit -am content &&

  git checkout -b side HEAD^
  chmod +x file &&
  git commit -am mode

If I merge that with merge-recursive, I get what you'd expect: mode
10755, and content "changed".

However, with merge-resolve, I get a conflict:

  $ git merge -s resolve master
  Trying really trivial in-index merge...
  error: Merge requires file-level merging
  Nope.
  Trying simple merge.
  Simple merge failed, trying Automatic merge.
  Auto-merging file
  ERROR: permissions conflict: 100644->100755,100644 in file
  fatal: merge program failed
  Automatic merge failed; fix conflicts and then commit the result.

I think this is only a half-bug, really. It's definitely a funny
situation, and it's not unreasonable for a merge driver to punt on a
funny situation rather than resolving it. But I would say:

  - it would probably be a nice improvement to resolve this as
merge-recursive does

  - the "ERROR" message is silly and misleading; the permissions resolve
just fine, it is only that the combination with the content-level
change confuses the script (but the output does not mention that).

This is a leftover from my experiments with merge-resolve versus
merge-recursive last fall, which resulted in a few actual bug-fixes. I
looked into fixing this case, too, at that time. It seemed possible, but
a little more involved than you might think (because the logic is driven
by a bunch of case statements, and this adds a multiplicative layer to
the cases; we might need to resolve the permissions, and _then_ see if
the content can be resolved).

So I didn't actually come up with a patch, but I figured I'd write it up
here for posterity. And just didn't get around to it until now.

-Peff
--
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 4/4] tag: use pgp_verify_function in tag -v call

2016-04-02 Thread Jeff King
On Sat, Apr 02, 2016 at 07:16:15PM -0400, santi...@nyu.edu wrote:

> diff --git a/builtin/tag.c b/builtin/tag.c
> index 1705c94..3dffdff 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -65,9 +65,10 @@ static int list_tags(struct ref_filter *filter, struct 
> ref_sorting *sorting, con
>  }
>  
>  typedef int (*each_tag_name_fn)(const char *name, const char *ref,
> - const unsigned char *sha1);
> + const unsigned char *sha1, unsigned flags);

I'm not sure it's a good idea to add a flags field here; most of the
callbacks don't use it, and as you probably noticed, it makes the patch
a lot noisier. It does let you directly use pgp_verify_tag like this:

>   if (cmdmode == 'v')
> - return for_each_tag_name(argv, verify_tag);
> + return for_each_tag_name(argv, pgp_verify_tag,
> + GPG_VERIFY_VERBOSE);

but I think that is coupling too closely. What happens later when the
public, multi-file pgp_verify_tag function changes its interface? Or we
want to change our interface here, and it no longer matches
pgp_verify_tag? The results ripple a lot further than they should.

I think you probably want to keep a simple adapter callback in this
file, like:

  int verify_tag(const char *name, const char *ref, const unsigned char *sha1)
  {
return pgp_verify_tag(name, GPG_VERIFY_VERBOSE));
  }

> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> index f776778..8abc357 100644
> --- a/builtin/verify-tag.c
> +++ b/builtin/verify-tag.c
> @@ -30,6 +30,8 @@ int cmd_verify_tag(int argc, const char **argv, const char 
> *prefix)
>  {
>   int i = 1, verbose = 0, had_error = 0;
>   unsigned flags = 0;
> + unsigned char sha1[20];
> + const char *name;
>   const struct option verify_tag_options[] = {
>   OPT__VERBOSE(, N_("print tag contents")),
>   OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
> GPG_VERIFY_RAW),
> @@ -46,8 +48,16 @@ int cmd_verify_tag(int argc, const char **argv, const char 
> *prefix)
>   if (verbose)
>   flags |= GPG_VERIFY_VERBOSE;
>  
> - while (i < argc)
> - if (pgp_verify_tag(argv[i++], flags))
> + while (i < argc) {
> + name = argv[i++];
> + if (get_sha1(name, sha1)) {
> + error("tag '%s' not found.", name);
>   had_error = 1;
> + }
> +
> + if (pgp_verify_tag(name, NULL, sha1, flags))
> + had_error = 1;
> +
> + }

So this is a good example of the rippling I mentioned earlier.

As a side note, it might actually be an improvement for pgp_verify_tag
to take a sha1 (so that git-tag is sure that it is verifying the same
object that it is printing), but that refactoring should probably come
separately, I think.

-Peff
--
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/4] builtin/verify-tag: move verification code to tag.c

2016-04-02 Thread Jeff King
On Sat, Apr 02, 2016 at 07:16:14PM -0400, santi...@nyu.edu wrote:

> From: Santiago Torres 
> 
> The PGP verification routine for tags could be accessed by other
> commands that require it. We do this by moving it to the common tag.c
> code. We rename the verify_tag() function to pgp_verify_tag() to avoid
> conflicts with the mktag.c function.

One nit: even though GPG is just an implementation of PGP, and
technically the standard and formats are called PGP, we tend to name
everything GPG in the code. So this probably should be gpg_verify_tag().

> - len = parse_signature(buf, size);
> -
> - if (size == len) {
> - if (flags & GPG_VERIFY_VERBOSE)
> - write_in_full(1, buf, len);
> - return error("no signature found");
> - }
> [...]
> + payload_size = parse_signature(buf, size);
> +
> + if (size == payload_size) {
> + write_in_full(1, buf, payload_size);
> + return error("No PGP signature found in this tag!");
> + }

I'm happy to see the more readable variable name here. I wonder if we
should leave the error message as-is, though, as this is just supposed
to be about code movement (and if we are changing it, it should adhere
to our usual style of not starting with a capital letter, and not ending
in punctuation).

-Peff
--
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 2/4] t/t7030-verify-tag.sh: Adds validation for multiple tags

2016-04-02 Thread Jeff King
On Sat, Apr 02, 2016 at 07:16:13PM -0400, santi...@nyu.edu wrote:

> The verify-tag command supports mutliple tag names as an argument.

s/mutliple/multiple/

> +test_expect_success GPG 'verify multiple tags' '
> + git verify-tag -v --raw fourth-signed sixth-signed seventh-signed 
> 2>actual 1> tagnames &&

Style: we don't put a space between ">" and the filename. Also, we
usually omit "1" when redirecting stdout.

> + grep -c "GOODSIG" actual > count &&

Funny indentation here.

I wondered if we could use test_cmp instead of a counting grep here, but
this is looking at gpg spew, and we probably don't want to count on that
never changing.

I don't see us actually verifying that "count" is 3, though.

> + ! grep "BADSIG" actual &&

Makes sense...

> + grep -E "tag\ .*" tagnames | uniq -u | wc - l | grep "3"

Do we need "grep -E" here? I don't see any extended regex in use. Is
there a reason to backslash-escape the space?

Your "wc -l" has an extra space, which means "read stdin, and then the
file 'l'". Which sort-of happens to work, except as you noticed, you
have to grep for "3" instead of matching it.

I think, though, that rather than counting we could just write what we
expect into a file and compare that. It makes it easier for somebody
reading the test to see what it is we're trying to do.

In fact, I suspect you could replace the "GOODSIG" check as well by
doing something like:

  # verifying 3 tags in one invocation should be exactly like
  # verifying the 3 separately
  tags="fourth-signed sixth-signed seventh-signed"
  for i in $tags; do
  git verify-tag -v --raw $i || return 1
  done >expect.stdout 2>expect.stderr &&
  git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr &&
  test_cmp expect.stdout actual.stdout &&
  test_cmp expect.stderr actual.stderr

but I didn't test it.

-Peff
--
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/4] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

2016-04-02 Thread Jeff King
On Sat, Apr 02, 2016 at 07:16:12PM -0400, santi...@nyu.edu wrote:

> From: Santiago Torres 
> 
> The verify_signed_buffer comand might cause a SIGPIPE signal when the
> gpg child process terminates early (due to a bad keyid, for example) and
> git tries to write to it afterwards. Previously, ignoring SIGPIPE was
> done on the builtin/gpg-verify.c command to avoid this issue. However,

s/gpg-verify/verify-tag/ here, I think.

Other than that, nicely explained.

-Peff
--
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] branch: fix shortening of non-remote symrefs

2016-04-02 Thread Jeff King
On Sun, Apr 03, 2016 at 02:54:22PM +1200, Phil Sainty wrote:

> Given the following symbolic reference:
> 
> $ git symbolic-ref refs/heads/m refs/heads/master
> 
> 
> Correct in 2.6.6:
> 
> $ PATH=~/git/git-2.6.6:$PATH git branch
>   m -> master
> * master
> 
> 
> Wrong in 2.7.0:
> 
> $ PATH=~/git/git-2.7.0:$PATH git branch
>   m -> m
> * master

Thanks for an easy test case. Though we don't officially support
arbitrary symrefs in the ref namespace, they do mostly work. And
certainly the current output is nonsense, and it worked before. This
bisects to aedcb7d (branch.c: use 'ref-filter' APIs, 2015-09-23).

The fix is below. Karthik, I didn't look at all how this interacts with
your work to convert branch to ref-filter for printing. I imagine it
drops this code completely, but we should make sure that ref-filter gets
this case right. I almost didn't prepare this patch at all, but I
suspect we may want it for "maint", while the full conversion would wait
for "master".

-- >8 --
Subject: branch: fix shortening of non-remote symrefs

Commit aedcb7d (branch.c: use 'ref-filter' APIs, 2015-09-23)
adjusted the symref-printing code to look like this:

if (item->symref) {
skip_prefix(item->symref, "refs/remotes/", );
strbuf_addf(, " -> %s", desc);
}

This has three bugs in it:

  1. It always skips past "refs/remotes/", instead of
 skipping past the prefix associated with the branch we
 are showing (so commonly we see "refs/remotes/" for the
 refs/remotes/origin/HEAD symref, but the previous code
 would skip "refs/heads/" when showing a symref it found
 in refs/heads/.

  2. If skip_prefix() does not match, it leaves "desc"
 untouched, and we show whatever happened to be in it
 (which is the refname from a call to skip_prefix()
 earlier in the function).

  3. If we do match with skip_prefix(), we stomp on the
 "desc" variable, which is later passed to
 add_verbose_info(). We probably want to retain the
 original refname there (though it likely doesn't matter
 in practice, since after all, one points to the other).

The fix to match the original code is fairly easy: record
the prefix to strip based on item->kind, and use it here.
However, since we already have a local variable named "prefix",
let's give the two prefixes verbose names so we don't
confuse them.

Signed-off-by: Jeff King 
---
The test makes sure we restored the v2.6.x behavior, namely that
cross-prefix symrefs will not be shortened at all. It might be nice to
show:

  ref-to-remote -> remotes/origin/branch-one

or something, but that should be separate from the fix (and I don't
overly care either way, so I probably won't work on it).

 builtin/branch.c | 19 ---
 t/t3203-branch-output.sh | 12 
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 7b45b6b..f6c23bf 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -393,22 +393,25 @@ static void format_and_print_ref_item(struct 
ref_array_item *item, int maxwidth,
int current = 0;
int color;
struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
-   const char *prefix = "";
+   const char *prefix_to_show = "";
+   const char *prefix_to_skip = NULL;
const char *desc = item->refname;
char *to_free = NULL;
 
switch (item->kind) {
case FILTER_REFS_BRANCHES:
-   skip_prefix(desc, "refs/heads/", );
+   prefix_to_skip = "refs/heads/";
+   skip_prefix(desc, prefix_to_skip, );
if (!filter->detached && !strcmp(desc, head))
current = 1;
else
color = BRANCH_COLOR_LOCAL;
break;
case FILTER_REFS_REMOTES:
-   skip_prefix(desc, "refs/remotes/", );
+   prefix_to_skip = "refs/remotes/";
+   skip_prefix(desc, prefix_to_skip, );
color = BRANCH_COLOR_REMOTE;
-   prefix = remote_prefix;
+   prefix_to_show = remote_prefix;
break;
case FILTER_REFS_DETACHED_HEAD:
desc = to_free = get_head_description();
@@ -425,7 +428,7 @@ static void format_and_print_ref_item(struct ref_array_item 
*item, int maxwidth,
color = BRANCH_COLOR_CURRENT;
}
 
-   strbuf_addf(, "%s%s", prefix, desc);
+   strbuf_addf(, "%s%s", prefix_to_show, desc);
if (filter->verbose) {
int utf8_compensation = strlen(name.buf) - 
utf8_strwidth(name.buf);
strbuf_addf(, "%c %s%-*s%s", c, branch_get_color(color),
@@ -436,8 +439,10 @@ static void format_and_print_ref_item(struct 
ref_array_item *item, int maxwidth,
name.buf, branch_get_color(BRANCH_COLOR_RESET));
 
if (item->symref) {
-   skip_prefix(item->symref, "refs/remotes/", );
-   

RFD: removing git.spec.in (Re: git 2.8.0 tarball rpmbuild error)

2016-04-02 Thread Junio C Hamano
It is likely that I'll cut 2.8.1 with only the attached patch

Message-ID: <1459494651-32618-1-git-send-email-matthieu@imag.fr>
aka $gmane/290510

and I'll explicitly mark that this maintenance release is ignorable
by people other than those who build their own RPM packages from my
tree.

I think by now it is very clear that nobody active in the Git
development community tests RPM binaries built with git.spec.in we
have in our tree.  I suspect RPM based distros are using their own
RPM build recipe without paying any attention to what we have in our
tree, and that is why no packager from RPM land gave any bug report
and correction before the release happened.

I'd propose that during the cycle for the next feature release, we'd
remove git.spec.in and stop pretending as if we support RPM packaging.

If a group of people feel strongly against the removal, they are
welcome to form a volunteer group and promise to be responsible for
regularly testing the tip of 'next' so that a breakage like this one
will never slip to the 'master' branch.

Unless we see such a promise from dedicated group of people (whose
size does not have to be more than 1), I think it is more harmful
for us to pretend that we support RPM packaging out of our tree than
being honest about it and removing the (pretense of) support.

Discuss.

Thanks.

-- >8 --
Subject: [PATCH] git.spec.in: use README.md, not README
From: Matthieu Moy 

The file was renamed in 4ad21f5 (README: use markdown syntax,
2016-02-25), but that commit forgot to update git.spec.in.

Reported-by: Ron Isaacson 
Signed-off-by: Matthieu Moy 
---
 git.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git.spec.in b/git.spec.in
index d61d537..bfd1cfb 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -146,7 +146,7 @@ rm -rf $RPM_BUILD_ROOT
 %files -f bin-man-doc-files
 %defattr(-,root,root)
 %{_datadir}/git-core/
-%doc README COPYING Documentation/*.txt
+%doc README.md COPYING Documentation/*.txt
 %{!?_without_docs: %doc Documentation/*.html Documentation/howto}
 %{!?_without_docs: %doc Documentation/technical}
 %{_sysconfdir}/bash_completion.d
-- 
2.7.2.334.g35ed2ae.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


Since 2.7.0 git branch displays symbolic references as ref -> ref instead of ref -> branch

2016-04-02 Thread Phil Sainty

Given the following symbolic reference:

$ git symbolic-ref refs/heads/m refs/heads/master


Correct in 2.6.6:

$ PATH=~/git/git-2.6.6:$PATH git branch
  m -> master
* master


Wrong in 2.7.0:

$ PATH=~/git/git-2.7.0:$PATH git branch
  m -> m
* master


Still wrong in current version 2.8.0:

$ PATH=~/git/git-2.8.0:$PATH git branch
  m -> m
* master



As the new output isn't very useful, I assume this is an
unintentional bug/regression.

The 2.6.6 output has been used since at least version 1.7.12.4
(when I first started making use of this ability).


-Phil

--
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: Bug Report

2016-04-02 Thread Jacob Keller
Hi,

On Sat, Apr 2, 2016 at 5:25 PM, Benjamin Sandeen
 wrote:
> Today, I managed to create a duplicate branch in a git repository.  While
> this may not be a bug per se, I do think that it is confusing and some way
> of preventing such issues in the future may be helpful.
>
> I first cloned the repository:
>
> $ git clone https://github.com/CodeForChicago/superclass.git
>
> Then, I created a new branch (or so I thought):
>
> $ git checkout -b lesson_page
>
> However, this branch has already existed for about 4 weeks, without my
> knowledge.  I proceeded to do some work on the files it contained, and when
> it came time to commit and push, and when I pushed, I got the following
> message:

The branch existed in the remote repository, but it doesn't exist
locally. You never fetched a copy into refs/remotes/origin since you
didn't say you were interested, and git will fully allow you to create
local branches with the same name as remote branches.

>
> To https://github.com/CodeForChicago/superclass.git
>  ! [rejected]lesson_page -> lesson_page (non-fast-forward)
> error: failed to push some refs to '
> https://github.com/CodeForChicago/superclass.git'
> hint: Updates were rejected because the tip of your current branch is behind
> hint: its remote counterpart. Integrate the remote changes (e.g.
> hint: 'git pull ...') before pushing again.
> hint: See the 'Note about fast-forwards' in 'git push --help' for details.
>

When you tried to push this branch, it will push into
refs/heads/lesson_page on the remote, which already exists. Since it
cannot perform a fast-forward update, as your local work isn't based
directly on the tip of the remote branch, you either need to merge,
rebase, or start from scratch.

> Given that I had believed that I had created the branch just a few hours
> prior and was the first to attempt to push to it, this error was
> consternating.

Let me try to explain. You created your own local branch, which
happened to share the same name as an already existing branch. Had you
know this you could have fetched and checked out that branch. You can
view all branches using "git branch -a" or "git ls-remote".

>
> I may be wrong (I am aware that my understanding of git is limited), but I
> believe that the git checkout -b command is simply supposed to create a new
> branch and then switch to it (I'm not aware of any subtle behavior that goes
> on behind the scenes if the "new" branch that the user is attempting to
> create already exists).  This is why I said it "may not be a bug per se".
> However, I expect most people who use git to expect this command to create a
> new branch and then switch to it (this is what most sources online will tell
> users to do to create a new branch), and as such, it would be extremely
> beneficial if git were to, at the very least, alert the user to the conflict
> in some way or another.

git checkout -b  will create a new branch in your local
copy of the repository. Git is distributed. You can do "git checkout
--track " to attempt to create a local branch which tracks the
upstream branch, and then git status will provide useful information
about the relationship between your local work and the remote work.

It could maybe be improved to notice that a remote has a branch with
the same name. However, git can support multiple remotes, so
determining which remote to care about is difficult.

In your case you have a couple of options to fix it. I would suggest at least

"git branch --set-upstream-to=origin/" so that git status
will give you useful information about the branch relationship. Then
you can merge or rebase your work into the branch.

The issue is in understanding how git distributes branches, and how it
could handle this. I suspect improvements could be made so that it
will attempt to warn you when you create a branch that already exists.
However, often you do this *intending* to make it track the specific
branch so I am not sure how much good a warning would do. Just a
message wouldn't really hurt anything, however.

Thanks,
Jake

>
> Thanks,
> Ben
>
--
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: Bug Report

2016-04-02 Thread Eric N. Vander Weele
On Sat, Apr 2, 2016 at 8:25 PM, Benjamin Sandeen
 wrote:
> Today, I managed to create a duplicate branch in a git repository.  While
> this may not be a bug per se, I do think that it is confusing and some way
> of preventing such issues in the future may be helpful.

This can be confusing.  I'll hopefully try to help and explain below.

> I first cloned the repository:
>
> $ git clone https://github.com/CodeForChicago/superclass.git
>
> Then, I created a new branch (or so I thought):
>
> $ git checkout -b lesson_page

At this point, you created a *local* branch called 'lesson_page' which
points to the current HEAD and then switched to that branch .  This
local branch is independent of the remote branch called
'origin/lesson_page'.

> However, this branch has already existed for about 4 weeks, without my
> knowledge.  I proceeded to do some work on the files it contained, and when
> it came time to commit and push, and when I pushed, I got the following
> message:
>
> To https://github.com/CodeForChicago/superclass.git
>  ! [rejected]lesson_page -> lesson_page (non-fast-forward)
> error: failed to push some refs to '
> https://github.com/CodeForChicago/superclass.git'
> hint: Updates were rejected because the tip of your current branch is behind
> hint: its remote counterpart. Integrate the remote changes (e.g.
> hint: 'git pull ...') before pushing again.
> hint: See the 'Note about fast-forwards' in 'git push --help' for details.
>
> Given that I had believed that I had created the branch just a few hours
> prior and was the first to attempt to push to it, this error was
> consternating.

The non-fast-forward push is preventing history being rewritten- this
is a good thing :).

> I may be wrong (I am aware that my understanding of git is limited), but I
> believe that the git checkout -b command is simply supposed to create a new
> branch and then switch to it (I'm not aware of any subtle behavior that goes
> on behind the scenes if the "new" branch that the user is attempting to
> create already exists).  This is why I said it "may not be a bug per se".
> However, I expect most people who use git to expect this command to create a
> new branch and then switch to it (this is what most sources online will tell
> users to do to create a new branch), and as such, it would be extremely
> beneficial if git were to, at the very least, alert the user to the conflict
> in some way or another.

The `git checkout -b` command is working as expected.  `git checkout
-b ` is equivalent to `git branch  && git checkout
`.  If you were to execute `git checkout lesson_page`, you would
get the desired behavior you were expecting because in the presence of
one remote, git will actually execute `git checkout -b lesson_page
--track origin/lesson_page` - more details can be found in `git help
checkout`.  Effectively, it checkouts 'origin/lesson_page' as a new
local branch named 'lesson_page'.

However, you indicated that you did not know there was a remote
branch already named 'lesson_page'.  After cloning the repository, you
can use `git branch -a` to see all remotes to determine which form of
`git checkout` to use.

> Thanks,
> Ben
>
> Lead Consultant, Northwestern University Information Technology
> Research Assistant, Center for Interdisciplinary Exploration and Research in
> Astrophysics at Northwestern University
> Phsyics, Weinberg College of Arts and Sciences
> Computer Science, Weinberg College of Arts and Sciences
> Classics, Weinberg College of Arts and Sciences
>
>
>
>
> --
> 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


[PATCH] builtin/log.c: fixup format-patch --base segfault

2016-04-02 Thread Ramsay Jones

Signed-off-by: Ramsay Jones 
---

Hi Xiaolong,

When you next re-roll your 'xy/format-patch-base' branch could you
please squash this (or something like it) into the relevant patch.
(commit 50ff6afd, "format-patch: add '--base' option to record base
tree info", 31-03-2016).

The pu branch, for me, fails a shed load of tests in the following:

t3301-notes.sh
t3901-i18n-patch.sh
t4014-format-patch.sh
t4021-format-patch-numbered.sh
t4028-format-patch-mime-headers.sh
t4030-diff-textconv.sh
t4036-format-patch-signer-mime.sh
t4052-stat-output.sh
t4122-apply-symlink-inside.sh
t4150-am.sh
t4151-am-abort.sh
t4152-am-subjects.sh
t4255-am-submodule.sh
t7400-submodule-basic.sh
t7512-status-help.sh
t9001-send-email.sh

Looking at the first failure, the cause was a segfault while running
git-format-patch. A quick trip to the debugger showed that the segfault
was in print_bases(). Furthermore, the contents of the bases structure
passed in looked very dodgy (bases->nr_patch_id was 32767 and bases->patch_id[0]
was 0xc). Indeed, it looked like it had not been initialized ...

[NOTE: t6038-merge-text-auto.sh also fails for me, but it has nothing
to do with your patch series. ;-)]

This patch was just a quick fix, you may chose a different approach to
fix the problem (eg don't call print_bases() unconditionally ...).

ATB,
Ramsay Jones

 builtin/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 48c74f5..fed0f99 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1625,8 +1625,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
signature = strbuf_detach(, NULL);
}
 
+   memset(, 0, sizeof(bases));
if (base_commit || config_base_commit) {
-   memset(, 0, sizeof(bases));
reset_revision_walk();
prepare_bases(, base_commit, list, nr);
}
-- 
2.8.0
--
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


Bug Report

2016-04-02 Thread Benjamin Sandeen
Today, I managed to create a duplicate branch in a git repository.  While
this may not be a bug per se, I do think that it is confusing and some way
of preventing such issues in the future may be helpful.

I first cloned the repository:

$ git clone https://github.com/CodeForChicago/superclass.git

Then, I created a new branch (or so I thought):

$ git checkout -b lesson_page

However, this branch has already existed for about 4 weeks, without my
knowledge.  I proceeded to do some work on the files it contained, and when
it came time to commit and push, and when I pushed, I got the following
message:

To https://github.com/CodeForChicago/superclass.git
 ! [rejected]lesson_page -> lesson_page (non-fast-forward)
error: failed to push some refs to '
https://github.com/CodeForChicago/superclass.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

Given that I had believed that I had created the branch just a few hours
prior and was the first to attempt to push to it, this error was
consternating.

I may be wrong (I am aware that my understanding of git is limited), but I
believe that the git checkout -b command is simply supposed to create a new
branch and then switch to it (I'm not aware of any subtle behavior that goes
on behind the scenes if the "new" branch that the user is attempting to
create already exists).  This is why I said it "may not be a bug per se".
However, I expect most people who use git to expect this command to create a
new branch and then switch to it (this is what most sources online will tell
users to do to create a new branch), and as such, it would be extremely
beneficial if git were to, at the very least, alert the user to the conflict
in some way or another.

Thanks,
Ben

Lead Consultant, Northwestern University Information Technology
Research Assistant, Center for Interdisciplinary Exploration and Research in
Astrophysics at Northwestern University
Phsyics, Weinberg College of Arts and Sciences
Computer Science, Weinberg College of Arts and Sciences
Classics, Weinberg College of Arts and Sciences




--
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 v12 5/5] commit: add a commit.verbose config variable

2016-04-02 Thread Pranit Bauva
Add commit.verbose configuration variable as a convenience for those
who always prefer --verbose.

Helped-by: Junio C Hamano 
Helped-by: Eric Sunshine 
Signed-off-by: Pranit Bauva 

---
The previous version of the patch are:
 - [v11] $gmane/288820
 - [v10] $gmane/288820
 - [v9] $gmane/288820
 - [v8] $gmane/288820
 - [v7] $gmane/288820
 - [v6] $gmane/288728
 - [v5] $gmane/288728
 - [v4] $gmane/288652
 - [v3] $gmane/288634
 - [v2] $gmane/288569
 - [v1] $gmane/287540

   Note: One might think some tests are extra but I think that it will
   be better to include them as they "complete the continuity" thus
   generalising the series which will make the patch even more clearer.
---
 Documentation/config.txt |   4 +
 Documentation/git-commit.txt |   3 +-
 builtin/commit.c |  14 +++-
 t/t7507-commit-verbose.sh| 175 +++
 4 files changed, 194 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2cd6bdd..1d0ec2e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1110,6 +1110,10 @@ commit.template::
"`~/`" is expanded to the value of `$HOME` and "`~user/`" to the
specified user's home directory.
 
+commit.verbose::
+   A boolean or int to specify the level of verbose with `git commit`.
+   See linkgit:git-commit[1].
+
 credential.helper::
Specify an external helper to be called when a username or
password credential is needed; the helper may consult external
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 9ec6b3c..d474226 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -290,7 +290,8 @@ configuration variable documented in linkgit:git-config[1].
what changes the commit has.
Note that this diff output doesn't have its
lines prefixed with '#'. This diff will not be a part
-   of the commit message.
+   of the commit message. See the `commit.verbose` configuration
+   variable in linkgit:git-config[1].
 +
 If specified twice, show in addition the unified diff between
 what would be committed and the worktree files, i.e. the unstaged
diff --git a/builtin/commit.c b/builtin/commit.c
index b3bd2d4..96e6190 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -113,7 +113,9 @@ static char *edit_message, *use_message;
 static char *fixup_message, *squash_message;
 static int all, also, interactive, patch_interactive, only, amend, signoff;
 static int edit_flag = -1; /* unspecified */
-static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
+static int config_verbose = -1; /* unspecified */
+static int verbose = -1; /* unspecified */
+static int quiet, no_verify, allow_empty, dry_run, renew_authorship;
 static int no_post_rewrite, allow_empty_message;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
 static char *sign_commit;
@@ -1354,6 +1356,8 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
 builtin_status_usage, 0);
finalize_colopts(, -1);
finalize_deferred_config();
+   if (verbose == -1)
+   verbose = 0;
 
handle_untracked_files_arg();
if (show_ignored_in_status)
@@ -1505,6 +1509,11 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
}
+   if (!strcmp(k, "commit.verbose")) {
+   int is_bool;
+   config_verbose = git_config_bool_or_int(k, v, _bool);
+   return 0;
+   }
 
status = git_gpg_config(k, v, NULL);
if (status)
@@ -1654,6 +1663,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
  builtin_commit_usage,
  prefix, current_head, );
+   if (verbose == -1)
+   verbose = (config_verbose < 0) ? 0 : config_verbose;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, );
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 0f28a86..7c79484 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -98,4 +98,179 @@ test_expect_success 'verbose diff is stripped out with set 
core.commentChar' '
test_i18ngrep "Aborting commit due to empty commit message." err
 '
 
+test_expect_success 'set up -v -v' '
+   echo dirty >file &&
+   echo dirty >file2 &&
+   git add file2
+'
+test_expect_success 'commit.verbose true and --verbose omitted' '
+   git -c commit.verbose=true commit -F message &&
+   test_line_count = 1 out
+'

[PATCH v12 1/5] t0040-test-parse-options.sh: fix style issues

2016-04-02 Thread Pranit Bauva
Signed-off-by: Pranit Bauva 

---
Changes wrt previous version (v11):
 - This patch is a split up of patch 1/4 v11 as requested by Junio.
 - This patch uses the backslash with EOF as suggested by Junio
   for 2 tests namely "detect possible typos"
---
 t/t0040-parse-options.sh | 72 
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 9be6411..c6f205b 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -7,7 +7,7 @@ test_description='our own option parser'
 
 . ./test-lib.sh
 
-cat > expect << EOF
+cat >expect <
 
 --yes get a boolean
@@ -49,7 +49,7 @@ Standard options
 EOF
 
 test_expect_success 'test help' '
-   test_must_fail test-parse-options -h > output 2> output.err &&
+   test_must_fail test-parse-options -h >output 2>output.err &&
test_must_be_empty output.err &&
test_i18ncmp expect output
 '
@@ -156,7 +156,7 @@ test_expect_success 'OPT_MAGNITUDE() 3giga' '
check magnitude: 3221225472 -m 3g
 '
 
-cat > expect << EOF
+cat >expect < expect << EOF
+cat >expect < expect << EOF
+cat >expect < output 2> output.err &&
+   >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
-cat > expect << EOF
+cat >expect < output 2> output.err &&
+   test-parse-options --int 2 --boolean --no-bo >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
 test_expect_success 'unambiguously abbreviated option with "="' '
-   test-parse-options --int=2 > output 2> output.err &&
+   test-parse-options --int=2 >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
@@ -256,7 +256,7 @@ test_expect_success 'ambiguously abbreviated option' '
test_expect_code 129 test-parse-options --strin 123
 '
 
-cat > expect << EOF
+cat >expect < output 2> output.err &&
+   test-parse-options --st 123 >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
-cat > typo.err << EOF
-error: did you mean \`--boolean\` (with two dashes ?)
+cat >typo.err <<\EOF
+error: did you mean `--boolean` (with two dashes ?)
 EOF
 
 test_expect_success 'detect possible typos' '
-   test_must_fail test-parse-options -boolean > output 2> output.err &&
+   test_must_fail test-parse-options -boolean >output 2>output.err &&
test_must_be_empty output &&
test_cmp typo.err output.err
 '
 
-cat > typo.err << EOF
-error: did you mean \`--ambiguous\` (with two dashes ?)
+cat >typo.err <<\EOF
+error: did you mean `--ambiguous` (with two dashes ?)
 EOF
 
 test_expect_success 'detect possible typos' '
-   test_must_fail test-parse-options -ambiguous > output 2> output.err &&
+   test_must_fail test-parse-options -ambiguous >output 2>output.err &&
test_must_be_empty output &&
test_cmp typo.err output.err
 '
 
-cat > expect  output.err &&
+   test-parse-options --quux >output 2>output.err &&
test_must_be_empty output.err &&
-test_cmp expect output
+   test_cmp expect output
 '
 
-cat > expect  output.err &&
+   foo -q >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
-cat > expect  output.err &&
+   test-parse-options --length=four -b -4 >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
-cat > expect  output.err &&
+   test_must_fail test-parse-options --no-length >output 2>output.err &&
test_i18ncmp expect output &&
test_i18ncmp expect.err output.err
 '
 
-cat > expect  output.err &&
+   test-parse-options --set23 -b --no-or4 >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
 test_expect_success 'OPT_NEGBIT() and OPT_SET_INT() work' '
-   test-parse-options --set23 -b --neg-or4 > output 2> output.err &&
+   test-parse-options --set23 -b --neg-or4 >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
-cat > expect  output.err &&
+   test-parse-options -bb --or4 >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
 test_expect_success 'OPT_NEGBIT() works' '
-   test-parse-options -bb --no-neg-or4 > output 2> output.err &&
+   test-parse-options -bb --no-neg-or4 >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
 '
 
 test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '
-   test-parse-options + + + + + + > output 2> output.err &&
+   test-parse-options + + + + + + >output 2>output.err &&

[PATCH v12 2/5] test-parse-options: print quiet as integer

2016-04-02 Thread Pranit Bauva
Current implementation of parse-options.c treats OPT__QUIET() as integer
and not boolean and thus it is more appropriate to print it as integer
to avoid confusion.

Signed-off-by: Pranit Bauva 
---
 t/t0040-parse-options.sh | 26 +-
 test-parse-options.c |  2 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index c6f205b..302c315 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -64,7 +64,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
@@ -164,7 +164,7 @@ timestamp: 0
 string: 123
 abbrev: 7
 verbose: 2
-quiet: no
+quiet: 0
 dry run: yes
 file: prefix/my.file
 EOF
@@ -184,7 +184,7 @@ timestamp: 0
 string: 321
 abbrev: 10
 verbose: 2
-quiet: no
+quiet: 0
 dry run: no
 file: prefix/fi.le
 EOF
@@ -212,7 +212,7 @@ timestamp: 0
 string: 123
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 arg 00: a1
@@ -235,7 +235,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
@@ -264,7 +264,7 @@ timestamp: 0
 string: 123
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
@@ -303,7 +303,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 arg 00: --quux
@@ -323,7 +323,7 @@ timestamp: 1
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: yes
+quiet: 1
 dry run: no
 file: (not set)
 arg 00: foo
@@ -345,7 +345,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
@@ -374,7 +374,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
@@ -399,7 +399,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
@@ -430,7 +430,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
@@ -449,7 +449,7 @@ timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0
-quiet: no
+quiet: 0
 dry run: no
 file: (not set)
 EOF
diff --git a/test-parse-options.c b/test-parse-options.c
index 2c8c8f1..86afa98 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -90,7 +90,7 @@ int main(int argc, char **argv)
printf("string: %s\n", string ? string : "(not set)");
printf("abbrev: %d\n", abbrev);
printf("verbose: %d\n", verbose);
-   printf("quiet: %s\n", quiet ? "yes" : "no");
+   printf("quiet: %d\n", quiet);
printf("dry run: %s\n", dry_run ? "yes" : "no");
printf("file: %s\n", file ? file : "(not set)");
 

--
https://github.com/git/git/pull/218
--
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 v12 3/5] parse-options.c: make OPTION_COUNTUP respect "unspecified" values

2016-04-02 Thread Pranit Bauva
The reason to make it respect "unspecified" values is to give the
ability to differentiate whether `--option` or `--no-option` was
specified at all. "unspecified" values should be in the form of negative
values. If initial value is set to negative and `--option` specified
then it will reflect the number of occurrences (counting done from 0),
if `--no-option` is specified then it will reflect 0 and if nothing at
all is given then it will retain its negative value.

This change will not affect existing users of COUNTUP, because they all
use the initial value of 0 (or more).

Note that builtin/clean.c initializes the variable used with
OPT__FORCE (which uses OPT_COUNTUP()) to a negative value, but it is set
to either 0 or 1 by reading the configuration before the code calls
parse_options(), i.e. as far as parse_options() is concerned, the
initial value of the variable is not negative.

To test this behavior "verbose" is set to "unspecified" while quiet is
set to 0 which will test the new behavior with all sets of values.

Helped-by: Jeff King 
Helped-by: Eric Sunshine 
Helped-by: Junio C Hamano 
Signed-off-by: Pranit Bauva 

---
The discussion about this patch:
[1] : http://thread.gmane.org/gmane.comp.version-control.git/289027

Previous version of the patch:
[v11] : http://thread.gmane.org/gmane.comp.version-control.git/288820
[v10] : http://thread.gmane.org/gmane.comp.version-control.git/288820
[v9] : http://thread.gmane.org/gmane.comp.version-control.git/288820
[v1] : http://thread.gmane.org/gmane.comp.version-control.git/289061

Changes wrt previous version (v11):
 - Use bits of commit message provided by Junio.

Please Note: The diff might seem improper especially the part where I
have introduced some continuous lines but this is a logical error by git
diff (nothing could be done about it) and thus the changes will be
clearly visible with the original file itself.
---
 Documentation/technical/api-parse-options.txt |  8 --
 parse-options.c   |  2 ++
 t/t0040-parse-options.sh  | 39 ---
 test-parse-options.c  |  3 ++-
 4 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 5f0757d..8908bf7 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -144,8 +144,12 @@ There are some macros to easily define options:
 
 `OPT_COUNTUP(short, long, _var, description)`::
Introduce a count-up option.
-   `int_var` is incremented on each use of `--option`, and
-   reset to zero with `--no-option`.
+   Each use of `--option` increments `int_var`, starting from zero
+   (even if initially negative), and `--no-option` resets it to
+   zero. To determine if `--option` or `--no-option` was set at
+   all, set `int_var` to a negative value, and if it is still
+   negative after parse_options(), then neither `--option` nor
+   `--no-option` was seen.
 
 `OPT_BIT(short, long, _var, description, mask)`::
Introduce a boolean option.
diff --git a/parse-options.c b/parse-options.c
index 47a9192..312a85d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -110,6 +110,8 @@ static int get_value(struct parse_opt_ctx_t *p,
return 0;
 
case OPTION_COUNTUP:
+   if (*(int *)opt->value < 0)
+   *(int *)opt->value = 0;
*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
return 0;
 
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 302c315..bfd8dea 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -63,7 +63,7 @@ magnitude: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 0
 dry run: no
 file: (not set)
@@ -211,7 +211,7 @@ magnitude: 0
 timestamp: 0
 string: 123
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 0
 dry run: no
 file: (not set)
@@ -234,7 +234,7 @@ magnitude: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 0
 dry run: no
 file: (not set)
@@ -263,7 +263,7 @@ magnitude: 0
 timestamp: 0
 string: 123
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 0
 dry run: no
 file: (not set)
@@ -302,7 +302,7 @@ magnitude: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 0
 dry run: no
 file: (not set)
@@ -322,7 +322,7 @@ magnitude: 0
 timestamp: 1
 string: (not set)
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 1
 dry run: no
 file: (not set)
@@ -344,7 +344,7 @@ magnitude: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 0
 dry run: no
 file: (not set)
@@ -373,7 +373,7 @@ magnitude: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
-verbose: 0
+verbose: -1
 quiet: 0
 dry run: no
 file: (not set)
@@ -398,7 +398,7 @@ 

[PATCH v12 4/5] t7507-commit-verbose: improve test coverage by testing number of diffs

2016-04-02 Thread Pranit Bauva
Make the fake "editor" store output of grep in a file so that we can
see how many diffs were contained in the message and use them in
individual tests where ever it is required. Also use write_script()
to create the fake "editor".

A subsequent commit will introduce scenarios where it is important to be
able to exactly determine how many diffs were present.

Helped-by: Eric Sunshine 
Signed-off-by: Pranit Bauva 

---
Previous version of this patch:
 - [v11] : $gmane/28820
 - [v10]: $gmane/288820

Changes this version wrt previous one:
Include the triple dash which I previously forgot, pointed out by Junio.
---
 t/t7507-commit-verbose.sh | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 2ddf28c..0f28a86 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -3,11 +3,10 @@
 test_description='verbose commit template'
 . ./test-lib.sh
 
-cat >check-for-diff message <<'EOF'
@@ -23,7 +22,8 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'initial commit shows verbose diff' '
-   git commit --amend -v
+   git commit --amend -v &&
+   test_line_count = 1 out
 '
 
 test_expect_success 'second commit' '
@@ -39,13 +39,15 @@ check_message() {
 
 test_expect_success 'verbose diff is stripped out' '
git commit --amend -v &&
-   check_message message
+   check_message message &&
+   test_line_count = 1 out
 '
 
 test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
git config diff.mnemonicprefix true &&
git commit --amend -v &&
-   check_message message
+   check_message message &&
+   test_line_count = 1 out
 '
 
 cat >diff <<'EOF'

--
https://github.com/git/git/pull/218
--
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/4] t/t7030-verify-tag.sh: Adds validation for multiple tags

2016-04-02 Thread santiago
From: Santiago Torres 

The verify-tag command supports mutliple tag names as an argument.
However, no previous tests try to verify multiple tags at once. This
test runs the verify-tag command against three trusted tags (created
previously), and ensures that:

1) Three tags are verified appropriately (grep GOODSIG) and
2) The three tags verified are indeed differently (uniq
-u)

Signed-off-by: Santiago Torres 
---
 t/t7030-verify-tag.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..5918f86 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,12 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   git verify-tag -v --raw fourth-signed sixth-signed seventh-signed 
2>actual 1> tagnames &&
+   grep -c "GOODSIG" actual > count &&
+   ! grep "BADSIG" actual &&
+   grep -E "tag\ .*" tagnames | uniq -u | wc - l | grep "3"
+'
+
+
 test_done
-- 
2.8.0

--
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 4/4] tag: use pgp_verify_function in tag -v call

2016-04-02 Thread santiago
From: Santiago Torres 

Instead of running the verify-tag plumbing command, we use the
pgp_verify_tag(). This avoids the usage of an extra fork call. To do
this, we extend the number of parameters that tag.c takes, and
verify-tag passes. Redundant calls done in the pgp_verify_tag function
are removed.

Signed-off-by: Santiago Torres 
---
 Notes:
 - In this version I fixed the issues with the brackets (the old patch
   doesn't work in this case due to the new test.

 builtin/tag.c| 28 +---
 builtin/verify-tag.c | 14 --
 tag.c|  7 ++-
 tag.h|  3 ++-
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..3dffdff 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -65,9 +65,10 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const unsigned char *sha1, unsigned flags);
 
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+   unsigned flags)
 {
const char **p;
char ref[PATH_MAX];
@@ -86,33 +87,21 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn)
had_error = 1;
continue;
}
-   if (fn(*p, ref, sha1))
+   if (fn(*p, ref, sha1, flags))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, unsigned flags)
 {
-   if (delete_ref(ref, sha1, 0))
+   if (delete_ref(ref, sha1, flags))
return 1;
printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, 
DEFAULT_ABBREV));
return 0;
 }
 
-static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
-{
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
-}
-
 static int do_sign(struct strbuf *buffer)
 {
return sign_buffer(buffer, buffer, get_signing_key());
@@ -424,9 +413,10 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag);
+   return for_each_tag_name(argv, delete_tag, 0);
if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, pgp_verify_tag,
+   GPG_VERIFY_VERBOSE);
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index f776778..8abc357 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -30,6 +30,8 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   unsigned char sha1[20];
+   const char *name;
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
@@ -46,8 +48,16 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (pgp_verify_tag(argv[i++], flags))
+   while (i < argc) {
+   name = argv[i++];
+   if (get_sha1(name, sha1)) {
+   error("tag '%s' not found.", name);
had_error = 1;
+   }
+
+   if (pgp_verify_tag(name, NULL, sha1, flags))
+   had_error = 1;
+
+   }
return had_error;
 }
diff --git a/tag.c b/tag.c
index 918ae39..2a0b24c 100644
--- a/tag.c
+++ b/tag.c
@@ -29,18 +29,15 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int pgp_verify_tag(const char *name, unsigned flags)
+int pgp_verify_tag(const char *name, const char *ref,
+   const unsigned char *sha1, unsigned flags)
 {
 
enum object_type type;
unsigned long size;
-   unsigned char sha1[20];
char* buf;

[PATCH v3 3/4] builtin/verify-tag: move verification code to tag.c

2016-04-02 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other
commands that require it. We do this by moving it to the common tag.c
code. We rename the verify_tag() function to pgp_verify_tag() to avoid
conflicts with the mktag.c function.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 51 +--
 tag.c| 50 ++
 tag.h|  1 +
 3 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..f776778 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,55 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   int len;
-   int ret;
-
-   memset(, 0, sizeof(sigc));
-
-   len = parse_signature(buf, size);
-
-   if (size == len) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, len, buf + len, size - len, );
-   print_signature_buffer(, flags);
-
-   signature_check_clear();
-   return ret;
-}
-
-static int verify_tag(const char *name, unsigned flags)
-{
-   enum object_type type;
-   unsigned char sha1[20];
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
-
-   buf = read_sha1_file(sha1, , );
-   if (!buf)
-   return error("%s: unable to read file.", name);
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -96,7 +47,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
flags |= GPG_VERIFY_VERBOSE;
 
while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   if (pgp_verify_tag(argv[i++], flags))
had_error = 1;
return had_error;
 }
diff --git a/tag.c b/tag.c
index d72f742..918ae39 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,56 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int payload_size;
+   int ret;
+
+   memset(, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   write_in_full(1, buf, payload_size);
+   return error("No PGP signature found in this tag!");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, );
+   print_signature_buffer(, flags);
+
+   signature_check_clear();
+   return ret;
+}
+
+int pgp_verify_tag(const char *name, unsigned flags)
+{
+
+   enum object_type type;
+   unsigned long size;
+   unsigned char sha1[20];
+   char* buf;
+   int ret;
+
+   if (get_sha1(name, sha1))
+   return error("tag '%s' not found.", name);
+
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   name, typename(type));
+
+   buf = read_sha1_file(sha1, , );
+   if (!buf)
+   return error("%s: unable to read file.", name);
+
+   ret = run_gpg_verify(buf, size, flags);
+
+   free(buf);
+   return ret;
+}
+
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
while (o && o->type == OBJ_TAG)
diff --git a/tag.h b/tag.h
index f4580ae..09e71f9 100644
--- a/tag.h
+++ b/tag.h
@@ -17,5 +17,6 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
+extern int pgp_verify_tag(const char *name, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.8.0

--
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/4] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

2016-04-02 Thread santiago
From: Santiago Torres 

The verify_signed_buffer comand might cause a SIGPIPE signal when the
gpg child process terminates early (due to a bad keyid, for example) and
git tries to write to it afterwards. Previously, ignoring SIGPIPE was
done on the builtin/gpg-verify.c command to avoid this issue. However,
any other caller who wanted to use the verify_signed_buffer command
would have to include this signal call.

Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the
verify_signed_buffer call (pretty much like in sign_buffer()) so
that any caller is not required to perform this task. This will avoid
possible mistakes by further developers using verify_signed_buffer.

Signed-off-by: Santiago Torres 
---
Notes:
 I dropped the multiline comment altogheter.

 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..c1f6b2d 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -232,6 +232,8 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
if (gpg_output)
gpg.err = -1;
args_gpg[3] = path;
+
+   sigchain_push(SIGPIPE, SIG_IGN);
if (start_command()) {
unlink(path);
return error(_("could not run gpg."));
@@ -250,6 +252,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command();
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
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/4] tag: move PGP verification code to tag.c

2016-04-02 Thread santiago
This is a follow up of [1] and [2]:

v3 (this):
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0.

Thanks!
-Santiago

[1]
http://git.661346.n2.nabble.com/PATCH-RFC-builtin-tag-c-move-PGP-verification-inside-builtin-td7651529.html#a7651547
[2] 
http://git.661346.n2.nabble.com/PATCH-tag-c-move-PGP-verification-code-from-plumbing-td7651562.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: [PATCH 0/2] mergetools: add support for ExamDiff

2016-04-02 Thread David Aguilar
On Fri, Apr 01, 2016 at 11:10:56AM -0700, Junio C Hamano wrote:
> Jacob Nisnevich  writes:
> 
> > OK I add the quotes and modified the comment. I also changed $folder to 
> > $sub_directory. I think that makes a little bit more sense and sounds a lot
> > better.
> >
> > Jacob Nisnevich (2):
> >   mergetools: create mergetool_find_win32_cmd() helper function for
> > winmerge
> >   mergetools: add support for ExamDiff
> >
> >  git-mergetool--lib.sh | 25 +
> >  mergetools/examdiff   | 18 ++
> >  mergetools/winmerge   | 21 +
> >  3 files changed, 44 insertions(+), 20 deletions(-)
> >  create mode 100644 mergetools/examdiff
> 
> This round looked good to me.
> David, does this look sensible to you?
> 
> Thanks.

Yes, thank you.

Acked-by: David Aguilar 

cheers,
-- 
David
--
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 1/4] config --show-origin: report paths with forward slashes

2016-04-02 Thread Johannes Sixt
Am 29.03.2016 um 22:05 schrieb Junio C Hamano:
> Johannes Sixt  writes:
> 
>> This part of your 45bf3297 (t1300: fix the new --show-origin tests on
>> Windows)
>>
>> @@ -1205,6 +1205,9 @@ test_expect_success POSIXPERM,PERL 'preserves existing 
>> per
>>"die q(badrename) if ((stat(q(.git/config)))[2] & 0) != 0600"
>>   '
>>   
>> +! test_have_prereq MINGW ||
>> +HOME="$(pwd)" # convert to Windows path
>> +
>>   test_expect_success 'set up --show-origin tests' '
>>  INCLUDE_DIR="$HOME/include" &&
>>  mkdir -p "$INCLUDE_DIR" &&
>>
>> is actually a much more concise version of my proposed patch,
>> although the result still misuses $HOME where it does not have
>> to. In fact, if I revert 5ca6b7bb (config --show-origin: report
>> paths with forward slashes), the tests still pass. But since it
>> does not make a difference save for a few microseconds more or
>> less during startup, it is not worth the churn at this point.
> 
> Well, from the point of view of codebase cleanliness, if we can do
> without 5ca6b7bb4, we would be much better off in the longer term,
> so I would say it would be wonderful if we can safely revert it.

Although I am convinced that the change is not necessary for
correctness, I can buy the justification that we should produce forward
slashes for consistency. There are a number of occasions where we
present paths to the user, and we do show forward-slashes in all cases
that I found. We should keep the commit.

But then let's do this:

 8< 
Subject: [PATCH] Windows: shorten code by re-using convert_slashes()

Make a few more spots more readable by using the recently introduced,
Windows-specific helper.

Signed-off-by: Johannes Sixt 
---
 abspath.c  | 5 +
 compat/mingw.c | 9 ++---
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/abspath.c b/abspath.c
index 5edb4e7..2825de8 100644
--- a/abspath.c
+++ b/abspath.c
@@ -167,7 +167,6 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
const char *arg)
strbuf_add(, pfx, pfx_len);
strbuf_addstr(, arg);
 #else
-   char *p;
/* don't add prefix to absolute paths, but still replace '\' by '/' */
strbuf_reset();
if (is_absolute_path(arg))
@@ -175,9 +174,7 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
const char *arg)
else if (pfx_len)
strbuf_add(, pfx, pfx_len);
strbuf_addstr(, arg);
-   for (p = path.buf + pfx_len; *p; p++)
-   if (*p == '\\')
-   *p = '/';
+   convert_slashes(path.buf + pfx_len);
 #endif
return path.buf;
 }
diff --git a/compat/mingw.c b/compat/mingw.c
index 54c82ec..0413d5c 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -763,15 +763,12 @@ struct tm *localtime_r(const time_t *timep, struct tm 
*result)
 
 char *mingw_getcwd(char *pointer, int len)
 {
-   int i;
wchar_t wpointer[MAX_PATH];
if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
return NULL;
if (xwcstoutf(pointer, wpointer, len) < 0)
return NULL;
-   for (i = 0; pointer[i]; i++)
-   if (pointer[i] == '\\')
-   pointer[i] = '/';
+   convert_slashes(pointer);
return pointer;
 }
 
@@ -2112,9 +2109,7 @@ static void setup_windows_environment()
 * executable (by not mistaking the dir separators
 * for escape characters).
 */
-   for (; *tmp; tmp++)
-   if (*tmp == '\\')
-   *tmp = '/';
+   convert_slashes(tmp);
}
 
/* simulate TERM to enable auto-color (see color.c) */
-- 
2.7.0.118.g90056ae

--
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 1/4] config --show-origin: report paths with forward slashes

2016-04-02 Thread Johannes Sixt

Am 30.03.2016 um 07:52 schrieb Johannes Sixt:

Am 29.03.2016 um 21:18 schrieb Johannes Sixt:

It does pass. The reason is that pwd -W generates forward slashes.


It just occurred to me that we might be observing a difference in
behavior of pwd -W between the modern MSYS2 bash and the old MSYS1 bash
that I am using.


No that isn't it. Both versions produce forward slashes. Puzzled...

-- Hannes

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

2016-04-02 Thread Johannes Sixt

Am 02.04.2016 um 19:58 schrieb Mehul Jain:

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

Helped-by: Eric Sunshine 
Signed-off-by: Mehul Jain 
---
  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


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.


You could also change the variable name, but to be sufficiently safe, 
you would have to use an unsightly long name. 'opt' would be just as bad 
as 'i'.




  test_expect_success 'pull.rebase' '
git reset --hard before-rebase &&



-- Hannes

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

2016-04-02 Thread 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 
Signed-off-by: Mehul Jain 
---
 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

2016-04-02 Thread 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 
---
 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

2016-04-02 Thread Mehul Jain
These two tests are almost similar and thus can be folded in a for-loop.

Helped-by: Eric Sunshine 
Signed-off-by: Mehul Jain 
---
 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

2016-04-02 Thread 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 
Signed-off-by: Mehul Jain 
---
 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

2016-04-02 Thread 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 
---
 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

2016-04-02 Thread Mehul Jain
Signed-off-by: Mehul Jain 
---
 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

2016-04-02 Thread 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 
---
 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

2016-04-02 Thread Mehul Jain
The following series is applicable on mj/pull-rebase-autostash.

Thanks Eric and Junio for there comments on previous version[1]

Changes made vs v1:
* [Patch v1 4/5] is broken into three patches to increase
  readability of the patches.

* [Patch 4/5] Factor out code in two functions 
  test_pull_autostash() and test_pull_autostash_fail()
  instead of test_rebase_autostash() and 
  test_rebase_no_autostash(). This leads to further 
  simplification of code.
  
  Also removed two for-loops as they didn't provided
  the simplicity intended for.
  
  For-loop was over-intended. Corrected it.

* Commit message for patches 1/5, 2/5, 3/5 are improved
  as suggested by Eric in the previous round.

Here's interdiff with v1:

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 4da9e52..bed75f5 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -9,22 +9,22 @@ modify () {
mv "$2.x" "$2"
 }
 
-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 &&
test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
 }
 
-test_rebase_no_autostash () {
+test_pull_autostash_fail () {
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_must_fail git pull $@ . copy 2>err &&
+   test_i18ngrep "uncommitted changes." err
 }
 
 test_expect_success setup '
@@ -265,48 +265,46 @@ 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 &&
+   test_pull_autostash --rebase --autostash
 '
 
-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=false' '
+   test_config rebase.autostash false &&
+   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 &&
-   test_rebase_autostash
+   test_pull_autostash --rebase --autostash
+'
+
+test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
+   test_config rebase.autostash true &&
+   test_pull_autostash_fail --rebase --no-autostash
 '
 
-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=false' '
+   test_config rebase.autostash false &&
+   test_pull_autostash_fail --rebase --no-autostash
+'
 
 test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
-   test_rebase_no_autostash
+   test_pull_autostash_fail --rebase --no-autostash
 '
 
 for i in --autostash --no-autostash
-   do
-   test_expect_success "pull $i (without --rebase) is illegal" '
-   test_must_fail git pull $i . copy 2>actual &&
-   test_i18ngrep "only valid with --rebase" actual
-   '
-   done
+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 &&
@@ -318,22 +316,12 @@ test_expect_success 'pull.rebase' '
 
 test_expect_success 'pull --autostash & pull.rebase=true' '
test_config pull.rebase true &&
-   git reset 

Re: Merge driver not called for locally modified files?

2016-04-02 Thread Junio C Hamano
Gioele Barabucci  writes:

> it seems to me that merge drivers are not called while merging commits
> that touch locally modified (but uncommited) files. Is this correct?

Yes.  "git merge" first notices this situation and stops before it
has to decide which merge driver to use.

When you try to merge commit 'B' when you are at commit 'A', and
have some local changes, and these two branches were forked from a
common ancestor 'X', the history may look like this:

1
   /
X--o--A
 \
  --o--B

where '1' is a hypothetical commit that would result if you were to
make a commit with all your local changes, i.e. diff(1,A) is your
uncommitted changes.  As usual, time flows from left to right.

When you merge branch 'B' into your history, you would want to end
up with this history (tentatively ignoring what is in the working
tree):

X--o--A--M
 \  /
  --o--B

where 'M' is the merge between 'A' and 'B', and the change diff(M,A)
must represent what happened between 'X' and 'B' that did not happen
between 'X' and 'A'.  When A and B are independent and without
conflict, that is roughly the same as diff(B,X), in other words, M
is roughly the same as patch(A,diff(B,X)).

As you haven't committed your local changes, diff(1,A) must not
participate in computing the result M of this merge.  After this
merge is done, the blob in M is checked out to the working tree, but
doing so by overwriting the working tree files would lose your local
changes, and that is the reason why you see this error message:

> error: Your local changes to the following files would
> be overwritten by merge:
>   .local/share/pw/passwords
> Please, commit your changes or stash them before you can merge.

What you would want at the very end with is like this:

1  2
   /  /
X--o--A--M
 \  /
  --o--B

where '2' is a hypothetical commit that would result if you were to
cherry pick '1' on top of 'M', after making 'M' according to
thediscussion above (i.e. ignoring the local changes you made since
'A').  But just like you did not have '1' because you were not ready
to record your changes based on 'A' as a commit, you are not ready
to actually make this commit '2', so you would want your head to be
at 'M' and the state of '2' in your working tree, leaving diff(2,M)
as the local uncommitted change.

However.

"git merge" does not do the "create the hypothetical commit '1'" to
store away the local changes, and it does not do the "cherry pick
'1' to create the hypothetical commit '2'" to forward-port the local
changes on top of the merge result 'M'.

This is primarily because there are two distinct steps in the above
hypothetifcal "enhanced" merge.  Creating 'M' may conflict and you
would have to resolve it, while "git merge" somehow need to remember
it has to further do the "cherry pick of '1'" on the result (but
there is no facility to do so in the system).  And after you resolve
the conflict to help it create the merge result 'M', it has to
somehow remember that it needs to "cherry-pick --no-commit '1'", and
have the user resolve the conflict.  As the presence of '1' is not
made explicit to the user (we do not even create '1'), when the
latter step of patch(M,diff(1,A)) fails in conflicts, it is hard for
the user to attempt to resolve them starting from scratch, which
likely leads to "I lost the local change" when in fact it is more
like "I had some local change, but because the merge result was
vastly different from what I had when I started the local change, I
was unable to forward-port them and instead I had to redo it from
scratch".  It is not a good user experience.

> Is it possible to configure git so that the merge driver is called also
> while merging locally modified files?

No.  But you _can_ do that

1  2
   /  /
X--o--A--M
 \  /
  --o--B

thing manually, by following the advice you received from the error
message, by creating '1' yourself.

$ git merge 78d4f09 ;# should fail

$ git checkout -b store-local-changes-away
$ git commit -a -m 'local changes'
$ git checkout @{-1} ;# come back to the original branch
  # at this point, "git status" would report
  # there is no local changes, hence ...

$ git merge 78d4f09   ;# ...this should succeed

$ git cherry-pick --no-commit store-local-changes-away

The last step may conflict (this is what I called 'the latter step'
in the explanation) but at least you have the exact state of '1'
recorded and you know what branch (i.e. store-local-changes-away)
contains the changes, so you can resolve the conflicts in your
working tree without fearing "git reset --hard" to clear the slate
in order to start and retry the conflict resolution from scratch
losing your precious local modification.

Re: git 2.8.0 tarball rpmbuild error

2016-04-02 Thread Todd Zullinger

Pranit Bauva wrote:

On Sat, Apr 2, 2016 at 9:18 PM, マッチョコ太郎  wrote:
hi 
I downloaded tarball (tar.gz) from git web site and tried to make rpm file. 
But, when I run command "$rpmbuild -tb --clean git-2.8.0.tar.gz", 
error message is displayed and rpm file creation stopped. 
The error message is looks like this:
cp -pr README ~/rpmbuild/BUILDROOT/git-2.8.0-1.el7.centos.x86_64/usr/share/doc/git-2.8.0 
cp: cannot stat 'README': No such file or directory


It looks like README file does not exist on expected directory.

Im using Centos 7.2. 
I hope this bug will fix soon. 
thank you


Actually this bug was identified a few days ago and a patch has also 
been sent by Matthieu Moy. Thanks for reporting!


D'oh, I didn't notice this had been reported already and a patch 
provided.


--
Todd
~~
Sometimes I wonder whether the world is being run by smart people who
are putting us on or by imbeciles who really mean it.
   -- Mark Twain

--
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] spec: Adjust README name

2016-04-02 Thread Todd Zullinger

The README file moved to README.md in ad21f5 (README: use markdown
syntax, 2016-02-25).

Reported-by: マッチョコ太郎 
Signed-off-by: Todd Zullinger 

---

Hi,

マッチョコ太郎 wrote:
hi 
I downloaded tarball (tar.gz) from git web site and tried to make rpm file. 
But, when I run command "$rpmbuild -tb --clean git-2.8.0.tar.gz", 
error message is displayed and rpm file creation stopped. 
The error message is looks like this:
cp -pr README ~/rpmbuild/BUILDROOT/git-2.8.0-1.el7.centos.x86_64/usr/share/doc/git-2.8.0 
cp: cannot stat 'README': No such file or directory


It looks like README file does not exist on expected directory.

Im using Centos 7.2. 
I hope this bug will fix soon. 
thank you


I haven't tested this, but I think it should resolve the README issue 
in the spec file.  Confirmation from folks who regularly build from 
this git spec file would be most welcome.


You could also rebuild the Fedora git packages for CentOS¹.  We try to 
maintain compatibility as long as possible.  The Fedora git package 
currently builds as far back as CentOS 5.  I am pretty sure that it 
requires the EPEL repo to be enabled, but that might also be true of 
the git.spec included in here.


¹ 
https://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/Everything/source/tree/Packages/g/git-2.8.0-1.fc25.src.rpm

git.spec.in | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/git.spec.in b/git.spec.in
index d61d537..260022e 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -146,7 +146,7 @@ rm -rf $RPM_BUILD_ROOT
%files -f bin-man-doc-files
%defattr(-,root,root)
%{_datadir}/git-core/
-%doc README COPYING Documentation/*.txt
+%doc README.md COPYING Documentation/*.txt
%{!?_without_docs: %doc Documentation/*.html Documentation/howto}
%{!?_without_docs: %doc Documentation/technical}
%{_sysconfdir}/bash_completion.d
@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT
# No files for you!

%changelog
+* Sat Apr 02 2016 Todd Zullinger 
+- Adjust README name
+
* Sun Sep 18 2011 Jakub Narebski 
- Add gitweb manpages to 'gitweb' subpackage

--
2.4.11

--
Todd
~~
Ambition is a poor excuse for not having enough sense to be lazy.

--
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 2.8.0 tarball rpmbuild error

2016-04-02 Thread Pranit Bauva
On Sat, Apr 2, 2016 at 9:18 PM, マッチョコ太郎  wrote:
> hi
> I downloaded tarball (tar.gz) from git web site and tried to make rpm file.
> But, when I run command "$rpmbuild -tb --clean git-2.8.0.tar.gz",
> error message is displayed and rpm file creation stopped.
> The error message is looks like this:
> cp -pr README 
> ~/rpmbuild/BUILDROOT/git-2.8.0-1.el7.centos.x86_64/usr/share/doc/git-2.8.0
> cp: cannot stat 'README': No such file or directory
>
> It looks like README file does not exist on expected directory.
>
> Im using Centos 7.2.
> I hope this bug will fix soon.
> thank you

Actually this bug was identified a few days ago and a patch has also
been sent by Matthieu Moy. Thanks for reporting!
--
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 2.8.0 tarball rpmbuild error

2016-04-02 Thread マッチョコ太郎
hi
I downloaded tarball (tar.gz) from git web site and tried to make rpm file.
But, when I run command "$rpmbuild -tb --clean git-2.8.0.tar.gz",
error message is displayed and rpm file creation stopped.
The error message is looks like this:
cp -pr README 
~/rpmbuild/BUILDROOT/git-2.8.0-1.el7.centos.x86_64/usr/share/doc/git-2.8.0
cp: cannot stat 'README': No such file or directory

It looks like README file does not exist on expected directory.

Im using Centos 7.2.
I hope this bug will fix soon.
thank you
--
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


Merge driver not called for locally modified files?

2016-04-02 Thread Gioele Barabucci
Hello,

it seems to me that merge drivers are not called while merging commits
that touch locally modified (but uncommited) files. Is this correct?

I made a (simple) merge driver for files in the `pw` format. [1] This
driver works correctly when a file is modified by multiple commits.
However, if a file has only been modified locally (and not committed),
then merging a commit that modifies the same file raises the following
error:

$ git merge --ff-only 78d4f09
Updating 5180202..78d4f09
error: Your local changes to the following files would
be overwritten by merge:
.local/share/pw/passwords
Please, commit your changes or stash them before you can merge.
Aborting

>From my experiments, it looks like the merge driver is not called at all.

Is it possible to configure git so that the merge driver is called also
while merging locally modified files?

Regards,

[1] https://github.com/gioele/pw/blob/master/bin/git-pw-merge

-- 
Gioele Barabucci 
--
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