[sort-of-BUG] merge-resolve cannot resolve "content/mode" conflict
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
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
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
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
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
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)
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 MoyThe 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
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
Hi, On Sat, Apr 2, 2016 at 5:25 PM, Benjamin Sandeenwrote: > 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
On Sat, Apr 2, 2016 at 8:25 PM, Benjamin Sandeenwrote: > 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
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
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
Add commit.verbose configuration variable as a convenience for those who always prefer --verbose. Helped-by: Junio C HamanoHelped-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
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
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
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 KingHelped-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
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 SunshineSigned-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
From: Santiago TorresThe 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
From: Santiago TorresInstead 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
From: Santiago TorresThe 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
From: Santiago TorresThe 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
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
On Fri, Apr 01, 2016 at 11:10:56AM -0700, Junio C Hamano wrote: > Jacob Nisnevichwrites: > > > 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
Am 29.03.2016 um 22:05 schrieb Junio C Hamano: > Johannes Sixtwrites: > >> 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
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
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 SunshineSigned-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
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 SunshineSigned-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
"--[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
These two tests are almost similar and thus can be folded in a for-loop. Helped-by: Eric SunshineSigned-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
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 SunshineSigned-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
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
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
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
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?
Gioele Barabucciwrites: > 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
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
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
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
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?
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