* Re: [PATCH 0/3] delete_ref(): support reflog messages
From: Jeff King @ 2017-02-17 8:17 UTC (permalink / raw)
To: Kyle Meyer; +Cc: Junio C Hamano, git
In-Reply-To: <20170217035800.13214-1-kyle@kyleam.com>
On Thu, Feb 16, 2017 at 10:57:57PM -0500, Kyle Meyer wrote:
> [Sorry for the slow response.]
No problem. The pace of open source varies wildly. :)
> > - "git branch -m" does seem to realize when we are renaming HEAD,
> > because it updates HEAD to point to the new branch name. But it
> > should probably insert another reflog entry mentioning the rename
> > (we do for "git checkout foo", even when "foo" has the same sha1 as
> > the current HEAD).
>
> I haven't worked out how to do this part yet. I'm guessing the change
> will involve modifying split_head_update().
>
> If this is added, should it be instead of, rather than in addition to,
> the deletion entry? If a "Branch: renamed ..." entry is present, it
> doesn't seem like the deletion entry is providing any additional
> information.
I think you could do an "instead of" that goes from sha1 X to X, and
just mentions the rename. Or you could add a second entry after the
delete that takes it from 0{40} back to X.
I suspect the latter is easier to do, and I doubt anybody would care
that much of the exact form. These entries aren't really doing anything
for reachability. They're just giving an audit log of what happened. So
I don't think anybody would really care unless they were debugging a
confusing situation by hand. And as long as there's enough information
to figure out what happened, they'll be happy.
-Peff
^ permalink raw reply
* Re: [PATCH 1/3] delete_refs(): accept a reflog message argument
From: Jeff King @ 2017-02-17 8:12 UTC (permalink / raw)
To: Kyle Meyer; +Cc: Junio C Hamano, git
In-Reply-To: <20170217035800.13214-2-kyle@kyleam.com>
On Thu, Feb 16, 2017 at 10:57:58PM -0500, Kyle Meyer wrote:
> When the current branch is renamed with 'git branch -m/-M' or deleted
> with 'git update-ref -m<msg> -d', the event is recorded in HEAD's log
> with an empty message.
>
> In preparation for adding a more meaningful message to HEAD's log in
> these cases, update delete_ref() to take a message argument and pass
> it along to ref_transaction_delete(). Modify all callers to pass NULL
> for the new message argument; no change in behavior is intended.
Seems like a good first step.
> diff --git a/refs.h b/refs.h
> index 9fbff90e7..81627a63d 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -277,7 +277,7 @@ int reflog_exists(const char *refname);
> * be NULL_SHA1. flags is passed through to ref_transaction_delete().
> */
> int delete_ref(const char *refname, const unsigned char *old_sha1,
> - unsigned int flags);
> + unsigned int flags, const char *msg);
Should the "msg" argument go at the beginning, to match update_ref()?
-Peff
^ permalink raw reply
* Re: [PATCH] tempfile: avoid "ferror | fclose" trick
From: Jeff King @ 2017-02-17 8:07 UTC (permalink / raw)
To: Michael Haggerty
Cc: Andreas Schwab, Junio C Hamano, Jáchym Barvínek, git
In-Reply-To: <923e328c-7fea-a9e4-1059-3bd6b8e58164@alum.mit.edu>
On Fri, Feb 17, 2017 at 09:00:09AM +0100, Michael Haggerty wrote:
> As you pointed out, if ferror() fails, it doesn't set errno properly. At
> least one caller tries to strerror(errno), so it would probably be good
> to store *something* in there, probably EIO.
Yeah, we discussed this up-thread a bit, and my "solution" was similar
to yours. I don't like it, because EIO is a real thing that can happen,
too, and it would certainly be surprising to a user to see. But it's
probably better than the alternative, which is getting whatever random
value happened to be in errno.
The only downside is that if the value of errno _was_ valid (because the
last thing you did really was writing to the filehandle, then we'd
overwrite it).
> To be really pedantic, there's also the question of what errno the
> caller would want if *both* ferror() and fclose() fail. Normally I would
> say "the first error that occurred", but in this case we don't know the
> correct errno from the error reported by ferror(), so maybe the fclose()
> errno is more likely to hint at the underlying reason for the failure.
Yes, I think we're better to take what fclose gives us, if we can.
> So I (reluctantly) propose
>
> if (ferror(fp)) {
> if (!fclose(fp)) {
> /*
> * ferror() doesn't set errno, so we have to
> * set one. (By contrast, when fclose() fails
> * too, we leave *its* errno in place.)
> */
> errno = EIO;
> }
> return -1;
> }
> return fclose();
That's similar to what I wrote earlier, but if we don't mind overwriting
errno unconditionally, I think just:
errno = EIO; /* covers ferror(), overwritten by failing fclose() */
err |= ferror(fp);
err |= fclose(fp);
does the same thing.
-Peff
^ permalink raw reply
* Re: [PATCH] tempfile: avoid "ferror | fclose" trick
From: Michael Haggerty @ 2017-02-17 8:00 UTC (permalink / raw)
To: Jeff King, Andreas Schwab; +Cc: Junio C Hamano, Jáchym Barvínek, git
In-Reply-To: <20170216213140.xqw7gzjimhvg7tcm@sigill.intra.peff.net>
On 02/16/2017 10:31 PM, Jeff King wrote:
> On Thu, Feb 16, 2017 at 11:43:59AM -0500, Jeff King wrote:
>
>> On Thu, Feb 16, 2017 at 11:10:18AM +0100, Andreas Schwab wrote:
>>
>>>>> int xfclose(FILE *fp)
>>>>> {
>>>>> return ferror(fp) | fclose(fp);
>>>>> }
>>>>
>>>> Yes, that's exactly what I had in mind (might be worth calling out the
>>>> bitwise-OR, though, just to make it clear it's not a typo).
>>>
>>> Since the order of evaluation is unspecified, it would be better to
>>> force sequencing ferror before fclose.
>>
>> Good point. Arguably the call in tempfile.c is buggy.
>
> Here's a fix.
>
> I think close_tempfile() suffers from the same errno problem discussed
> earlier in this thread (i.e., that after calling it, you may get an
> error return with a random, unrelated errno value if ferror() failed but
> fclose() did not).
>
> -- >8 --
> Subject: [PATCH] tempfile: avoid "ferror | fclose" trick
>
> The current code wants to record an error condition from
> either ferror() or fclose(), but makes sure that we always
> call both functions. So it can't use logical-OR "||", which
> would short-circuit when ferror() is true. Instead, it uses
> bitwise-OR "|" to evaluate both functions and set one or
> more bits in the "err" flag if they reported a failure.
>
> Unlike logical-OR, though, bitwise-OR does not introduce a
> sequence point, and the order of evaluation for its operands
> is unspecified. So a compiler would be free to generate code
> which calls fclose() first, and then ferror() on the
> now-freed filehandle.
>
> There's no indication that this has happened in practice,
> but let's write it out in a way that follows the standard.
>
> Noticed-by: Andreas Schwab <schwab@linux-m68k.org>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> tempfile.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/tempfile.c b/tempfile.c
> index 2990c9242..ffcc27237 100644
> --- a/tempfile.c
> +++ b/tempfile.c
> @@ -247,12 +247,8 @@ int close_tempfile(struct tempfile *tempfile)
> tempfile->fd = -1;
> if (fp) {
> tempfile->fp = NULL;
> -
> - /*
> - * Note: no short-circuiting here; we want to fclose()
> - * in any case!
> - */
> - err = ferror(fp) | fclose(fp);
> + err = ferror(fp);
> + err |= fclose(fp);
> } else {
> err = close(fd);
> }
>
Thanks for fixing this; the old code was definitely wrong.
As you pointed out, if ferror() fails, it doesn't set errno properly. At
least one caller tries to strerror(errno), so it would probably be good
to store *something* in there, probably EIO.
To be really pedantic, there's also the question of what errno the
caller would want if *both* ferror() and fclose() fail. Normally I would
say "the first error that occurred", but in this case we don't know the
correct errno from the error reported by ferror(), so maybe the fclose()
errno is more likely to hint at the underlying reason for the failure.
So I (reluctantly) propose
if (ferror(fp)) {
if (!fclose(fp)) {
/*
* ferror() doesn't set errno, so we have to
* set one. (By contrast, when fclose() fails
* too, we leave *its* errno in place.)
*/
errno = EIO;
}
return -1;
}
return fclose();
Michael
^ permalink raw reply
* Re: difflame improvements
From: Edmundo Carmona Antoranz @ 2017-02-17 7:01 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
In-Reply-To: <20170217051755.fx2ueizsprw2vida@sigill.intra.peff.net>
On Thu, Feb 16, 2017 at 11:17 PM, Jeff King <peff@peff.net> wrote:
> This isn't difflame's fault; that's what "git blame" tells you about
> that line. But since I already told difflame "v2.6.5..HEAD", it would
> probably make sense to similarly limit the blame to that range. That
> turns up a boundary commit for the line. Which is _also_ not helpful,
> but at least the tool is telling me that the line came from before
> v2.6.5, and I don't really need to care much about it.
I'm running my own tests on difflame and I have a theory about "when"
it breaks.... at least one of the cases when it breaks:
Analysis for deleted lines is being driven by git blame --reverse.
What I have noticed is that it "breaks" when blame --reverse drives
the analysis into revisions where "treeish1" is not part of their
history (like, bringing analysis "to the sides" of treeish1 instead of
keeping analysis in revisions in the history of treeish2 that have
treeish1 as one of their ancestors.... which is definitely a valid
case for analysis, anyway). In this case, blame --reverse stops being
helpful.
Take this example (I just pushed a debug-deletion branch into gh...
probably more debugging messages will be needed):
$ difflame.py HEAD~100 HEAD -- Documentation/git-commit.txt
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index f2ab0ee2e..4f8f20a36 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -265,7 +265,8 @@ FROM UPSTREAM REBASE" section in linkgit:git-rebase[1].)
bcf9626a71 (Matthieu Moy 2016-06-28 13:40:11 +0200 265) If this
option is specified together with `--amend`, then
04c8ce9c1c (Markus Heidelberg 2008-12-19 13:14:18 +0100 266) no
paths need to be specified, which can be used to amend
d4ba07cac5 (Johannes Sixt 2008-04-10 13:33:09 +0200 267) the
last commit without committing changes that have
Range of revisions: 02db2d..066fb04
Treeish1 02db2d04: 02db2d042 Merge branch 'ah/grammos'
Treeish2 066fb0494: 066fb0494 blame: draft of line format
Blamed Revision afe0e2a39: afe0e2a39 Merge branch
'da/difftool-dir-diff-fix'
Original Filename a/Documentation/git-commit.txt Deleted Line 268
Children revisions:
3aead1cad7a: 3aead1cad Merge branch 'ak/commit-only-allow-empty'
There's only one child revision.... on that revision the line
we are tracking is gone
Parents of this child revision:
afe0e2a39166: afe0e2a39 Merge branch 'da/difftool-dir-diff-fix'
beb635ca9ce: beb635ca9 commit: remove 'Clever' message
for --only --amend
Finding parent where the line has been deleted:
beb635ca9: beb635ca9 commit: remove 'Clever' message
for --only --amend
Range of revisions: 02db2d042..beb635c
Treeish1 02db2d0: 02db2d042 Merge branch 'ah/grammos'
Treeish2 beb635c: beb635ca9 commit: remove 'Clever'
message for --only --amend
Blamed Revision 02db2d0: 02db2d042 Merge branch 'ah/grammos'
Original Filename a/Documentation/git-commit.txt Deleted Line 268
Children revisions:
Found no children... will return the original blamed revision
(02db2d0) saying that the deleting revision could not be found
beb635ca9 commit: remove 'Clever' message for --only --amend
-beb635ca9 (Andreas Krey 2016-12-09 05:10:21 +0100 268)
already been staged.
319d83524 commit: make --only --allow-empty work without paths
+319d835240 (Andreas Krey 2016-12-02 23:15:13 +0100 268)
already been staged. ...
+319d835240 (Andreas Krey 2016-12-02 23:15:13 +0100 269) paths
are also not requi...
d4ba07cac5 (Johannes Sixt 2008-04-10 13:33:09 +0200 270)
1947bdbc31 (Junio C Hamano 2008-06-22 14:32:27 -0700 271) -u[<mode>]::
1947bdbc31 (Junio C Hamano 2008-06-22 14:32:27 -0700 272)
--untracked-files[=<mode>]::
I know that line 268 was deleted on 319d835240.
So.... on the first round of merge analysis it says "let's go into
beb635ca9". That's fine. That's exactly the path that is required to
reach 319d835240. But then when using this new "range of revisions"
for git blame --reverse, we get that line 268 is not telling us
anything useful:
$ git blame --reverse -L268,268 02db2d042..beb635c --
Documentation/git-commit.txt
^02db2d042 (Junio C Hamano 2016-12-19 14:45:30 -0800 268)
already been staged.
So, instead of pointing to 319d835240 (the parent of beb635c), it's
basically saying something like "I give up". My hunch (haven't sat
down to digest all the details about the output of git blame
--reverse... YET) is that, given that 02db2d042 is _not_ part of the
history of beb635c, git blame reverse is trying to tell me just
that... and that means I'll have to "script around this scenario".
$ git merge-base 02db2d042 beb635c
0202c411edc25940cc381bf317badcdf67670be4
Thanks in advance.
^ permalink raw reply related
* Re: difflame improvements
From: Jeff King @ 2017-02-17 5:17 UTC (permalink / raw)
To: Edmundo Carmona Antoranz; +Cc: Git List
In-Reply-To: <CAOc6etYz6+FzDRbkLS2SB9=F9DP18=6uLKdfCN3D3yd2Gug-tw@mail.gmail.com>
On Tue, Feb 14, 2017 at 11:19:05PM -0600, Edmundo Carmona Antoranz wrote:
> I've been working on detecting revisions where a "real" deletion was
> made and I think I advanced a lot in that front. I still have to work
> on many scenarios (renamed files, for example... also performance) but
> at least I'm using a few runs against git-scm history and the results
> are "promising":
I played with this a bit more, and it did turn up the correct results
for some deletions in my experiments.
One thing I noticed is that it also turned up nonsense for lines that
blame in weird ways. For instance, I have a diff like this (these are
real examples, but don't pay attention to the sha1s; it's in a fork of
git, not upstream):
$ git diff v2.6.5 builtin/prune-packed.c
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index 7cf900ea07..5e3727e841 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -2,6 +2,7 @@
#include "cache.h"
#include "progress.h"
#include "parse-options.h"
+#include "gh-log.h"
static const char * const prune_packed_usage[] = {
N_("git prune-packed [-n | --dry-run] [-q | --quiet]"),
@@ -29,8 +30,11 @@ static int prune_object(const unsigned char *sha1, const char *path,
if (*opts & PRUNE_PACKED_DRY_RUN)
printf("rm -f %s\n", path);
- else
+ else {
+ gh_logf("prune", "%s Duplicate loose object pruned\n",
+ sha1_to_hex(sha1));
unlink_or_warn(path);
+ }
return 0;
}
Running difflame on it says this:
$ python /path/to/difflame.py v2.6.5..HEAD -- builtin/prune-packed.c
[...]
-2c0b29e662 (Jeff King 2016-01-26 15:27:55 -0500 32) else
+d60032f8640 builtin/prune-packed.c (Jeff King 2015-02-02 23:15:33 -0500 33) else {
+d60032f8640 builtin/prune-packed.c (Jeff King 2015-02-02 23:15:33 -0500 34) gh_logf("prune", "%s Duplicate loose object pruned\n",
+d60032f8640 builtin/prune-packed.c (Jeff King 2015-02-02 23:15:33 -0500 35) sha1_to_hex(sha1));
0d3b729680e builtin/prune-packed.c (Jeff King 2014-10-15 18:40:53 -0400 36) unlink_or_warn(path);
+2396ec85bd1 prune-packed.c (Linus Torvalds 2005-07-03 14:27:34 -0700 37) }
There are two weird things. One is that the old "else" is attributed to
my 2c0b29e662. That's quite weird, because that is a merge commit which
did not touch the file at all. I haven't tracked it down, but presumably
that is weirdness with the --reverse blame.
But there's another one, that I think is easy to fix. The closing brace
is attributed to some ancient commit from Linus. Which yes, I'm sure had
a closing brace, but not _my_ closing brace that was added by
d60032f8640, that the rest of the lines got attributed to.
This isn't difflame's fault; that's what "git blame" tells you about
that line. But since I already told difflame "v2.6.5..HEAD", it would
probably make sense to similarly limit the blame to that range. That
turns up a boundary commit for the line. Which is _also_ not helpful,
but at least the tool is telling me that the line came from before
v2.6.5, and I don't really need to care much about it.
Part of this is that my use case may be a bit different than yours. I
don't actually want to look at the blame results directly. I just want
to see the set of commits that I'd need to look at and possibly
cherry-pick in order to re-create the diff.
-Peff
^ permalink raw reply
* Re: [PATCH] show-branch: fix crash with long ref name
From: Jeff King @ 2017-02-17 5:03 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, git, Maxim Kuvyrkov, Pranit Bauva
In-Reply-To: <CAP8UFD20=zVy=1Tr4D1Rxf+a4yR_X2zmexNCTNKV5RSF9ueYrg@mail.gmail.com>
On Thu, Feb 16, 2017 at 01:40:00PM +0100, Christian Couder wrote:
> > I started to add some tests, but I had second thoughts. It _is_ nice
> > to show off the fix, but as far as regressions go, this specific case is
> > unlikely to come up again. What would be more valuable, I think, is a
> > test script which set up a very long refname (not just 150 bytes or
> > whatever) and ran it through a series of git commands.
>
> I agree that a test script running through a series of command with
> long refnames would be great.
>
> But I think the refname should not necesarily be too long. As I wrote
> in the commit message of my patch, if the ref name had been much
> longer the crash would not have happened because the ref could not
> have been created in the first place.
Right, I think there's a tension there. Too short and it is not
interesting, and too long and things start to fail for uninteresting
reasons (e.g., your filesystem can't handle the name).
> So the best would be to run through a series of commands with a
> refname ranging from let's say 80 chars to 300 chars.
>
> That would have a chance to catch crashes due to legacy code using for
> example things like `char stuff[128]` or `char stuff[256]`.
But that doesn't catch `char stuff[512]`. I think you'd really want a
range of sizes, and to test all of them. Worse, though, is it's not
clear how you decide when a test has failed. Obviously smashing the
stack is a bad outcome, but part of the goal would presumably be to
flush out unnecessary length limitations elsewhere.
I got about that far in my thinking before saying "wow, this is getting
to be complicated for not much gain".
> > So I dunno. It seems like being thorough is a
> > lot of hassle for not much gain. Being not-thorough is easy, but is
> > mostly a token that is unlikely to find any real bugs.
>
> Yeah, if we really care, it might be better to start using a fuzzer or
> a property based testing tool instead of bothering with these kind of
> tests by ourselves, which is also a different topic.
Yes, I'd agree that a fuzzer is probably a better choice. I played with
AFL a bit back when it came out, but I never got it to turn up anything
useful.
I am disappointed that this obvious memory problem survived for so long.
I did quite a bit of auditing for such problems a year or two ago, but I
focused on problematic functions like strcpy, sprintf, etc.
It's easy to use memcpy() wrong, too, but it's hard to audit. There are
a lot of calls, and you have to match up the copied length with a value
computed elsewhere. I traded a lot of them out back then for safer
variants (like the FLEX_ALLOC stuff), but many calls just fundamentally
need something unsafe like memcpy to get their job done.
-Peff
^ permalink raw reply
* Re: `git show --oneline commit` does not do what's intuitively expected
From: Jeff King @ 2017-02-17 4:05 UTC (permalink / raw)
To: Luna Kid; +Cc: git
In-Reply-To: <CA+-W3ctdRtLpziJ9TX2hqk7RagMyJpHsrfwj=rN7oXQ8EeUPnw@mail.gmail.com>
On Fri, Feb 17, 2017 at 02:51:36AM +0100, Luna Kid wrote:
> tl;dr; --> Please add --no-diff (or --msg-only) to git show. We'll
> love you for that. :)
I think it is already spelled "--no-patch", or "-s" for short.
> Please note that "show" is such a profoundly generic command verb,
> probably also used heavily in git, especially to show commits, that it
> comes to mind as second nature, without thinking, as the primary (or
> even as "the only") choice for showing various items in various ways
> -- which, in fact, it already properly does, indeed.
Right. That's what's it for. The DESCRIPTION section of the manpage
starts with:
Shows one or more objects (blobs, trees, tags and commits).
For commits it shows the log message and textual diff. It also
presents the merge commit in a special format as produced by git
diff-tree --cc.
For tags, it shows[...etc...]
I guess that second paragraph could mention "--no-patch" explicitly to
disable it. It's documented in the COMMON DIFF OPTIONS section, but of
course there are quite a few options listed, so it's easy to miss.
-Peff
^ permalink raw reply
* [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log
From: Kyle Meyer @ 2017-02-17 3:58 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170217035800.13214-1-kyle@kyleam.com>
When the current branch is renamed, the deletion of the old ref is
recorded in HEAD's log with an empty message. Now that delete_refs()
accepts a reflog message, provide a more descriptive message. This
message will not be included in the reflog of the renamed branch, but
its log already covers the renaming event with a message of "Branch:
renamed ...".
Signed-off-by: Kyle Meyer <kyle@kyleam.com>
---
refs/files-backend.c | 8 +++++++-
t/t3200-branch.sh | 4 ++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index ffa75d816..bb5df7ee6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2598,6 +2598,7 @@ static int files_rename_ref(struct ref_store *ref_store,
struct stat loginfo;
int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
struct strbuf err = STRBUF_INIT;
+ struct strbuf logmsg_del = STRBUF_INIT;
if (log && S_ISLNK(loginfo.st_mode))
return error("reflog for %s is a symlink", oldrefname);
@@ -2616,10 +2617,15 @@ static int files_rename_ref(struct ref_store *ref_store,
return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
oldrefname, strerror(errno));
- if (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {
+ strbuf_addf(&logmsg_del, "Deleted %s", oldrefname);
+
+ if (delete_ref(oldrefname, orig_sha1, REF_NODEREF, logmsg_del.buf)) {
error("unable to delete old %s", oldrefname);
+ strbuf_release(&logmsg_del);
goto rollback;
}
+ strbuf_release(&logmsg_del);
+
/*
* Since we are doing a shallow lookup, sha1 is not the
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 8a833f354..4af7cd2b7 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -139,6 +139,10 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
test $(git rev-parse --abbrev-ref HEAD) = bam
'
+test_expect_success 'git branch -M baz bam should add entry to .git/logs/HEAD' '
+ grep "Deleted refs/heads/baz$" .git/logs/HEAD >/dev/null
+'
+
test_expect_success 'git branch -M baz bam should succeed when baz is checked out as linked working tree' '
git checkout master &&
git worktree add -b baz bazdir &&
--
2.11.1
^ permalink raw reply related
* [PATCH 2/3] update-ref: pass reflog message argument to delete_refs
From: Kyle Meyer @ 2017-02-17 3:57 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170217035800.13214-1-kyle@kyleam.com>
Now that delete_refs() accepts a reflog message, pass the
user-provided message to delete_refs() rather than silently dropping
it. The doesn't matter for the deleted ref's log because the log is
deleted along with the ref, but this entry will show up in HEAD's
reflog when deleting a checked out branch.
Signed-off-by: Kyle Meyer <kyle@kyleam.com>
---
I'm not sure if the test here (or in the next patch) is worth including.
builtin/update-ref.c | 2 +-
t/t1400-update-ref.sh | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index a41f9adf1..f642acc22 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -435,7 +435,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
*/
return delete_ref(refname,
(oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,
- flags, NULL);
+ flags, msg);
else
return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
flags | create_reflog_flag,
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b0ffc0b57..65918d984 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -85,6 +85,15 @@ test_expect_success "delete $m (by HEAD)" '
'
rm -f .git/$m
+test_expect_success "deleting current branch adds message to HEAD's log" '
+ git update-ref $m $A &&
+ git symbolic-ref HEAD $m &&
+ git update-ref -mdelmsg -d $m &&
+ ! test -f .git/$m &&
+ grep "delmsg$" .git/logs/HEAD >/dev/null
+'
+rm -f .git/$m
+
test_expect_success 'update-ref does not create reflogs by default' '
test_when_finished "git update-ref -d $outside" &&
git update-ref $outside $A &&
--
2.11.1
^ permalink raw reply related
* [PATCH 1/3] delete_refs(): accept a reflog message argument
From: Kyle Meyer @ 2017-02-17 3:57 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170217035800.13214-1-kyle@kyleam.com>
When the current branch is renamed with 'git branch -m/-M' or deleted
with 'git update-ref -m<msg> -d', the event is recorded in HEAD's log
with an empty message.
In preparation for adding a more meaningful message to HEAD's log in
these cases, update delete_ref() to take a message argument and pass
it along to ref_transaction_delete(). Modify all callers to pass NULL
for the new message argument; no change in behavior is intended.
Signed-off-by: Kyle Meyer <kyle@kyleam.com>
---
builtin/am.c | 4 ++--
builtin/branch.c | 2 +-
builtin/notes.c | 4 ++--
builtin/remote.c | 4 ++--
builtin/replace.c | 2 +-
builtin/reset.c | 2 +-
builtin/symbolic-ref.c | 2 +-
builtin/tag.c | 2 +-
builtin/update-ref.c | 2 +-
fast-import.c | 2 +-
refs.c | 4 ++--
refs.h | 2 +-
refs/files-backend.c | 6 +++---
transport.c | 2 +-
14 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 31fb60578..e08c739d4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1049,7 +1049,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
} else {
write_state_text(state, "abort-safety", "");
if (!state->rebasing)
- delete_ref("ORIG_HEAD", NULL, 0);
+ delete_ref("ORIG_HEAD", NULL, 0, NULL);
}
/*
@@ -2172,7 +2172,7 @@ static void am_abort(struct am_state *state)
has_curr_head ? &curr_head : NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
else if (curr_branch)
- delete_ref(curr_branch, NULL, REF_NODEREF);
+ delete_ref(curr_branch, NULL, REF_NODEREF, NULL);
free(curr_branch);
am_destroy(state);
diff --git a/builtin/branch.c b/builtin/branch.c
index 9d30f55b0..44f23208f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -252,7 +252,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
}
if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,
- REF_NODEREF)) {
+ REF_NODEREF, NULL)) {
error(remote_branch
? _("Error deleting remote-tracking branch '%s'")
: _("Error deleting branch '%s'"),
diff --git a/builtin/notes.c b/builtin/notes.c
index 5248a9bad..991448d4e 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -681,9 +681,9 @@ static int merge_abort(struct notes_merge_options *o)
* notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE.
*/
- if (delete_ref("NOTES_MERGE_PARTIAL", NULL, 0))
+ if (delete_ref("NOTES_MERGE_PARTIAL", NULL, 0, NULL))
ret += error(_("failed to delete ref NOTES_MERGE_PARTIAL"));
- if (delete_ref("NOTES_MERGE_REF", NULL, REF_NODEREF))
+ if (delete_ref("NOTES_MERGE_REF", NULL, REF_NODEREF, NULL))
ret += error(_("failed to delete ref NOTES_MERGE_REF"));
if (notes_merge_abort(o))
ret += error(_("failed to remove 'git notes merge' worktree"));
diff --git a/builtin/remote.c b/builtin/remote.c
index 5339ed6ad..bfa8a5189 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -691,7 +691,7 @@ static int mv(int argc, const char **argv)
read_ref_full(item->string, RESOLVE_REF_READING, oid.hash, &flag);
if (!(flag & REF_ISSYMREF))
continue;
- if (delete_ref(item->string, NULL, REF_NODEREF))
+ if (delete_ref(item->string, NULL, REF_NODEREF, NULL))
die(_("deleting '%s' failed"), item->string);
}
for (i = 0; i < remote_branches.nr; i++) {
@@ -1248,7 +1248,7 @@ static int set_head(int argc, const char **argv)
head_name = xstrdup(states.heads.items[0].string);
free_remote_ref_states(&states);
} else if (opt_d && !opt_a && argc == 1) {
- if (delete_ref(buf.buf, NULL, REF_NODEREF))
+ if (delete_ref(buf.buf, NULL, REF_NODEREF, NULL))
result |= error(_("Could not delete %s"), buf.buf);
} else
usage_with_options(builtin_remote_sethead_usage, options);
diff --git a/builtin/replace.c b/builtin/replace.c
index b58c714cb..d32d0a3ae 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -121,7 +121,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
static int delete_replace_ref(const char *name, const char *ref,
const unsigned char *sha1)
{
- if (delete_ref(ref, sha1, 0))
+ if (delete_ref(ref, sha1, 0, NULL))
return 1;
printf("Deleted replace ref '%s'\n", name);
return 0;
diff --git a/builtin/reset.c b/builtin/reset.c
index 8ab915bfc..cccd3f099 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -256,7 +256,7 @@ static int reset_refs(const char *rev, const struct object_id *oid)
update_ref_oid(msg.buf, "ORIG_HEAD", orig, old_orig, 0,
UPDATE_REFS_MSG_ON_ERR);
} else if (old_orig)
- delete_ref("ORIG_HEAD", old_orig->hash, 0);
+ delete_ref("ORIG_HEAD", old_orig->hash, 0, NULL);
set_reflog_message(&msg, "updating HEAD", rev);
update_ref_status = update_ref_oid(msg.buf, "HEAD", oid, orig, 0,
UPDATE_REFS_MSG_ON_ERR);
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 96eed9446..c9d5bd3e8 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -58,7 +58,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
die("Cannot delete %s, not a symbolic ref", argv[0]);
if (!strcmp(argv[0], "HEAD"))
die("deleting '%s' is not allowed", argv[0]);
- return delete_ref(argv[0], NULL, REF_NODEREF);
+ return delete_ref(argv[0], NULL, REF_NODEREF, NULL);
}
switch (argc) {
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a967..850a0674c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -97,7 +97,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
static int delete_tag(const char *name, const char *ref,
const unsigned char *sha1, const void *cb_data)
{
- if (delete_ref(ref, sha1, 0))
+ if (delete_ref(ref, sha1, 0, NULL))
return 1;
printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, DEFAULT_ABBREV));
return 0;
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 7f30d3a76..a41f9adf1 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -435,7 +435,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
*/
return delete_ref(refname,
(oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,
- flags);
+ flags, NULL);
else
return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
flags | create_reflog_flag,
diff --git a/fast-import.c b/fast-import.c
index 64fe602f0..07412a85a 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1752,7 +1752,7 @@ static int update_branch(struct branch *b)
if (is_null_sha1(b->sha1)) {
if (b->delete)
- delete_ref(b->name, NULL, 0);
+ delete_ref(b->name, NULL, 0, NULL);
return 0;
}
if (read_ref(b->name, old_sha1))
diff --git a/refs.c b/refs.c
index cd36b64ed..4c80ebf2e 100644
--- a/refs.c
+++ b/refs.c
@@ -592,7 +592,7 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1
}
int delete_ref(const char *refname, const unsigned char *old_sha1,
- unsigned int flags)
+ unsigned int flags, const char *msg)
{
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
@@ -603,7 +603,7 @@ int delete_ref(const char *refname, const unsigned char *old_sha1,
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_delete(transaction, refname, old_sha1,
- flags, NULL, &err) ||
+ flags, msg, &err) ||
ref_transaction_commit(transaction, &err)) {
error("%s", err.buf);
ref_transaction_free(transaction);
diff --git a/refs.h b/refs.h
index 9fbff90e7..81627a63d 100644
--- a/refs.h
+++ b/refs.h
@@ -277,7 +277,7 @@ int reflog_exists(const char *refname);
* be NULL_SHA1. flags is passed through to ref_transaction_delete().
*/
int delete_ref(const char *refname, const unsigned char *old_sha1,
- unsigned int flags);
+ unsigned int flags, const char *msg);
/*
* Delete the specified references. If there are any problems, emit
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c041d4ba2..ffa75d816 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2489,7 +2489,7 @@ static int files_delete_refs(struct ref_store *ref_store,
for (i = 0; i < refnames->nr; i++) {
const char *refname = refnames->items[i].string;
- if (delete_ref(refname, NULL, flags))
+ if (delete_ref(refname, NULL, flags, NULL))
result |= error(_("could not remove reference %s"), refname);
}
@@ -2616,7 +2616,7 @@ static int files_rename_ref(struct ref_store *ref_store,
return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
oldrefname, strerror(errno));
- if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
+ if (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {
error("unable to delete old %s", oldrefname);
goto rollback;
}
@@ -2630,7 +2630,7 @@ static int files_rename_ref(struct ref_store *ref_store,
*/
if (!read_ref_full(newrefname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
sha1, NULL) &&
- delete_ref(newrefname, NULL, REF_NODEREF)) {
+ delete_ref(newrefname, NULL, REF_NODEREF, NULL)) {
if (errno==EISDIR) {
struct strbuf path = STRBUF_INIT;
int result;
diff --git a/transport.c b/transport.c
index d72e08948..e025d9b29 100644
--- a/transport.c
+++ b/transport.c
@@ -299,7 +299,7 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v
if (verbose)
fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
if (ref->deletion) {
- delete_ref(rs.dst, NULL, 0);
+ delete_ref(rs.dst, NULL, 0, NULL);
} else
update_ref("update by push", rs.dst,
ref->new_oid.hash, NULL, 0, 0);
--
2.11.1
^ permalink raw reply related
* [PATCH 0/3] delete_ref(): support reflog messages
From: Kyle Meyer @ 2017-02-17 3:57 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170126211205.5gz3zsrptop7n34n@sigill.intra.peff.net>
[Sorry for the slow response.]
Jeff King <peff@peff.net> writes:
> On Mon, Jan 16, 2017 at 06:17:29PM -0500, Kyle Meyer wrote:
[...]
>> $ cat .git/logs/refs/heads/new-master
>> 00000... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 commit (initial): Add file.txt
>> 68730... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 Branch: renamed refs/heads/master to refs/heads/new-master
>>
>> $ cat .git/logs/HEAD
>> 00000... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 commit (initial): Add file.txt
>> 68730... 00000... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500
>>
>> I expected the second line of .git/logs/HEAD to mirror the second line
>> of .git/logs/refs/heads/new-master. Are the empty message and null sha1
>> in HEAD's entry intentional?
>
> The null sha1 is correct, I think. The branch we were on went away, and
> we use the null sha1 to indicate "no value" in both the creation and
> deletion cases.
I see, thanks.
> I'd say there are two potential improvements:
>
> - delete_ref() should take a reflog message argument, in case it
> updates the HEAD reflog (as a bonus, this will future-proof us for a
> day when we might keep reflogs for deleted refs).
I've tried to do this in the following patches.
> - "git branch -m" does seem to realize when we are renaming HEAD,
> because it updates HEAD to point to the new branch name. But it
> should probably insert another reflog entry mentioning the rename
> (we do for "git checkout foo", even when "foo" has the same sha1 as
> the current HEAD).
I haven't worked out how to do this part yet. I'm guessing the change
will involve modifying split_head_update().
If this is added, should it be instead of, rather than in addition to,
the deletion entry? If a "Branch: renamed ..." entry is present, it
doesn't seem like the deletion entry is providing any additional
information.
delete_refs(): accept a reflog message argument
update-ref: pass reflog message argument to delete_refs
rename_ref: replace empty deletion message in HEAD's log
builtin/am.c | 4 ++--
builtin/branch.c | 2 +-
builtin/notes.c | 4 ++--
builtin/remote.c | 4 ++--
builtin/replace.c | 2 +-
builtin/reset.c | 2 +-
builtin/symbolic-ref.c | 2 +-
builtin/tag.c | 2 +-
builtin/update-ref.c | 2 +-
fast-import.c | 2 +-
refs.c | 4 ++--
refs.h | 2 +-
refs/files-backend.c | 12 +++++++++---
t/t1400-update-ref.sh | 9 +++++++++
t/t3200-branch.sh | 4 ++++
transport.c | 2 +-
16 files changed, 39 insertions(+), 20 deletions(-)
--
Kyle
^ permalink raw reply
* `git show --oneline commit` does not do what's intuitively expected
From: Luna Kid @ 2017-02-17 1:51 UTC (permalink / raw)
To: git
Hello,
tl;dr; --> Please add --no-diff (or --msg-only) to git show. We'll
love you for that. :)
Expanded:
(I've been a casual git user for 2-3 years; "casual" means using a
small subset, nowadays daily, with some less trivial things at times,
like per-commit patching for post-hoc auto-versioning, or
push-deploying (the old way), so not 100% newbie; doing SW for 25+
years. I'm telling this, because I'm probably a representative sample
of a huge number of experienced professionals, who are relatively new
to git (compared to their age...), but already routinely use it in
"survival mode", learning it gradually, on-demand.)
Today I bumped into this issue, which I then tried googling for, and
found it kinda' hanging:
http://stackoverflow.com/questions/17226117/git-show-commit-oneline-not-showing-oneline
The source of the confusion is that git show --oneline insists on
showing the diff, no matter what, while the man page is misleadingly
subtle on that, as illustrated best by a comment on that SO page,
exactly matching my case, too:
"the answer does not however explain why git show HEAD --oneline
does not produce an output as stated in the documentation:
<sha1> <title line> This is designed to be as compact as possible."
Note: I do understand now (after 5-10 minutes of googling in vain,
mucking around with git, re-reading the man page a few times, and
considering the fact that a bug like that in such a feature couldn't
exist for this long -- in fact, this clue helped me more than the docs
+ googling together), that it's indeed, not a bug, but our lack of
intimate familiarity with git and the terseness/wording of the man
page together cause the confusion.
(BTW, just realized: on the git show man page, also the strangely
implied "log of a commit" concept (i.e. in "Pretty-print the contents
of the commit logs") is partly responsible for the confusion. The term
"log" doesn't mean a single event-like item, but a series of entries,
a running record of events. In VCS lingo the established concept is
that "a log is a list of commits", and that's even the case with git,
too, of course. So, readers of the git show man page will have an
impedance match there, and unnecessary difficulties understanding what
is actually going on. My overloaded brain, for example, apparently
opted for just filtering out that part without warning, upon the first
(two) reading.)
Assuming that we (the SO guy + his upvoters + me) are not the only
ones, I'd suggest that instead of perhaps changing the man page,
there's an other easy, also backwardly compatible, and quite
straightforward way to address this: actually implementing that
"unexpectedly missing" feature people intuitively look for.
Please note that "show" is such a profoundly generic command verb,
probably also used heavily in git, especially to show commits, that it
comes to mind as second nature, without thinking, as the primary (or
even as "the only") choice for showing various items in various ways
-- which, in fact, it already properly does, indeed.
Forcing us to use a different command (git log) for a minor sub-case
of the main "show me a commit" scenario (of git show), is highly
unnatural.
Also, --oneline (as all the other formatting options) look and feel
global to the entire context (result) of the show command, so people
who have not yet unlearned to expect that, will be surprised
(unpleasantly).
However, simply adding --no-diff to the git show command (and the man
page) would help a lot. (Something like --message-only or --only-msg
etc., would be more correct (than the "potentially leaky" complement),
but I'm not familiar with the general use of the "only" modifier in
git options and cannot comment on that, but I've certainly seen the
--no-... form at least.)
Thanks a lot, and have a nice next message! :)
Szabolcs
^ permalink raw reply
* Re: [PATCH] config: preserve <subsection> case for one-shot config on the command line
From: Junio C Hamano @ 2017-02-17 1:25 UTC (permalink / raw)
To: Jeff King; +Cc: Lars Schneider, Jonathan Tan, git, sbeller
In-Reply-To: <20170216232730.xsx3xks5ppjws5rg@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Feb 16, 2017 at 11:30:28AM +0100, Lars Schneider wrote:
>
>>
>> > On 16 Feb 2017, at 00:48, Junio C Hamano <gitster@pobox.com> wrote:
>> >
>> > The "git -c <var>=<val> cmd" mechanism is to pretend that a
>>
>> The problem is also present for gitconfig variables e.g.
>> git config --local submodule.UPPERSUB.update none
>
> Hrm, is it?
>
> $ git config --file foo submodule.UPPERSUB.update none
> $ cat foo
> [submodule "UPPERSUB"]
> update = none
>
> I could believe that some of the submodule code may try to pass it
> through "-c", though, so certain config ends up being missed.
You are right.
The builtin/config.c::get_value() codepath, when it is not using the
hacky regexp interface, uses config.c::git_config_parse_key(), and
it does the right thing. git_config_parse_parameter(), which is the
broken one we found, is not used.
When I did the patch in response to Jonathan's observation, I did
briefly wonder if it should be using git_config_parse_key() instead
of doing its own thing, but I didn't follow it up fully before
deciding that it is the quickest to replace the tolower thing. If I
at least tried to see if it is feasible, I would have noticed that
the query from the command line wouldn't share the same problem as
Lars reported.
I still haven't queued any of the variants I posted (and I do not
think other people sent their own versions, either). I need to pick
one and queue, with a test or two. Perhaps after -rc2.
Others are welcome to work on it while I cut -rc2 tomorrow, so that
by the time I see their patch all that is left for me to do is to
apply it ;-)
^ permalink raw reply
* Re: [PATCH V2 0/2] Fix l10n
From: Junio C Hamano @ 2017-02-17 1:16 UTC (permalink / raw)
To: MM; +Cc: git, peff, jonathantanmy
In-Reply-To: <1487288237.25073.1.camel@gmail.com>
Unknown <franchesko.salias.hudro.pedros@gmail.com> writes:
> В Чт, 16/02/2017 в 15:33 -0800, Junio C Hamano пишет:
>>
>> Thanks. Queued with minor log message fixes and pushed out.
>
> If is not too late, please check the version 3.
I already squashed in the change s/bufp/buf/ in the version I queued
and pushed out on 'pu', so we should be good. I compared the result
of applying your updated patches and did not see any other changes.
Thanks.
^ permalink raw reply
* Re: [PATCH V2 0/2] Fix l10n
From: Unknown @ 2017-02-16 23:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, peff, jonathantanmy
In-Reply-To: <xmqq8tp5vhgk.fsf@gitster.mtv.corp.google.com>
В Чт, 16/02/2017 в 15:33 -0800, Junio C Hamano пишет:
> Maxim Moseychuk <franchesko.salias.hudro.pedros@gmail.com> writes:
>
> > In some places static size buffers can't store formatted string.
> > If it be happen then git die.
> >
> > Jonathan Tan: Thanks a lot for your help.
> >
> > Maxim Moseychuk (2):
> > bisect_next_all: convert xsnprintf to xstrfmt
> > stop_progress_msg: convert xsnprintf to xstrfmt
> >
> > bisect.c | 9 +++++----
> > progress.c | 9 +++------
> > 2 files changed, 8 insertions(+), 10 deletions(-)
> >
> > --
> > 2.11.1
>
> Thanks. Queued with minor log message fixes and pushed out.
If is not too late, please check the version 3.
^ permalink raw reply
* Re: [PATCH V2 0/2] Fix l10n
From: Junio C Hamano @ 2017-02-16 23:33 UTC (permalink / raw)
To: Maxim Moseychuk; +Cc: git, peff, jonathantanmy
In-Reply-To: <20170216170713.10065-1-franchesko.salias.hudro.pedros@gmail.com>
Maxim Moseychuk <franchesko.salias.hudro.pedros@gmail.com> writes:
> In some places static size buffers can't store formatted string.
> If it be happen then git die.
>
> Jonathan Tan: Thanks a lot for your help.
>
> Maxim Moseychuk (2):
> bisect_next_all: convert xsnprintf to xstrfmt
> stop_progress_msg: convert xsnprintf to xstrfmt
>
> bisect.c | 9 +++++----
> progress.c | 9 +++------
> 2 files changed, 8 insertions(+), 10 deletions(-)
>
> --
> 2.11.1
Thanks. Queued with minor log message fixes and pushed out.
^ permalink raw reply
* [PATCH v3 2/2] stop_progress_msg(): simplification function
From: Maxim Moseychuk @ 2017-02-16 23:32 UTC (permalink / raw)
To: git; +Cc: peff, jonathantanmy, Maxim Moseychuk
In-Reply-To: <20170216233249.24757-1-franchesko.salias.hudro.pedros@gmail.com>
stop_progress_msg() is rarely used and is not demanding to
performance. Use dynamically allocates memory.
Signed-off-by: Maxim Moseychuk <franchesko.salias.hudro.pedros@gmail.com>
---
progress.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/progress.c b/progress.c
index 76a88c573..29378caa0 100644
--- a/progress.c
+++ b/progress.c
@@ -243,21 +243,18 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
*p_progress = NULL;
if (progress->last_value != -1) {
/* Force the last update */
- char buf[128], *bufp;
- size_t len = strlen(msg) + 5;
+ char *buf;
struct throughput *tp = progress->throughput;
- bufp = (len < sizeof(buf)) ? buf : xmallocz(len);
if (tp) {
unsigned int rate = !tp->avg_misecs ? 0 :
tp->avg_bytes / tp->avg_misecs;
throughput_string(&tp->display, tp->curr_total, rate);
}
progress_update = 1;
- xsnprintf(bufp, len + 1, ", %s.\n", msg);
- display(progress, progress->last_value, bufp);
- if (buf != bufp)
- free(bufp);
+ buf = xstrfmt(", %s.\n", msg);
+ display(progress, progress->last_value, buf);
+ free(buf);
}
clear_progress_signal();
if (progress->throughput)
--
2.11.1
^ permalink raw reply related
* [PATCH v3 1/2] bisect_next_all(): fix bisect crash when used the gettext translation
From: Maxim Moseychuk @ 2017-02-16 23:32 UTC (permalink / raw)
To: git; +Cc: peff, jonathantanmy, Maxim Moseychuk
In-Reply-To: <20170216233249.24757-1-franchesko.salias.hudro.pedros@gmail.com>
The buffer steps_msg[32] is too small.
Translated "(roughly %d step)" string can not be located in the buffer.
Solution: using xstrfmt which dynamically allocates memory.
Dummy solution: just increase steps_msg size but is not safe.
That feels pretty hacky, though. In practice the set of translations is
contained, but it doesn't have to be (and certainly nobody would notice
if a new translation was added with a longer name until a user
complained).
Reproduce bug: "LANG=ru_RU.UTF8 git bisect start v4.9 v4.8" on linux sources.
Signed-off-by: Maxim Moseychuk <franchesko.salias.hudro.pedros@gmail.com>
---
bisect.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/bisect.c b/bisect.c
index 21bc6daa4..787543cad 100644
--- a/bisect.c
+++ b/bisect.c
@@ -940,7 +940,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
struct commit_list *tried;
int reaches = 0, all = 0, nr, steps;
const unsigned char *bisect_rev;
- char steps_msg[32];
+ char *steps_msg;
read_bisect_terms(&term_bad, &term_good);
if (read_bisect_refs())
@@ -990,14 +990,15 @@ int bisect_next_all(const char *prefix, int no_checkout)
nr = all - reaches - 1;
steps = estimate_bisect_steps(all);
- xsnprintf(steps_msg, sizeof(steps_msg),
- Q_("(roughly %d step)", "(roughly %d steps)", steps),
- steps);
+
+ steps_msg = xstrfmt(Q_("(roughly %d step)", "(roughly %d steps)",
+ steps), steps);
/* TRANSLATORS: the last %s will be replaced with
"(roughly %d steps)" translation */
printf(Q_("Bisecting: %d revision left to test after this %s\n",
"Bisecting: %d revisions left to test after this %s\n",
nr), nr, steps_msg);
+ free(steps_msg);
return bisect_checkout(bisect_rev, no_checkout);
}
--
2.11.1
^ permalink raw reply related
* [PATCH v3 0/2] Fix l10n
From: Maxim Moseychuk @ 2017-02-16 23:32 UTC (permalink / raw)
To: git; +Cc: peff, jonathantanmy, Maxim Moseychuk
In some places fixed-size buffers can't store formatted string.
If it be happen then git die.
Jonathan Tan, Jeff King thanks a lot for your help.
This is really my first patches. Your help is invaluable.
Maxim Moseychuk (2):
bisect_next_all(): fix bisect crash when used the gettext translation
stop_progress_msg(): simplification function
bisect.c | 9 +++++----
progress.c | 11 ++++-------
2 files changed, 9 insertions(+), 11 deletions(-)
--
2.11.1
^ permalink raw reply
* Re: [PATCH] config: preserve <subsection> case for one-shot config on the command line
From: Jeff King @ 2017-02-16 23:27 UTC (permalink / raw)
To: Lars Schneider; +Cc: Junio C Hamano, Jonathan Tan, git, sbeller
In-Reply-To: <D0CDD1AC-05CA-47F3-8CB5-61EA1C6515A8@gmail.com>
On Thu, Feb 16, 2017 at 11:30:28AM +0100, Lars Schneider wrote:
>
> > On 16 Feb 2017, at 00:48, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > The "git -c <var>=<val> cmd" mechanism is to pretend that a
>
> The problem is also present for gitconfig variables e.g.
> git config --local submodule.UPPERSUB.update none
Hrm, is it?
$ git config --file foo submodule.UPPERSUB.update none
$ cat foo
[submodule "UPPERSUB"]
update = none
I could believe that some of the submodule code may try to pass it
through "-c", though, so certain config ends up being missed.
AFAICT, though, the writing code takes what you gave it verbatim. The
reader is responsible for downcasing everything but the subsection
before it hands it to a callback. Commands calling git-config for lookup
should generally ask for the canonical downcased name. There is some
code to downcase, but IIRC there are corner cases around some of the
regex lookup functions.
-Peff
^ permalink raw reply
* Re: [RFCv4 PATCH 00/14] Checkout aware of Submodules!
From: Jacob Keller @ 2017-02-16 23:27 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Git mailing list, brian m. carlson,
Jonathan Nieder, Brandon Williams
In-Reply-To: <CAGZ79kYQf2SL-RVETv8=6NaZHhub2kQmKevK32X_xfFkr_0yyA@mail.gmail.com>
On Thu, Feb 16, 2017 at 2:56 PM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Feb 16, 2017 at 2:54 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Thu, Feb 16, 2017 at 2:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Stefan Beller <sbeller@google.com> writes:
>>>
>>>> Integrate updating the submodules into git checkout,...
>>>
>>> It was more or less a pleasant read, once I decided to pretend that
>>> I were a machine who uses identifiers only to identify locations in
>>> the program ;-) IOW, for human consumption, the new names introduced
>>> were sometimes quite confusing and went against helping understanding.
>>>
>>
>> Based on my cursory reading, I agree. I had trouble understanding how
>> the process worked because the names were somewhat confusing. They
>> started to make some sense as I read more. I think v4 had better names
>> than v3, but they were still somewhat confusing to me.
>>
>
> Now if only you could tell me what names were better to understand. ;)
> I'll reply to the individual patch remarks and hopefully there we find
> good names for these functions.
>
> Thanks,
> Stefan
I'll try to read it again and see if I think of anything.
Thanks,
Jake
^ permalink raw reply
* Re: [BUG] submodule config does not apply to upper case submodules?
From: Jeff King @ 2017-02-16 23:22 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stefan Beller, Jonathan Tan, Lars Schneider, git@vger.kernel.org
In-Reply-To: <xmqqh93v10vy.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 15, 2017 at 03:37:37PM -0800, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
>
> > Yes; though I'd place it in strbuf.{c,h} as it is operating
> > on the internals of the strbuf. (Do we make any promises outside of
> > strbuf about the internals? I mean we use .buf all the time, so maybe
> > I am overly cautious here)
>
> I'd rather have it not use struct strbuf as an interface. It only
> needs to pass "char *" and its promise that it touches the string
> in-place without changing the length need to be documented as a
> comment before the function.
This code also uses the hacky strbuf_split() interface. It would be nice
to one day move off of it (the only other strbuf-specific function used
there is strbuf_trim).
One _could_ actually parse the whole thing left-to-right (soaking up
whitespace and doing the canonicalizing) instead of dealing with a split
function at all. But the canonicalize bit you added here would not be
reusable then. And it's probably not worth holding up the bugfix here.
> >> +static void canonicalize_config_variable_name(struct strbuf *var)
> >> +{
> >> + char *first_dot = strchr(var->buf, '.');
> >> + char *last_dot = strrchr(var->buf, '.');
> >
> > If first_dot != NULL, then last_dot !+ NULL as well.
> > (either both are NULL or none of them),
> > so we can loose one condition below.
>
> I do not think it is worth it, though.
If you really want to be picky, you do not need to find the first dot
at all. You can downcase everything until you see a dot, and then
find the last dot (if any) from there.
I don't think it matters much in practice.
-Peff
^ permalink raw reply
* What's cooking in git.git (Feb 2017, #05; Thu, 16)
From: Junio C Hamano @ 2017-02-16 23:01 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
I'd like to get pull requests for gitk and git-gui updates soonish,
if we are to have one during this cycle.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* cw/completion (2017-02-03) 7 commits
(merged to 'next' on 2017-02-10 at b3a5cbf39c)
+ completion: recognize more long-options
+ completion: teach remote subcommands to complete options
+ completion: teach replace to complete options
+ completion: teach ls-remote to complete options
+ completion: improve bash completion for git-add
+ completion: add subcommand completion for rerere
+ completion: teach submodule subcommands to complete options
More command line completion (in contrib/) for recent additions.
* dp/submodule-doc-markup-fix (2017-02-16) 1 commit
(merged to 'next' on 2017-02-16 at 698cdcff0a)
+ config.txt: fix formatting of submodule.alternateErrorStrategy section
Doc fix.
* jk/doc-remote-helpers-markup-fix (2017-02-13) 1 commit
(merged to 'next' on 2017-02-14 at 95af1267c7)
+ docs/gitremote-helpers: fix unbalanced quotes
Doc markup fix.
* jk/doc-submodule-markup-fix (2017-02-13) 1 commit
(merged to 'next' on 2017-02-14 at b2f807f7d8)
+ docs/git-submodule: fix unbalanced quote
Doc markup fix.
* jk/reset-to-break-a-commit-doc (2017-02-16) 1 commit
(merged to 'next' on 2017-02-16 at 3203e91525)
+ Revert "reset: add an example of how to split a commit into two"
(this branch is used by jk/reset-to-break-a-commit-doc-updated.)
Doc update.
* jk/reset-to-break-a-commit-doc-updated (2017-02-16) 1 commit
(merged to 'next' on 2017-02-16 at bf729e9e25)
+ reset: add an example of how to split a commit into two
(this branch uses jk/reset-to-break-a-commit-doc.)
Doc update.
* jk/tempfile-ferror-fclose-confusion (2017-02-16) 1 commit
(merged to 'next' on 2017-02-16 at 86cc4e77d7)
+ tempfile: avoid "ferror | fclose" trick
Code clean-up.
* js/mingw-isatty (2017-02-14) 1 commit
(merged to 'next' on 2017-02-15 at f3d3ccc978)
+ mingw: make stderr unbuffered again
A hotfix for a topic already in 'master'.
* ls/p4-path-encoding (2017-02-10) 1 commit
(merged to 'next' on 2017-02-15 at 73af90bf0f)
+ git-p4: fix git-p4.pathEncoding for removed files
When "git p4" imports changelist that removes paths, it failed to
convert pathnames when the p4 used encoding different from the one
used on the Git side. This has been corrected.
* rs/cocci-check-free-only-null (2017-02-11) 1 commit
(merged to 'next' on 2017-02-15 at a628ee7142)
+ cocci: detect useless free(3) calls
A new coccinelle rule that catches a check of !pointer before the
pointer is free(3)d, which most likely is a bug.
* rs/ls-files-partial-optim (2017-02-13) 2 commits
(merged to 'next' on 2017-02-15 at 7a21b55424)
+ ls-files: move only kept cache entries in prune_cache()
+ ls-files: pass prefix length explicitly to prune_cache()
"ls-files" run with pathspec has been micro-optimized to avoid
having to memmove(3) unnecessary bytes.
* rs/strbuf-cleanup-in-rmdir-recursively (2017-02-13) 1 commit
(merged to 'next' on 2017-02-15 at 920f58b3b1)
+ rm: reuse strbuf for all remove_dir_recursively() calls, again
Code clean-up.
* rs/swap (2017-01-30) 5 commits
(merged to 'next' on 2017-02-10 at 5253797d0a)
+ graph: use SWAP macro
+ diff: use SWAP macro
+ use SWAP macro
+ apply: use SWAP macro
+ add SWAP macro
Code clean-up.
* sb/doc-unify-bottom (2017-02-09) 1 commit
(merged to 'next' on 2017-02-10 at 7229c4c1f7)
+ Documentation: unify bottom "part of git suite" lines
Doc clean-up.
* sb/push-options-via-transport (2017-02-08) 1 commit
(merged to 'next' on 2017-02-10 at 3e2d08e1fa)
+ push options: pass push options to the transport helper
The push-options given via the "--push-options" option were not
passed through to external remote helpers such as "smart HTTP" that
are invoked via the transport helper.
* sb/submodule-doc (2017-01-12) 2 commits
(merged to 'next' on 2017-02-10 at 5bfad5f30e)
+ submodule update documentation: don't repeat ourselves
+ submodule documentation: add options to the subcommand
Doc updates.
* tg/stash-doc-cleanup (2017-02-13) 1 commit
(merged to 'next' on 2017-02-14 at 5b9ffbc741)
+ Documentation/stash: remove mention of git reset --hard
(this branch is used by tg/stash-push.)
The documentation explained what "git stash" does to the working
tree (after stashing away the local changes) in terms of "reset
--hard", which was exposing an unnecessary implementation detail.
--------------------------------------------------
[New Topics]
* ls/submodule-config-ucase (2017-02-15) 2 commits
- submodule config does not apply to upper case submodules?
- t7400: cleanup "submodule add clone shallow submodule" test
Demonstrate a breakage in handling submodule.UPPERCASENAME.update
configuration variables.
cf. <f238248f-0df2-19a5-581d-95c8a61b4632@google.com>
Jonathan Tan already found the root cause of this, which is not
limited to submodules.
* nd/clean-preserve-errno-in-warning (2017-02-16) 1 commit
(merged to 'next' on 2017-02-16 at c0802f7627)
+ clean: use warning_errno() when appropriate
Some warning() messages from "git clean" were updated to show the
errno from failed system calls.
Will cook in 'next'.
* mm/two-more-xstrfmt (2017-02-16) 2 commits
- bisect_next_all: convert xsnprintf to xstrfmt
- stop_progress_msg: convert xsnprintf to xstrfmt
Code clean-up and a string truncation fix.
Will merge to 'next' and then to 'master'.
* sb/checkout-recurse-submodules (2017-02-16) 15 commits
- builtin/checkout: add --recurse-submodules switch
- entry.c: update submodules when interesting
- read-cache: remove_marked_cache_entries to wipe selected submodules.
- unpack-trees: check if we can perform the operation for submodules
- unpack-trees: pass old oid to verify_clean_submodule
- update submodules: add submodule_go_from_to
- update submodules: move up prepare_submodule_repo_env
- submodules: introduce check to see whether to touch a submodule
- update submodules: add a config option to determine if submodules are updated
- update submodules: add submodule config parsing
- connect_work_tree_and_git_dir: safely create leading directories
- make is_submodule_populated gently
- lib-submodule-update.sh: define tests for recursing into submodules
- lib-submodule-update.sh: do not use ./. as submodule remote
- lib-submodule-update.sh: reorder create_lib_submodule_repo
"git checkout" is taught --recurse-submodules option.
--------------------------------------------------
[Stalled]
* vn/xdiff-func-context (2017-01-15) 1 commit
- xdiff -W: relax end-of-file function detection
"git diff -W" has been taught to handle the case where a new
function is added at the end of the file better.
Will hold.
Discussion on an follow-up change to go back from the line that
matches the funcline to show comments before the function
definition has not resulted in an actionable conclusion.
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Expecting a reroll.
cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>
cf. <CAFZEwPNUXcNY9Qdz=_B7q2kQuaecPzJtTMGdv8YMUPEz2vnp8A@mail.gmail.com>
* ls/filter-process-delayed (2017-01-08) 1 commit
. convert: add "status=delayed" to filter process protocol
Ejected, as does not build when merged to 'pu'.
* sh/grep-tree-obj-tweak-output (2017-01-20) 2 commits
- grep: use '/' delimiter for paths
- grep: only add delimiter if there isn't one already
"git grep", when fed a tree-ish as an input, shows each hit
prefixed with "<tree-ish>:<path>:<lineno>:". As <tree-ish> is
almost always either a commit or a tag that points at a commit, the
early part of the output "<tree-ish>:<path>" can be used as the
name of the blob and given to "git show". When <tree-ish> is a
tree given in the extended SHA-1 syntax (e.g. "<commit>:", or
"<commit>:<dir>"), however, this results in a string that does not
name a blob (e.g. "<commit>::<path>" or "<commit>:<dir>:<path>").
"git grep" has been taught to be a bit more intelligent about these
cases and omit a colon (in the former case) or use slash (in the
latter case) to produce "<commit>:<path>" and
"<commit>:<dir>/<path>" that can be used as the name of a blob.
Waiting for the review discussion to settle, followed by a reroll.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* jh/preload-index-skip-skip (2017-02-10) 1 commit
(merged to 'next' on 2017-02-16 at 39077062f9)
+ preload-index: avoid lstat for skip-worktree items
The preload-index code has been taught not to bother with the index
entries that are paths that are not checked out by "sparse checkout".
Will cook in 'next'.
* tg/stash-push (2017-02-13) 6 commits
- stash: allow pathspecs in the no verb form
- stash: use stash_push for no verb form
- stash: teach 'push' (and 'create') to honor pathspec
- stash: introduce new format create
- stash: add test for the create command line arguments
- stash: introduce push verb
Allow "git stash" to take pathspec so that the local changes can be
stashed away only partially.
Waiting for the review discussion to settle, followed by a reroll.
cf. <20170212215420.16701-1-t.gummerer@gmail.com>
* bc/object-id (2017-02-14) 19 commits
- wt-status: convert to struct object_id
- builtin/merge-base: convert to struct object_id
- object_id: use struct object_id in object iteration callbacks
- sha1_file: introduce an nth_packed_object_oid function
- refs: simplify parsing of reflog entries
- hex: introduce parse_oid_hex
- refs: convert each_reflog_ent_fn to struct object_id
- reflog-walk: convert struct reflog_info to struct object_id
- builtin/replace: convert to struct object_id
- object_id: convert remaining callers of resolve_refdup()
- builtin/merge: convert to struct object_id
- builtin/clone: convert to struct object_id
- builtin/branch: convert to struct object_id
- builtin/grep: convert to struct object_id
- builtin/fmt-merge-message: convert to struct object_id
- builtin/fast-export: convert to struct object_id
- builtin/describe: convert to struct object_id
- builtin/diff-tree: convert to struct object_id
- builtin/commit: convert to struct object_id
"uchar [40]" to "struct object_id" conversion continues.
* jk/grep-no-index-fix (2017-02-14) 7 commits
(merged to 'next' on 2017-02-16 at c84c927fa8)
+ grep: treat revs the same for --untracked as for --no-index
+ grep: do not diagnose misspelt revs with --no-index
+ grep: avoid resolving revision names in --no-index case
+ grep: fix "--" rev/pathspec disambiguation
+ grep: re-order rev-parsing loop
+ grep: do not unnecessarily query repo for "--"
+ grep: move thread initialization a little lower
The code to parse the command line "git grep <patterns>... <rev>
[[--] <pathspec>...]" has been cleaned up, and a handful of bugs
have been fixed (e.g. we used to check "--" if it is a rev).
Will cook in 'next'.
* jk/show-branch-lift-name-len-limit (2017-02-15) 3 commits
(merged to 'next' on 2017-02-16 at 40d22f5f34)
+ show-branch: use skip_prefix to drop magic numbers
+ show-branch: store resolved head in heap buffer
+ show-branch: drop head_len variable
"git show-branch" expected there were only very short branch names
in the repository and used a fixed-length buffer to hold them
without checking for overflow.
Will cook in 'next'.
* lt/oneline-decoration-at-end (2017-02-14) 1 commit
(merged to 'next' on 2017-02-16 at 5854e58811)
+ show decorations at the end of the line
The output from "git log --oneline --decorate" has been updated to
show the extra information at the end of the line, not near the
front.
Will cook in 'next'.
* jn/remote-helpers-with-git-dir (2017-02-14) 2 commits
(merged to 'next' on 2017-02-16 at c093c543c4)
+ remote helpers: avoid blind fall-back to ".git" when setting GIT_DIR
+ remote: avoid reading $GIT_DIR config in non-repo
"git ls-remote" and "git archive --remote" are designed to work
without being in a directory under Git's control. However, recent
updates revealed that we randomly look into a directory called
.git/ without actually doing necessary set-up when working in a
repository. Stop doing so.
Will cook in 'next'.
* jk/alternate-ref-optim (2017-02-08) 11 commits
(merged to 'next' on 2017-02-10 at f26f32cff6)
+ receive-pack: avoid duplicates between our refs and alternates
+ receive-pack: treat namespace .have lines like alternates
+ receive-pack: fix misleading namespace/.have comment
+ receive-pack: use oidset to de-duplicate .have lines
+ add oidset API
+ fetch-pack: cache results of for_each_alternate_ref
+ for_each_alternate_ref: replace transport code with for-each-ref
+ for_each_alternate_ref: pass name/oid instead of ref struct
+ for_each_alternate_ref: use strbuf for path allocation
+ for_each_alternate_ref: stop trimming trailing slashes
+ for_each_alternate_ref: handle failure from real_pathdup()
Optimizes resource usage while enumerating refs from alternate
object store, to help receiving end of "push" that hosts a
repository with many "forks".
Will cook in 'next'.
* lt/pathspec-negative (2017-02-10) 2 commits
(merged to 'next' on 2017-02-10 at 8ea7874076)
+ pathspec: don't error out on all-exclusionary pathspec patterns
+ pathspec magic: add '^' as alias for '!'
The "negative" pathspec feature was somewhat more cumbersome to use
than necessary in that its short-hand used "!" which needed to be
escaped from shells, and it required "exclude from what?" specified.
Will cook in 'next'.
* nd/worktree-gc-protection (2017-02-08) 2 commits
- worktree.c: use submodule interface to access refs from another worktree
- refs.c: add resolve_ref_submodule()
(hopefully) a beginning of safer "git worktree" that is resistant
to "gc".
Michael had a suggestion for a (hopefully) better alternative way
to do this. Expecting a reroll (either a tenative but quicker way,
or a better but slower way).
cf. <CACsJy8Diy92CNbJ1OBn893VFFrSsxBFWSyQHjt_Dzq9x7jfibQ@mail.gmail.com>
* js/rebase-helper (2017-02-09) 2 commits
(merged to 'next' on 2017-02-14 at ae2474048e)
+ rebase -i: use the rebase--helper builtin
+ rebase--helper: add a builtin helper for interactive rebases
"git rebase -i" starts using the recently updated "sequencer" code.
Will cook in 'next'.
The change itself is small, but what it enables is rather a large
body of new code. We are getting there ;-)
* mh/submodule-hash (2017-02-13) 9 commits
(merged to 'next' on 2017-02-14 at 43f2dcbe29)
+ read_loose_refs(): read refs using resolve_ref_recursively()
+ files_ref_store::submodule: use NULL for the main repository
+ base_ref_store_init(): remove submodule argument
+ refs: push the submodule attribute down
+ refs: store submodule ref stores in a hashmap
+ register_ref_store(): new function
+ refs: remove some unnecessary handling of submodule == ""
+ refs: make some ref_store lookup functions private
+ refs: reorder some function definitions
Code and design clean-up for the refs API.
Will cook in 'next'.
The tip one is newer than the one posted to the list but was sent
privately by the author via his GitHub repository.
* jh/mingw-openssl-sha1 (2017-02-09) 1 commit
(merged to 'next' on 2017-02-10 at 084b3d8503)
+ mingw: use OpenSSL's SHA-1 routines
Windows port wants to use OpenSSL's implementation of SHA-1
routines, so let them.
Will cook in 'next'.
cf. <31bb0b9f-d498-24b3-57d5-9f34cb8e3914@kdbg.org>
* dt/gc-ignore-old-gc-logs (2017-02-13) 1 commit
(merged to 'next' on 2017-02-16 at 8f48e1b405)
+ gc: ignore old gc.log files
A "gc.log" file left by a backgrounded "gc --auto" disables further
automatic gc; it has been taught to run at least once a day (by
default) by ignoring a stale "gc.log" file that is too old.
Will cook in 'next'.
This is v6 posted on Feb 10th.
* js/git-path-in-subdir (2017-02-10) 2 commits
- rev-parse: fix several options when running in a subdirectory
- rev-parse tests: add tests executed from a subdirectory
The "--git-path", "--git-common-dir", and "--shared-index-path"
options of "git rev-parse" did not produce usable output. They are
now updated to show the path to the correct file, relative to where
the caller is.
Review comments sent; expecting an update.
cf. <cover.1486740772.git.johannes.schindelin@gmx.de>
* mh/ref-remove-empty-directory (2017-01-07) 23 commits
(merged to 'next' on 2017-02-10 at bcfd359e95)
+ files_transaction_commit(): clean up empty directories
+ try_remove_empty_parents(): teach to remove parents of reflogs, too
+ try_remove_empty_parents(): don't trash argument contents
+ try_remove_empty_parents(): rename parameter "name" -> "refname"
+ delete_ref_loose(): inline function
+ delete_ref_loose(): derive loose reference path from lock
+ log_ref_write_1(): inline function
+ log_ref_setup(): manage the name of the reflog file internally
+ log_ref_write_1(): don't depend on logfile argument
+ log_ref_setup(): pass the open file descriptor back to the caller
+ log_ref_setup(): improve robustness against races
+ log_ref_setup(): separate code for create vs non-create
+ log_ref_write(): inline function
+ rename_tmp_log(): improve error reporting
+ rename_tmp_log(): use raceproof_create_file()
+ lock_ref_sha1_basic(): use raceproof_create_file()
+ lock_ref_sha1_basic(): inline constant
+ raceproof_create_file(): new function
+ safe_create_leading_directories(): set errno on SCLD_EXISTS
+ safe_create_leading_directories_const(): preserve errno
+ t5505: use "for-each-ref" to test for the non-existence of references
+ refname_is_safe(): correct docstring
+ files_rename_ref(): tidy up whitespace
Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
once there no longer is any other branch whose name begins with
"foo/", but we didn't do so so far. Now we do.
Will cook in 'next'.
* cw/tag-reflog-message (2017-02-08) 1 commit
(merged to 'next' on 2017-02-10 at 3968b3a58b)
+ tag: generate useful reflog message
"git tag", because refs/tags/* doesn't keep reflog by default, did
not leave useful message when adding a new entry to reflog.
Will cook in 'next'.
* sg/completion (2017-02-13) 22 commits
(merged to 'next' on 2017-02-13 at 118c192874)
+ completion: restore removed line continuating backslash
(merged to 'next' on 2017-02-10 at 55b2785d89)
+ completion: cache the path to the repository
+ completion: extract repository discovery from __gitdir()
+ completion: don't guard git executions with __gitdir()
+ completion: consolidate silencing errors from git commands
+ completion: don't use __gitdir() for git commands
+ completion: respect 'git -C <path>'
+ rev-parse: add '--absolute-git-dir' option
+ completion: fix completion after 'git -C <path>'
+ completion: don't offer commands when 'git --opt' needs an argument
+ completion: list short refs from a remote given as a URL
+ completion: don't list 'HEAD' when trying refs completion outside of a repo
+ completion: list refs from remote when remote's name matches a directory
+ completion: respect 'git --git-dir=<path>' when listing remote refs
+ completion: fix most spots not respecting 'git --git-dir=<path>'
+ completion: ensure that the repository path given on the command line exists
+ completion tests: add tests for the __git_refs() helper function
+ completion tests: check __gitdir()'s output in the error cases
+ completion tests: consolidate getting path of current working directory
+ completion tests: make the $cur variable local to the test helper functions
+ completion tests: don't add test cruft to the test repository
+ completion: improve __git_refs()'s in-code documentation
(this branch is used by sg/completion-refs-speedup.)
Clean-up and updates to command line completion (in contrib/).
Will cook in 'next'.
* sg/completion-refs-speedup (2017-02-13) 13 commits
- squash! completion: fill COMPREPLY directly when completing refs
- completion: fill COMPREPLY directly when completing refs
- completion: list only matching symbolic and pseudorefs when completing refs
- completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
- completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
- completion: let 'for-each-ref' strip the remote name from remote branches
- completion: let 'for-each-ref' and 'ls-remote' filter matching refs
- completion: don't disambiguate short refs
- completion: don't disambiguate tags and branches
- completion: support excluding full refs
- completion: support completing full refs after '--option=refs/<TAB>'
- completion: wrap __git_refs() for better option parsing
- completion: remove redundant __gitcomp_nl() options from _git_commit()
(this branch uses sg/completion.)
The refs completion for large number of refs has been sped up,
partly by giving up disambiguating ambiguous refs and partly by
eliminating most of the shell processing between 'git for-each-ref'
and 'ls-remote' and Bash's completion facility.
Will hold.
* sk/parse-remote-cleanup (2017-02-06) 1 commit
(merged to 'next' on 2017-02-06 at 6ec89f72d5)
+ parse-remote: remove reference to unused op_prep
Code clean-up.
Undecided. There may be third-party scripts that are dot-sourcing
this one.
* jk/delta-chain-limit (2017-01-27) 2 commits
(merged to 'next' on 2017-02-06 at 9ff36ae9b2)
+ pack-objects: convert recursion to iteration in break_delta_chain()
+ pack-objects: enforce --depth limit in reused deltas
"git repack --depth=<n>" for a long time busted the specified depth
when reusing delta from existing packs. This has been corrected.
Will cook in 'next'.
* mm/merge-rename-delete-message (2017-01-30) 1 commit
(merged to 'next' on 2017-02-10 at 8bf8146029)
+ merge-recursive: make "CONFLICT (rename/delete)" message show both paths
When "git merge" detects a path that is renamed in one history
while the other history deleted (or modified) it, it now reports
both paths to help the user understand what is going on in the two
histories being merged.
Will cook in 'next'.
* ps/urlmatch-wildcard (2017-02-01) 5 commits
(merged to 'next' on 2017-02-10 at 2ed9ea48ee)
+ urlmatch: allow globbing for the URL host part
+ urlmatch: include host in urlmatch ranking
+ urlmatch: split host and port fields in `struct url_info`
+ urlmatch: enable normalization of URLs with globs
+ mailmap: add Patrick Steinhardt's work address
The <url> part in "http.<url>.<variable>" configuration variable
can now be spelled with '*' that serves as wildcard.
E.g. "http.https://*.example.com.proxy" can be used to specify the
proxy used for https://a.example.com, https://b.example.com, etc.,
i.e. any host in the example.com domain.
Will cook in 'next'.
* sf/putty-w-args (2017-02-10) 5 commits
(merged to 'next' on 2017-02-14 at 7f157e7020)
+ connect.c: stop conflating ssh command names and overrides
+ connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
+ git_connect(): factor out SSH variant handling
+ connect: rename tortoiseplink and putty variables
+ connect: handle putty/plink also in GIT_SSH_COMMAND
The command line options for ssh invocation needs to be tweaked for
some implementations of SSH (e.g. PuTTY plink wants "-P <port>"
while OpenSSH wants "-p <port>" to specify port to connect to), and
the variant was guessed when GIT_SSH environment variable is used
to specify it. The logic to guess now applies to the command
specified by the newer GIT_SSH_COMMAND and also core.sshcommand
configuration variable, and comes with an escape hatch for users to
deal with misdetected cases.
Will cook in 'next'.
* jk/describe-omit-some-refs (2017-01-23) 5 commits
(merged to 'next' on 2017-01-23 at f8a14b4996)
+ describe: teach describe negative pattern matches
+ describe: teach --match to accept multiple patterns
+ name-rev: add support to exclude refs by pattern match
+ name-rev: extend --refs to accept multiple patterns
+ doc: add documentation for OPT_STRING_LIST
"git describe" and "git name-rev" have been taught to take more
than one refname patterns to restrict the set of refs to base their
naming output on, and also learned to take negative patterns to
name refs not to be used for naming via their "--exclude" option.
Will cook in 'next'.
* bw/attr (2017-02-01) 27 commits
(merged to 'next' on 2017-02-14 at d35c1d7e4a)
+ attr: reformat git_attr_set_direction() function
+ attr: push the bare repo check into read_attr()
+ attr: store attribute stack in attr_check structure
+ attr: tighten const correctness with git_attr and match_attr
+ attr: remove maybe-real, maybe-macro from git_attr
+ attr: eliminate global check_all_attr array
+ attr: use hashmap for attribute dictionary
+ attr: change validity check for attribute names to use positive logic
+ attr: pass struct attr_check to collect_some_attrs
+ attr: retire git_check_attrs() API
+ attr: convert git_check_attrs() callers to use the new API
+ attr: convert git_all_attrs() to use "struct attr_check"
+ attr: (re)introduce git_check_attr() and struct attr_check
+ attr: rename function and struct related to checking attributes
+ attr.c: outline the future plans by heavily commenting
+ Documentation: fix a typo
+ attr.c: add push_stack() helper
+ attr: support quoting pathname patterns in C style
+ attr.c: plug small leak in parse_attr_line()
+ attr.c: tighten constness around "git_attr" structure
+ attr.c: simplify macroexpand_one()
+ attr.c: mark where #if DEBUG ends more clearly
+ attr.c: complete a sentence in a comment
+ attr.c: explain the lack of attr-name syntax check in parse_attr()
+ attr.c: update a stale comment on "struct match_attr"
+ attr.c: use strchrnul() to scan for one line
+ commit.c: use strchrnul() to scan for one line
The gitattributes machinery is being taught to work better in a
multi-threaded environment.
Will cook in 'next'.
* nd/worktree-move (2017-01-27) 7 commits
. fixup! worktree move: new command
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
"git worktree" learned move and remove subcommands.
Tentatively ejected as it seems to break 'pu' when merged.
* cc/split-index-config (2016-12-26) 21 commits
- Documentation/git-update-index: explain splitIndex.*
- Documentation/config: add splitIndex.sharedIndexExpire
- read-cache: use freshen_shared_index() in read_index_from()
- read-cache: refactor read_index_from()
- t1700: test shared index file expiration
- read-cache: unlink old sharedindex files
- config: add git_config_get_expiry() from gc.c
- read-cache: touch shared index files when used
- sha1_file: make check_and_freshen_file() non static
- Documentation/config: add splitIndex.maxPercentChange
- t1700: add tests for splitIndex.maxPercentChange
- read-cache: regenerate shared index if necessary
- config: add git_config_get_max_percent_split_change()
- Documentation/git-update-index: talk about core.splitIndex config var
- Documentation/config: add information for core.splitIndex
- t1700: add tests for core.splitIndex
- update-index: warn in case of split-index incoherency
- read-cache: add and then use tweak_split_index()
- split-index: add {add,remove}_split_index() functions
- config: add git_config_get_split_index()
- config: mark an error message up for translation
The experimental "split index" feature has gained a few
configuration variables to make it easier to use.
Waiting for review comments to be addressed.
cf. <20161226102222.17150-1-chriscool@tuxfamily.org>
cf. <a1a44640-ff6c-2294-72ac-46322eff8505@ramsayjones.plus.com>
* kn/ref-filter-branch-list (2017-02-07) 21 commits
(merged to 'next' on 2017-02-10 at 794bb8284d)
+ ref-filter: resurrect "strip" as a synonym to "lstrip"
(merged to 'next' on 2017-01-31 at e7592a5461)
+ branch: implement '--format' option
+ branch: use ref-filter printing APIs
+ branch, tag: use porcelain output
+ ref-filter: allow porcelain to translate messages in the output
+ ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
+ ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
+ ref-filter: Do not abruptly die when using the 'lstrip=<N>' option
+ ref-filter: rename the 'strip' option to 'lstrip'
+ ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
+ ref-filter: introduce refname_atom_parser()
+ ref-filter: introduce refname_atom_parser_internal()
+ ref-filter: make "%(symref)" atom work with the ':short' modifier
+ ref-filter: add support for %(upstream:track,nobracket)
+ ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
+ ref-filter: introduce format_ref_array_item()
+ ref-filter: move get_head_description() from branch.c
+ ref-filter: modify "%(objectname:short)" to take length
+ ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
+ ref-filter: include reference to 'used_atom' within 'atom_value'
+ ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Will cook in 'next'.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-12-05 at 0c77e39cd5)
+ setup_git_env: avoid blind fall-back to ".git"
Originally merged to 'next' on 2016-10-26
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-12-05 at 041946dae0)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Originally merged to 'next' on 2016-10-11
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
--------------------------------------------------
[Discarded]
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
. exclude: do not respect symlinks for in-tree .gitignore
. attr: do not respect symlinks for in-tree .gitattributes
. exclude: convert "check_index" into a flags field
. attr: convert "macro_ok" into a flags field
. add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
* sb/push-make-submodule-check-the-default (2017-01-26) 2 commits
(merged to 'next' on 2017-01-26 at 5f4715cea6)
+ Revert "push: change submodule default to check when submodules exist"
(merged to 'next' on 2016-12-12 at 1863e05af5)
+ push: change submodule default to check when submodules exist
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Retracted.
^ permalink raw reply
* Re: There are too many unreachable loose objects
From: Jeff King @ 2017-02-16 22:57 UTC (permalink / raw)
To: Jacob Keller; +Cc: Hilco Wijbenga, Git Users
In-Reply-To: <CA+P7+xqWoUBOoFSiOTT5U-9aoqESUMnZeSGfvhGte2LqF18gmw@mail.gmail.com>
On Thu, Feb 16, 2017 at 02:36:10PM -0800, Jacob Keller wrote:
> > Whenever I run "git push --force(-with-lease)" I get a variation of
> >
> > Counting objects: 187, done.
> > Delta compression using up to 12 threads.
> > Compressing objects: 100% (126/126), done.
> > Writing objects: 100% (187/187), 21.35 KiB | 0 bytes/s, done.
> > Total 187 (delta 78), reused 71 (delta 20)
> > warning: There are too many unreachable loose objects; run 'git prune'
> > to remove them.
> > To git@git.company.com:project.git
> > + 51338ea...b0ebe39 my-branch -> my-branch (forced update)
> >
> > So I'll run "git prune" and, for good measure, "git gc" (which even
> > includes "git prune"?). The first seems to do nothing, the latter does
> > its thing.
> >
>
> It may be that it's the server side that needs to git-prune, and not
> your local side? I'm not really certain but you're doing a push which
> talks to a remote server.
Yes, certainly the position in the output implies that. These days you
should see:
remote: warning: There are too many...
to make it more clear. Perhaps the server is too old to have 860a2ebec
(receive-pack: send auto-gc output over sideband 2, 2016-06-05).
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox