* Re: [PATCH v4] tag: generate useful reflog message
From: Cornelius Weig @ 2017-02-08 22:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, karthik.188, peff, bitte.keine.werbung.einwerfen
In-Reply-To: <xmqqshnov0c4.fsf@gitster.mtv.corp.google.com>
On 02/08/2017 10:28 PM, Junio C Hamano wrote:
> cornelius.weig@tngtech.com writes:
>
>> From: Cornelius Weig <cornelius.weig@tngtech.com>
>>
>> When tags are created with `--create-reflog` or with the option
>> `core.logAllRefUpdates` set to 'always', a reflog is created for them.
>> So far, the description of reflog entries for tags was empty, making the
>> reflog hard to understand. For example:
>> 6e3a7b3 refs/tags/test@{0}:
>>
>> Now, a reflog message is generated when creating a tag, following the
>> pattern "tag: tagging <short-sha1> (<description>)". If
>> GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
>> (<description>)" instead. If the tag references a commit object, the
>> description is set to the subject line of the commit, followed by its
>> commit date. For example:
>> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
>>
>> If the tag points to a tree/blob/tag objects, the following static
>> strings are taken as description:
>>
>> - "tree object"
>> - "blob object"
>> - "other tag object"
>>
>> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
>> Reviewed-by: Junio C Hamano <gitster@pobox.com>
>
> This last line is inappropriate, as I didn't review _THIS_ version,
> which is different from the previous one, and I haven't checked if
> the way the comments on the previous review were addressed in this
> version is agreeable.
Sorry for that confusion. I'm still not used to when adding what
sign-off is appropriate. I thought that adding you as reviewer is also a
question of courtesy.
A version with revised tests will follow.
^ permalink raw reply
* Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
From: Brandon Williams @ 2017-02-08 22:35 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702072113040.25002@i7.lan>
On 02/07, Linus Torvalds wrote:
>
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Tue, 7 Feb 2017 21:05:28 -0800
> Subject: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
>
> The choice of '!' for a negative pathspec ends up not only not matching
> what we do for revisions, it's also a horrible character for shell
> expansion since it needs quoting.
>
> So add '^' as an alternative alias for an excluding pathspec entry.
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
> pathspec.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/pathspec.c b/pathspec.c
> index 7ababb315..ecad03406 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -224,6 +224,12 @@ static const char *parse_short_magic(unsigned *magic, const char *elem)
> char ch = *pos;
> int i;
>
> + /* Special case alias for '!' */
> + if (ch == '^') {
> + *magic |= PATHSPEC_EXCLUDE;
> + continue;
> + }
> +
> if (!is_pathspec_magic(ch))
> break;
I like adding '^' to be an alias for excluding patterns. There have
been numerous times where I have wanted to use exclude patterns and
forgotten that I've needed to do some escape magic to get my shell to
leave '!' alone.
The only issue I see with doing this is that if a user supplies an
exclude pattern for a command which doesn't support exclude pathspec
magic the unsupported_magic() function will have slightly cryptic
output.
git cmd -- :^dir
would produce some output which says:
':^dir': pathspec magic not supported by this command: 'exclude' (mnemonic: '!')
And the user may scratch their head for a second since they didn't
supply the '!' character, but rather '^'. That being said I think it
should be fine since the long name of the magic is also printed so the
user should be able to figure out what's wrong. I also don't think
there are any users of pathspecs which disallow exclude magic so this
may not even be an issue.
--
Brandon Williams
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Junio C Hamano @ 2017-02-08 21:11 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <xmqqy3xgwpiq.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> If you know offhand which callers pass neither of the two
> PATHSPEC_PREFER_* bits and remember for what purpose you allowed
> them to do so, please remind me. I'll keep digging in the meantime.
Answering my own questions, here are my findings so far and a
tentative conclusion.
With or without the patch in this thread, parse_pathspec() behaves
the same way for either CWD or FULL if you feed a non-empty
pathspec with at least one positive element. IOW, if a caller feeds
a non-empty pathspec and there is no "negative" element involved, it
does not matter if we feed CWD or FULL.
There are only a handful of callers that pass neither preference
bits to parse_pathspec(). Here are my observations on them that
tells me that most of them are OK if we change them to prefer
either CWD or FULL:
- archive.c::path_exists() feeds a pathspec with a single element
to see if read_tree_recursive() finds any matching paths, to
allow its caller to iterate over the original pathspec and see
if there is a typo (i.e. an element that matches nothing). It
should prefer FULL to match what parse_pathspec_arg(), its
caller, uses.
The caller probably should refrain from passing ones with
negative magic. I.e. "git archive -- t :\!t/perf" errors out
because checking each element independently in the loop means
that ":\!t/perf" is checked alone, triggering "there is nothing
to exclude from".
- blame.c::find_origin() feeds a pathspec with a single element,
which is a path in the history and does so as a literal, hence
no room for "negative" to kick in.
- builtin/check-ignore.c::check_ignore(), when argc==0, does not
call parse_pathspec(). It does not take any magic other than
FROMTOP, so "negative" won't come into the picture.
- builtin/checkout.c::cmd_checkout(), when argc==0, does not call
parse_pathspec(). This codepath will get affected by Linus's
change ("cd t && git checkout :\!perf" would try to check out
everything except t/perf, but what is a reasonable definition of
"everything" in the context of this command). We need to
decide, and I am leaning towards preferring CWD for this case.
- revision.c::setup_revisions() calls parse_pathspec() only when
the caller gave a non-empty pathspec. This pathspec is used for
pruning log traversal (e.g. "only show commits that touch these
paths") and is affected by Linus's change. It should favor
FULL.
- tree-diff.c::try_to_follow_renames() feeds a pathspec with a
single element as a literal, hence no room for "negative" to
kick in.
So, I am tempted to suggest us doing the following:
* Leave a NEEDSWORK comment to archive.c::path_exists() that is
used for checking the validation of pathspec elements. To fix it
properly, we need to be able to skip a negative pathspec to be
passed to this function by the caller, and to do so, we need to
expose a helper from the pathspec API that gets a single string
and returns what magic it has, but that is of lower priority.
* Retire the PATHSPEC_PREFER_CWD bit and replace its use with the
lack of the PATHSPEC_PREFER_FULL bit.
* Keep most of the above callsites that currently do not pass
CWD/FULL as they are, except the ones that should take FULL (see
above).
Comments?
^ permalink raw reply
* Re: Bug with fixup and autosquash
From: Junio C Hamano @ 2017-02-08 22:55 UTC (permalink / raw)
To: Ashutosh Bapat
Cc: git, Johannes Schindelin, Michael Haggerty, Michael J Gruber,
Matthieu Moy
In-Reply-To: <CAFjFpRe8zqxs4OLbCrjnuEzF=75sbBJ+HuZqek49B=O=TFHq8A@mail.gmail.com>
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> I have been using git rebase heavily these days and seem to have found a bug.
>
> If there are two commit messages which have same prefix e.g.
> yyyyyy This is prefix
> xxxxxx This is prefix and message
>
> xxxxxx comitted before yyyyyy
>
> Now I commit a fixup to yyyyyy using git commit --fixup yyyyyy
> zzzzzz fixup! This is prefix
>
> When I run git rebase -i --autosquash, the script it shows me looks like
> pick xxxxxx This is prefix and message
> fixup zzzzzz fixup! This is prefix
> pick yyyyyy This is prefix
>
> I think the correct order is
> pick xxxxxx This is prefix and message
> pick yyyyyy This is prefix
> fixup zzzzzz fixup! This is prefix
>
> Is that right?
Because "commit" pretends as if it took the exact commit object name
to be fixed up (after all, it accepts "yyyyyy" that is a name of the
commit object), it would be nice if the fixup is applied to that
exact commit, even if you had many commits that share exactly the
same title (i.e. not just shared prefix).
Unfortunately, "rebase -i --autosquash" reorders the entries by
identifying the commit by its title, and it goes with prefix match
so that fix-up commits created without using --fixup option but
manually records the title's prefix substring can also work.
We could argue that the logic should notice that there is one exact
match and another non-exact prefix match and favor the former, and
certainly such a change would make your made-up example (you didn't
actually have a commit whose title is literally "This is prefix")
above work better.
But I am not sure if adding such heuristics would really help in
general. It would not help those whose commits are mostly titled
ultra-vaguely, like "fix", "bugfix", "docfix", etc.
Another possibility is to teach "commit --fixup" to cast exact
commit object name in stone. That certainly would solve your
immediate problem, but it has a grave negative impact when the user
rebases the same topic many times (which happens often).
For example, I may have a series of commits A and B, notice that A
could be done a bit better and have "fixup A" on top, build a new
commit C on it, and then realize that the next step (i.e. D) would
need support from a newer codebase than where I started (i.e. A^).
At that point, I would have a 4-commit series (A, B, "fixup A", and
C), and I would rebase them on top of something newer. I am
undecided if that "fixup A" is really an improvement or unnecessary,
when I do this, but I do know that I want to build the series on top
of a different commit. So I do rebase without --autosquash (I would
probably rebase without --interactive for this one).
Then I keep working and add a new commit D on top. After all that,
I would have a more-or-less completed series and would be ready to
re-assess the whole series. I perhaps decide that "fixup A" is
really an improvement. And then I would "rebase -i" to squash the
fix-up into A.
But notice that at this point, what we are calling A has different
object name than the original A the fixup was written for, because
we rebased once on top of a newer codebase. That commit can still
be identified by its title, but not with its original commit object
name.
I think that is why "commit --fixup <commit>" turns the commit
object name (your "yyyyyy") into a string (your "This is prefix")
and that is a reasonable design decision [*1*].
So from that point of view, if we were to address your issue, it
should happen in "rebase -i --autosquash" side, not "commit --fixup"
side.
Let's hear from some of those (Cc'ed) who were involved in an
earlier --autosquash thread.
https://public-inbox.org/git/cover.1259934977.git.mhagger@alum.mit.edu/
[Footnote]
*1* "rebase -i --autosquash" does understand "fixup! yyyyyy", so if
you are willing to accept the consequence of not being able to
rebase twice, you could instead do
$ git commit -m "fixup! yyyyyy"
I would think.
^ permalink raw reply
* Re: "disabling bitmap writing, as some objects are not being packed"?
From: Jeff King @ 2017-02-08 23:00 UTC (permalink / raw)
To: David Turner; +Cc: Junio C Hamano, Duy Nguyen, Git Mailing List
In-Reply-To: <1486592043.1938.82.camel@novalis.org>
On Wed, Feb 08, 2017 at 05:14:03PM -0500, David Turner wrote:
> > I wonder if you'd want to either bump the auto-gc object limit, or
> > possibly reduce the gc.pruneExpire limit to keep this situation from
> > coming up in the first place (or at least mitigating the amount of time
> > it's the case).
>
> Auto-gc might not succeed in pruning objects, but it will at least
> reduce the number of packs, which is super-important for performance.
Right, I mean to bump the loose-object limit but keep the
gc.autoPackLimit at 50. If you couple that with setting
transfer.unpackLimit, then each push creates a single pack, and you
repack after 50 pushes.
You don't have to care about loose objects, because you know you only
get them when a "gc" ejects loose objects (so they're not as efficient,
but nothing actually accesses them; they just hang around until their
mtime grace period is up).
> I think the intent of automatic gc is to have a git repository be
> relatively low-maintenance from a server-operator perspective. (Side
> note: it's fairly trivial for a user with push access to mess with the
> check simply by pushing a bunch of objects whose shas start with 17).
> It seems odd that git gets itself into a state where it refuses to do
> any maintenance just because at some point some piece of the maintenance
> didn't make progress.
In my experience, auto-gc has never been a low-maintenance operation on
the server side (and I do think it was primarily designed with clients
in mind).
At GitHub we disable it entirely, and do our own gc based on a throttled
job queue (one reason to throttle is that repacking is memory and I/O
intensive, so you really don't want to a bunch of repacks kicking off
all at once). So basically any repo that gets pushed to goes on the
queue, and then we pick the worst cases from the queue based on how
badly they need packing[1].
I wish regular Git were more turn-key in that respect. Maybe it is for
smaller sites, but we certainly didn't find it so. And I don't know that
it's feasible to really share the solution. It's entangled with our
database (to store last-pushed and last-maintenance values for repos)
and our job scheduler.
[1] The "how bad" thing is a heuristic, and we found it's generally
proportional to the number of bytes stored in objects _outside_ of
the big "main" pack. So 2 big pushes may need maintenance more
than 10 tiny pushes, because they have more objects (and our goal
with maintenance isn't just saving disk space or avoiding the linear
pack search, but having up-to-date bitmaps and good on-disk deltas
to make serving fetches as cheap as possible).
> Sure, I could change my configuration, but that doesn't help the other
> folks (e.g. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813084 )
> who run into this.
Yeah, I'm certainly open to improving Git's defaults. If it's not clear
from the above, I mostly just gave up for a site the size of GitHub. :)
> Idea 1: when gc --auto would issue this message, instead it could create
> a file named gc.too-much-garbage (instead of gc.log), with this message.
> If that file exists, and it is less than one day (?) old, then we don't
> attempt to do a full gc; instead we just run git repack -A -d. (If it's
> more than one day old, we just delete it and continue anyway).
I kind of wonder if this should apply to _any_ error. I.e., just check
the mtime of gc.log and forcibly remove it when it's older than a day.
You never want to get into a state that will fail to resolve itself
eventually. That might still happen (e.g., corrupt repo), but at the
very least it won't be because Git is too dumb to try again.
> Idea 2 : Like idea 1, but instead of repacking, just smash the existing
> packs together into one big pack. In other words, don't consider
> dangling objects, or recompute deltas. Twitter has a tool called "git
> combine-pack" that does this:
> https://github.com/dturner-tw/git/blob/dturner/journal/builtin/combine-pack.c
We wrote something similar at GitHub, too, but we never ended up using
it in production. We found that with a sane scheduler, it's not too big
a deal to just do maintenance once in a while.
Also, our original problem was that repos which have gotten out of
hand (say, 5000 packs) repacked _very_ slowly with a normal repack. So
a "fast pack" followed by a real pack was a viable way out of that. In
the end, I just made pack-objects handle this case better, and we
don't need the fast-pack.
> That's less space-efficient than a true repack, but it's no worse than
> having the packs separate, and it's a win for read performance because
> there's no need to do a linear search over N packs to find an object.
Over the long term you may end up with worse packs, because the true
repack will drop some delta opportunities between objects in the same
pack (reasoning that they weren't made into deltas last time, so it's
not worth trying again). You'd probably need to use "-f" periodically.
This is all speculation, though. We never did it in production, so I was
never able to measure the real impact over time.
> Idea 3: As I suggested last time, separate fatal and non-fatal errors.
> If gc fails because of EIO or something, we probably don't want to touch
> the disk anymore. But here, the worst consequence is that we waste some
> processing power. And it's better to occasionally waste processing power
> in a non-interactive setting than it is to do so when a user will be
> blocked on it. So non-fatal warnings should go to gc.log, and fatal
> errors should go to gc.fatal. gc.log won't block gc from running. I
> think this is my preferred option.
This seems like your (1), except that it handles more than one type of
non-fatal error. So I like it much better.
I'm still not sure if it's worth making the fatal/non-fatal distinction.
Doing so is perhaps safer, but it does mean that somebody has to decide
which errors are important enough to block a retry totally, and which
are not. In theory, it would be safe to always _try_ and then the gc
process can decide when something is broken and abort. And all you've
wasted is some processing power each day.
-Peff
^ permalink raw reply
* Re: [PATCH] push options: fail properly in the stateless case
From: Junio C Hamano @ 2017-02-08 23:01 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jonathan Nieder
In-Reply-To: <CAGZ79kaszczpnu+VEV3y+61qgji-0+vRFVLk8Q1+BbSDgiHz1Q@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>>> +'option push-option <c-string>::
>>> + Transmit this push option.
>>> +
>>
>> There is no "c-string" in the current documentation used or
>> defined. The closest thing I found is
>>
>> ... that field will be quoted in the manner of a C string ...
>>
>> in git-status page, but I do not think you send the value for an
>> push-option after running quote_c_style(), so I am puzzled.
>
> When implementing push options, we discussed that and according to
> Documentation/git-push:
>
> The given string must not contain a NUL or LF character.
OK, so "Transmit <string> as a push option" is sufficient, as the
string is sent as-is. OK.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] rev-list-options.txt: update --all about HEAD
From: Jeff King @ 2017-02-08 23:01 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <20170208060641.13793-1-pclouds@gmail.com>
On Wed, Feb 08, 2017 at 01:06:41PM +0700, Nguyễn Thái Ngọc Duy wrote:
> This is the document patch for f0298cf1c6 (revision walker: include a
> detached HEAD in --all - 2009-01-16).
>
> Even though that commit is about detached HEAD, as Jeff pointed out,
> always adding HEAD in that case may have subtle differences with
> --source or --exclude. So the document mentions nothing about the
> detached-ness.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> v2 drops "detached".
Thanks. Seems like an obvious improvement to me.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
From: Junio C Hamano @ 2017-02-08 23:05 UTC (permalink / raw)
To: Brandon Williams; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <20170208223532.GB108686@google.com>
Brandon Williams <bmwill@google.com> writes:
> git cmd -- :^dir
>
> would produce some output which says:
> ':^dir': pathspec magic not supported by this command: 'exclude' (mnemonic: '!')
>
> And the user may scratch their head for a second since they didn't
> supply the '!' character, but rather '^'.
Yup, I am tempted to tweak Cornelius's glossary fixup and squash
this into the series, for two purposes.
- it makes it clear that '^' and '!' mean the same thing (and
clearer than Cornelius's original, "! or ^", which could leave
the reader wondering "ok there are two ways to say negative; do
they subtly mean different things?").
- it hints that '!' is the more official spelling, making the
output you showed above acceptable.
diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 8ad29e61a9..822ca83264 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -386,8 +386,8 @@ Glob magic is incompatible with literal magic.
exclude;;
After a path matches any non-exclude pathspec, it will be run
- through all exclude pathspec (magic signature: `!`). If it
- matches, the path is ignored.
+ through all exclude pathspec (magic signature: `!` or its
+ synonym `^`). If it matches, the path is ignored.
--
[[def_parent]]parent::
^ permalink raw reply related
* Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree
From: Junio C Hamano @ 2017-02-08 23:19 UTC (permalink / raw)
To: Stefan Beller
Cc: Nguyễn Thái Ngọc Duy, git@vger.kernel.org,
Michael Haggerty
In-Reply-To: <CAGZ79kZk1Chq1R-anz0RC+0GDEubGm5kEQOsy5bz9zwCYEi9nw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Wed, Feb 8, 2017 at 3:31 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> (**) At this point, we may want to rename refs *_submodule API to
>> something more neutral, maybe s/_submodule/_remote/
>
> I agree on (**), except that I am not sure if /_remote/ is a better name,
> because there is already a concept of a "remote" as well as
> "remote-tracking" in Git. (Usually it is not reachable on the same
> FS, but resides on another machine).
I agree with you that the concept of remote is quite detached from
the concept of wt and submodule whose refs need to be peeked at from
the local repository. After all, "remote" tracking branches are
part of local repository's refs.
> My gut reaction would be to s/submodule/alternative/ here,
> but we also have a thing called alternates already.
... and I tend to think that is far closer a concept. You borrow
objects from your alternate object store, and that alternate object
store may have its own set of refs you would need to peek when you
are computing reachability from refs.
Also don't we already enumerate such refs that pin objects in the
alternate object store when doing object transfer negotiation in
order to send ".have" entries for their tips? What API do we use to
do that, I wonder.
^ permalink raw reply
* Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree
From: Junio C Hamano @ 2017-02-08 23:25 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Michael Haggerty
In-Reply-To: <20170208113144.8201-3-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> As such, we can just linked worktree's path as a submodule. We just need
Lack of verb made me read this three times. "We can just treat
linked worktree's path as if it were a submodule"?
I agree with you that the "submodule" in the name is misleading. We
would want to be able to walk refs from other local repositories
(and repository-like entities, like the .git/ thing in a liked
worktree), like our own submodule, or the repository we borrow
objects from via the alternate mechanism.
^ permalink raw reply
* [PATCH v5] tag: generate useful reflog message
From: cornelius.weig @ 2017-02-08 22:41 UTC (permalink / raw)
To: git; +Cc: Cornelius Weig, gitster, bitte.keine.werbung.einwerfen
In-Reply-To: <xmqqshnov0c4.fsf@gitster.mtv.corp.google.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:
Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging <short-sha1> (<description>)". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
(<description>)" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:
- "tree object"
- "blob object"
- "other tag object"
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
Notes:
Interdiff v4..v5
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 894959f..1a3230f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,9 +80,10 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
test_must_fail git reflog exists refs/tags/mytag
'
-git log -1 > expected \
- --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
test_expect_success 'creating a tag with --create-reflog should create reflog' '
+ git log -1 \
+ --format="format:tag: tagging %h (%s, %cd)%n" \
+ --date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
@@ -90,9 +91,10 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' '
test_cmp expected actual
'
-git log -1 > expected \
- --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
test_expect_success 'annotated tag with --create-reflog has correct message' '
+ git log -1 \
+ --format="format:tag: tagging %h (%s, %cd)%n" \
+ --date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag -m "annotated tag" --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
builtin/tag.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
t/t7004-tag.sh | 18 +++++++++++++++++-
2 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..bca890f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,
}
}
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+ enum object_type type;
+ struct commit *c;
+ char *buf;
+ unsigned long size;
+ int subject_len = 0;
+ const char *subject_start;
+
+ char *rla = getenv("GIT_REFLOG_ACTION");
+ if (rla) {
+ strbuf_addstr(sb, rla);
+ } else {
+ strbuf_addstr(sb, _("tag: tagging "));
+ strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+ }
+
+ strbuf_addstr(sb, " (");
+ type = sha1_object_info(sha1, NULL);
+ switch (type) {
+ default:
+ strbuf_addstr(sb, _("object of unknown type"));
+ break;
+ case OBJ_COMMIT:
+ if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, sb->len, subject_start, subject_len);
+ } else {
+ strbuf_addstr(sb, _("commit object"));
+ }
+ free(buf);
+
+ if ((c = lookup_commit_reference(sha1)) != NULL)
+ strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
+ break;
+ case OBJ_TREE:
+ strbuf_addstr(sb, _("tree object"));
+ break;
+ case OBJ_BLOB:
+ strbuf_addstr(sb, _("blob object"));
+ break;
+ case OBJ_TAG:
+ strbuf_addstr(sb, _("other tag object"));
+ break;
+ }
+ strbuf_addch(sb, ')');
+}
+
struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
+ struct strbuf reflog_msg = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else
die(_("Invalid cleanup mode %s"), cleanup_arg);
+ create_reflog_msg(object, &reflog_msg);
+
if (create_tag_object) {
if (force_sign_annotate && !annotate)
opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
- NULL, &err) ||
+ reflog_msg.buf, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
strbuf_release(&err);
strbuf_release(&buf);
strbuf_release(&ref);
+ strbuf_release(&reflog_msg);
return 0;
}
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..1a3230f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -81,9 +81,25 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
'
test_expect_success 'creating a tag with --create-reflog should create reflog' '
+ git log -1 \
+ --format="format:tag: tagging %h (%s, %cd)%n" \
+ --date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag --create-reflog tag_with_reflog &&
- git reflog exists refs/tags/tag_with_reflog
+ git reflog exists refs/tags/tag_with_reflog &&
+ sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog > actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'annotated tag with --create-reflog has correct message' '
+ git log -1 \
+ --format="format:tag: tagging %h (%s, %cd)%n" \
+ --date=format:%Y-%m-%d >expected &&
+ test_when_finished "git tag -d tag_with_reflog" &&
+ git tag -m "annotated tag" --create-reflog tag_with_reflog &&
+ git reflog exists refs/tags/tag_with_reflog &&
+ sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog > actual &&
+ test_cmp expected actual
'
test_expect_success '--create-reflog does not create reflog on failure' '
--
2.10.2
^ permalink raw reply related
* Re: Automatically Add .gitignore Files
From: Junio C Hamano @ 2017-02-08 23:39 UTC (permalink / raw)
To: Thangalin; +Cc: git
In-Reply-To: <CAANrE7rmUZcJkw+thMczv3D=7sqcUHBsorzvEZgYg=6AEfrU=w@mail.gmail.com>
Thangalin <thangalin@gmail.com> writes:
> I frequently forget to add .gitignore files when creating new .gitignore files.
>
> I'd like to request a command-line option to always add .gitignore
> files (or, more generally, always add files that match a given file
> specification).
That is essentially what "Untracked files" listing in the editor
buffer given to you when you type "git commit" is about. By not
saying you ignore .gitattributes, .gitignore, .gitmodules, etc., you
are reminded if you forgot to add them. "git status" does the same,
and perhaps you want to make it a habit to run it before committing.
I am tempted to say that there should be a way to somehow forbid use
of the "-m" option to "git commit" by default, until the user gains
more familiarity with use of Git. That way, they will learn to pay
more attention to the "Untracked" and "Changes not staged" sections
;-)
^ permalink raw reply
* Re: Trying to use xfuncname without success.
From: Jack Adrian Zappa @ 2017-02-09 0:00 UTC (permalink / raw)
Cc: git-mailing-list
In-Reply-To: <CAKepmahcaV1Ro_wObkFxezrZntL6JQ_ft+06_3ZSrxEz-yNjzA@mail.gmail.com>
Well, it mostly works, but I'm getting some weirdness where it has
grabbed a line at 126 and is using that for the hunk header. Strange
thing is, I move the file to another repository, commit it, make a
change to the fileand do a diff, and it gets the correct hunk header.
I'm at a loss. :(
On Wed, Feb 8, 2017 at 4:12 PM, Jack Adrian Zappa <adrianh.bsc@gmail.com> wrote:
> That was it. I have a .gitattributes file in my home directory.
> Ahhh, but it's not in my %userprofile% directory, but in my ~
> directory.
>
> A bit confusing having 2 home directories. I made a link to my
> .gitconfig, but forgot to make a link to my .gitattributes.
>
> Thanks.
>
>
> A
>
> On Wed, Feb 8, 2017 at 4:05 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>> Double check .gitattributes?
>>
>> On Feb 8, 2017 2:58 PM, "Jack Adrian Zappa" <adrianh.bsc@gmail.com> wrote:
>>>
>>> Thanks Samuel,
>>>
>>> That example showed that there must be something wrong in my .git
>>> directory, because with it, I'm getting the correct output. Moving
>>> the same lines to my .git/config didn't work.
>>>
>>> On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>> > I just put this togther: https://github.com/sxlijin/xfuncname-test
>>> >
>>> > Try cloning and then for any of config1 thru 3,
>>> >
>>> > $ cp configX .git/config
>>> > $ git diff HEAD^ -- test.natvis
>>> >
>>> > On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa
>>> > <adrianh.bsc@gmail.com> wrote:
>>> >> Thanks Samuel,
>>> >>
>>> >> So, the question is, what is causing this problem on my system?
>>> >>
>>> >> Anyone have an idea to help diagnose this problem?
>>> >>
>>> >> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>> >>> On Windows 7, it works for me in both CMD and Git Bash:
>>> >>>
>>> >>> $ git --version
>>> >>> git version 2.11.0.windows.3
>>> >>>
>>> >>> $ git diff HEAD^ --word-diff
>>> >>> diff --git a/test.natvis b/test.natvis
>>> >>> index 93396ad..1233b8c 100644
>>> >>> --- a/test.natvis
>>> >>> +++ b/test.natvis
>>> >>> @@ -18,6 +18,7 @@ test
>>> >>>
>>> >>>
>>> >>> <!-- Non-blank line -->
>>> >>> {+<Item Name="added var">added_var</Item>+}
>>> >>>
>>> >>>
>>> >>> <Item Name="var2">var2</Item>
>>> >>>
>>> >>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe <l.s.r@web.de> wrote:
>>> >>>> Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>>> >>>>> Thanks Rene, but you seem to have missed the point. NOTHING is
>>> >>>>> working. No matter what I put there, it doesn't seem to get
>>> >>>>> matched.
>>> >>>>
>>> >>>> I'm not so sure about that. With your example I get this diff
>>> >>>> without
>>> >>>> setting diff.natvis.xfuncname:
>>> >>>>
>>> >>>> diff --git a/a.natvis b/a.natvis
>>> >>>> index 7f9bdf5..bc3c090 100644
>>> >>>> --- a/a.natvis
>>> >>>> +++ b/a.natvis
>>> >>>> @@ -19,7 +19,7 @@
>>> >>>> xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
>>> >>>>
>>> >>>>
>>> >>>> <!-- Non-blank line -->
>>> >>>> - <Item Name="added var">added_var</Item>
>>> >>>> + <Item Name="added var">added_vars</Item>
>>> >>>>
>>> >>>>
>>> >>>> <Item Name="var2">var2</Item>
>>> >>>>
>>> >>>> Note the XML namespace in the hunk header. It's put there by the
>>> >>>> default rule because "xmlns" starts at the beginning of the line.
>>> >>>> Your
>>> >>>> diff has nothing there, which means the default rule is not used,
>>> >>>> i.e.
>>> >>>> your user-defined rule is in effect.
>>> >>>>
>>> >>>> Come to think of it, this line break in the middle of the
>>> >>>> AutoVisualizer
>>> >>>> tab might have been added by your email client unintentionally, so
>>> >>>> that
>>> >>>> we use different test files, which then of course results in
>>> >>>> different
>>> >>>> diffs. Is that the case?
>>> >>>>
>>> >>>> Anyway, if I run the following two commands:
>>> >>>>
>>> >>>> $ git config diff.natvis.xfuncname "^[\t ]*<Type[\t
>>> >>>> ]+Name=\"([^\"]+)\".*$"
>>> >>>> $ echo '*.natvis diff=natvis' >.gitattributes
>>> >>>>
>>> >>>> ... then I get this, both on Linux (git version 2.11.1) and on
>>> >>>> Windows
>>> >>>> (git version 2.11.1.windows.1):
>>> >>>>
>>> >>>> diff --git a/a.natvis b/a.natvis
>>> >>>> index 7f9bdf5..bc3c090 100644
>>> >>>> --- a/a.natvis
>>> >>>> +++ b/a.natvis
>>> >>>> @@ -19,7 +19,7 @@ test
>>> >>>>
>>> >>>>
>>> >>>> <!-- Non-blank line -->
>>> >>>> - <Item Name="added var">added_var</Item>
>>> >>>> + <Item Name="added var">added_vars</Item>
>>> >>>>
>>> >>>>
>>> >>>> <Item Name="var2">var2</Item>
>>> >>>>
>>> >>>>> Just to be sure, I tested your regex and again it didn't work.
>>> >>>>
>>> >>>> At this point I'm out of ideas, sorry. :( The only way I was able to
>>> >>>> break it was due to mistyping the extension as "netvis" several times
>>> >>>> for some reason.
>>> >>>>
>>> >>>> René
^ permalink raw reply
* Re: Trying to use xfuncname without success.
From: Jack Adrian Zappa @ 2017-02-09 0:01 UTC (permalink / raw)
To: Jack Adrian Zappa; +Cc: git-mailing-list
In-Reply-To: <CAKepmaidYd4tMRwSNoTTga8s6ji-pc87fZqtxdeDcDVpFhcPWA@mail.gmail.com>
Tried to copy the .git/config file over to the non-working repository
and it didn't seem to do anything. Could the git database be
partially corrupted?
On Wed, Feb 8, 2017 at 7:00 PM, Jack Adrian Zappa <adrianh.bsc@gmail.com> wrote:
> Well, it mostly works, but I'm getting some weirdness where it has
> grabbed a line at 126 and is using that for the hunk header. Strange
> thing is, I move the file to another repository, commit it, make a
> change to the fileand do a diff, and it gets the correct hunk header.
>
> I'm at a loss. :(
>
> On Wed, Feb 8, 2017 at 4:12 PM, Jack Adrian Zappa <adrianh.bsc@gmail.com> wrote:
>> That was it. I have a .gitattributes file in my home directory.
>> Ahhh, but it's not in my %userprofile% directory, but in my ~
>> directory.
>>
>> A bit confusing having 2 home directories. I made a link to my
>> .gitconfig, but forgot to make a link to my .gitattributes.
>>
>> Thanks.
>>
>>
>> A
>>
>> On Wed, Feb 8, 2017 at 4:05 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>> Double check .gitattributes?
>>>
>>> On Feb 8, 2017 2:58 PM, "Jack Adrian Zappa" <adrianh.bsc@gmail.com> wrote:
>>>>
>>>> Thanks Samuel,
>>>>
>>>> That example showed that there must be something wrong in my .git
>>>> directory, because with it, I'm getting the correct output. Moving
>>>> the same lines to my .git/config didn't work.
>>>>
>>>> On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>>> > I just put this togther: https://github.com/sxlijin/xfuncname-test
>>>> >
>>>> > Try cloning and then for any of config1 thru 3,
>>>> >
>>>> > $ cp configX .git/config
>>>> > $ git diff HEAD^ -- test.natvis
>>>> >
>>>> > On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa
>>>> > <adrianh.bsc@gmail.com> wrote:
>>>> >> Thanks Samuel,
>>>> >>
>>>> >> So, the question is, what is causing this problem on my system?
>>>> >>
>>>> >> Anyone have an idea to help diagnose this problem?
>>>> >>
>>>> >> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>>> >>> On Windows 7, it works for me in both CMD and Git Bash:
>>>> >>>
>>>> >>> $ git --version
>>>> >>> git version 2.11.0.windows.3
>>>> >>>
>>>> >>> $ git diff HEAD^ --word-diff
>>>> >>> diff --git a/test.natvis b/test.natvis
>>>> >>> index 93396ad..1233b8c 100644
>>>> >>> --- a/test.natvis
>>>> >>> +++ b/test.natvis
>>>> >>> @@ -18,6 +18,7 @@ test
>>>> >>>
>>>> >>>
>>>> >>> <!-- Non-blank line -->
>>>> >>> {+<Item Name="added var">added_var</Item>+}
>>>> >>>
>>>> >>>
>>>> >>> <Item Name="var2">var2</Item>
>>>> >>>
>>>> >>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe <l.s.r@web.de> wrote:
>>>> >>>> Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>>>> >>>>> Thanks Rene, but you seem to have missed the point. NOTHING is
>>>> >>>>> working. No matter what I put there, it doesn't seem to get
>>>> >>>>> matched.
>>>> >>>>
>>>> >>>> I'm not so sure about that. With your example I get this diff
>>>> >>>> without
>>>> >>>> setting diff.natvis.xfuncname:
>>>> >>>>
>>>> >>>> diff --git a/a.natvis b/a.natvis
>>>> >>>> index 7f9bdf5..bc3c090 100644
>>>> >>>> --- a/a.natvis
>>>> >>>> +++ b/a.natvis
>>>> >>>> @@ -19,7 +19,7 @@
>>>> >>>> xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
>>>> >>>>
>>>> >>>>
>>>> >>>> <!-- Non-blank line -->
>>>> >>>> - <Item Name="added var">added_var</Item>
>>>> >>>> + <Item Name="added var">added_vars</Item>
>>>> >>>>
>>>> >>>>
>>>> >>>> <Item Name="var2">var2</Item>
>>>> >>>>
>>>> >>>> Note the XML namespace in the hunk header. It's put there by the
>>>> >>>> default rule because "xmlns" starts at the beginning of the line.
>>>> >>>> Your
>>>> >>>> diff has nothing there, which means the default rule is not used,
>>>> >>>> i.e.
>>>> >>>> your user-defined rule is in effect.
>>>> >>>>
>>>> >>>> Come to think of it, this line break in the middle of the
>>>> >>>> AutoVisualizer
>>>> >>>> tab might have been added by your email client unintentionally, so
>>>> >>>> that
>>>> >>>> we use different test files, which then of course results in
>>>> >>>> different
>>>> >>>> diffs. Is that the case?
>>>> >>>>
>>>> >>>> Anyway, if I run the following two commands:
>>>> >>>>
>>>> >>>> $ git config diff.natvis.xfuncname "^[\t ]*<Type[\t
>>>> >>>> ]+Name=\"([^\"]+)\".*$"
>>>> >>>> $ echo '*.natvis diff=natvis' >.gitattributes
>>>> >>>>
>>>> >>>> ... then I get this, both on Linux (git version 2.11.1) and on
>>>> >>>> Windows
>>>> >>>> (git version 2.11.1.windows.1):
>>>> >>>>
>>>> >>>> diff --git a/a.natvis b/a.natvis
>>>> >>>> index 7f9bdf5..bc3c090 100644
>>>> >>>> --- a/a.natvis
>>>> >>>> +++ b/a.natvis
>>>> >>>> @@ -19,7 +19,7 @@ test
>>>> >>>>
>>>> >>>>
>>>> >>>> <!-- Non-blank line -->
>>>> >>>> - <Item Name="added var">added_var</Item>
>>>> >>>> + <Item Name="added var">added_vars</Item>
>>>> >>>>
>>>> >>>>
>>>> >>>> <Item Name="var2">var2</Item>
>>>> >>>>
>>>> >>>>> Just to be sure, I tested your regex and again it didn't work.
>>>> >>>>
>>>> >>>> At this point I'm out of ideas, sorry. :( The only way I was able to
>>>> >>>> break it was due to mistyping the extension as "netvis" several times
>>>> >>>> for some reason.
>>>> >>>>
>>>> >>>> René
^ permalink raw reply
* Re: Automatically Add .gitignore Files
From: Stephan Beyer @ 2017-02-09 0:01 UTC (permalink / raw)
To: Junio C Hamano, Thangalin; +Cc: git
In-Reply-To: <xmqqmvdwtfq8.fsf@gitster.mtv.corp.google.com>
> I am tempted to say that there should be a way to somehow forbid use
> of the "-m" option to "git commit" by default, until the user gains
> more familiarity with use of Git.
Since I am using git, I am tempted to say that "git commit -m" should be
abolished. If I tell somebody how to use git, I never mention "git
commit -m". Unluckily they find it out themselves... ;D
The typical observations I have when people use "git commit -m":
* The commit messages are either too long (in one line!) or
too meaningless.
* People don't check what they added and commit wrong stuff.
* People make fast temporary commits with "asdafsd" commit messages.
Then they get distracted for some time and work on another branch.
When they turn back to the old branch they don't know what "asdafsd"
was...
Thanks to git rebase -i and/or git commit --amend, it is all not too bad
after all ;D
Best
Stephan
^ permalink raw reply
* Re: Trying to use xfuncname without success.
From: Jack Adrian Zappa @ 2017-02-09 0:10 UTC (permalink / raw)
Cc: git-mailing-list
In-Reply-To: <CAKepmagSysE_31Y3JJwhOKvD_kGfiyEXikep62g=cn9+=v_fZA@mail.gmail.com>
> where it has grabbed a line at 126 and is using that for the hunk header.
When I say that, I mean that it is using that line for *every* hunk
header, for every change, regardless if it has passed a hunk head that
it should have matched.
On Wed, Feb 8, 2017 at 7:01 PM, Jack Adrian Zappa <adrianh.bsc@gmail.com> wrote:
> Tried to copy the .git/config file over to the non-working repository
> and it didn't seem to do anything. Could the git database be
> partially corrupted?
>
> On Wed, Feb 8, 2017 at 7:00 PM, Jack Adrian Zappa <adrianh.bsc@gmail.com> wrote:
>> Well, it mostly works, but I'm getting some weirdness where it has
>> grabbed a line at 126 and is using that for the hunk header. Strange
>> thing is, I move the file to another repository, commit it, make a
>> change to the fileand do a diff, and it gets the correct hunk header.
>>
>> I'm at a loss. :(
>>
>> On Wed, Feb 8, 2017 at 4:12 PM, Jack Adrian Zappa <adrianh.bsc@gmail.com> wrote:
>>> That was it. I have a .gitattributes file in my home directory.
>>> Ahhh, but it's not in my %userprofile% directory, but in my ~
>>> directory.
>>>
>>> A bit confusing having 2 home directories. I made a link to my
>>> .gitconfig, but forgot to make a link to my .gitattributes.
>>>
>>> Thanks.
>>>
>>>
>>> A
>>>
>>> On Wed, Feb 8, 2017 at 4:05 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>>> Double check .gitattributes?
>>>>
>>>> On Feb 8, 2017 2:58 PM, "Jack Adrian Zappa" <adrianh.bsc@gmail.com> wrote:
>>>>>
>>>>> Thanks Samuel,
>>>>>
>>>>> That example showed that there must be something wrong in my .git
>>>>> directory, because with it, I'm getting the correct output. Moving
>>>>> the same lines to my .git/config didn't work.
>>>>>
>>>>> On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>>>> > I just put this togther: https://github.com/sxlijin/xfuncname-test
>>>>> >
>>>>> > Try cloning and then for any of config1 thru 3,
>>>>> >
>>>>> > $ cp configX .git/config
>>>>> > $ git diff HEAD^ -- test.natvis
>>>>> >
>>>>> > On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa
>>>>> > <adrianh.bsc@gmail.com> wrote:
>>>>> >> Thanks Samuel,
>>>>> >>
>>>>> >> So, the question is, what is causing this problem on my system?
>>>>> >>
>>>>> >> Anyone have an idea to help diagnose this problem?
>>>>> >>
>>>>> >> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>>>> >>> On Windows 7, it works for me in both CMD and Git Bash:
>>>>> >>>
>>>>> >>> $ git --version
>>>>> >>> git version 2.11.0.windows.3
>>>>> >>>
>>>>> >>> $ git diff HEAD^ --word-diff
>>>>> >>> diff --git a/test.natvis b/test.natvis
>>>>> >>> index 93396ad..1233b8c 100644
>>>>> >>> --- a/test.natvis
>>>>> >>> +++ b/test.natvis
>>>>> >>> @@ -18,6 +18,7 @@ test
>>>>> >>>
>>>>> >>>
>>>>> >>> <!-- Non-blank line -->
>>>>> >>> {+<Item Name="added var">added_var</Item>+}
>>>>> >>>
>>>>> >>>
>>>>> >>> <Item Name="var2">var2</Item>
>>>>> >>>
>>>>> >>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe <l.s.r@web.de> wrote:
>>>>> >>>> Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>>>>> >>>>> Thanks Rene, but you seem to have missed the point. NOTHING is
>>>>> >>>>> working. No matter what I put there, it doesn't seem to get
>>>>> >>>>> matched.
>>>>> >>>>
>>>>> >>>> I'm not so sure about that. With your example I get this diff
>>>>> >>>> without
>>>>> >>>> setting diff.natvis.xfuncname:
>>>>> >>>>
>>>>> >>>> diff --git a/a.natvis b/a.natvis
>>>>> >>>> index 7f9bdf5..bc3c090 100644
>>>>> >>>> --- a/a.natvis
>>>>> >>>> +++ b/a.natvis
>>>>> >>>> @@ -19,7 +19,7 @@
>>>>> >>>> xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> <!-- Non-blank line -->
>>>>> >>>> - <Item Name="added var">added_var</Item>
>>>>> >>>> + <Item Name="added var">added_vars</Item>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> <Item Name="var2">var2</Item>
>>>>> >>>>
>>>>> >>>> Note the XML namespace in the hunk header. It's put there by the
>>>>> >>>> default rule because "xmlns" starts at the beginning of the line.
>>>>> >>>> Your
>>>>> >>>> diff has nothing there, which means the default rule is not used,
>>>>> >>>> i.e.
>>>>> >>>> your user-defined rule is in effect.
>>>>> >>>>
>>>>> >>>> Come to think of it, this line break in the middle of the
>>>>> >>>> AutoVisualizer
>>>>> >>>> tab might have been added by your email client unintentionally, so
>>>>> >>>> that
>>>>> >>>> we use different test files, which then of course results in
>>>>> >>>> different
>>>>> >>>> diffs. Is that the case?
>>>>> >>>>
>>>>> >>>> Anyway, if I run the following two commands:
>>>>> >>>>
>>>>> >>>> $ git config diff.natvis.xfuncname "^[\t ]*<Type[\t
>>>>> >>>> ]+Name=\"([^\"]+)\".*$"
>>>>> >>>> $ echo '*.natvis diff=natvis' >.gitattributes
>>>>> >>>>
>>>>> >>>> ... then I get this, both on Linux (git version 2.11.1) and on
>>>>> >>>> Windows
>>>>> >>>> (git version 2.11.1.windows.1):
>>>>> >>>>
>>>>> >>>> diff --git a/a.natvis b/a.natvis
>>>>> >>>> index 7f9bdf5..bc3c090 100644
>>>>> >>>> --- a/a.natvis
>>>>> >>>> +++ b/a.natvis
>>>>> >>>> @@ -19,7 +19,7 @@ test
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> <!-- Non-blank line -->
>>>>> >>>> - <Item Name="added var">added_var</Item>
>>>>> >>>> + <Item Name="added var">added_vars</Item>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> <Item Name="var2">var2</Item>
>>>>> >>>>
>>>>> >>>>> Just to be sure, I tested your regex and again it didn't work.
>>>>> >>>>
>>>>> >>>> At this point I'm out of ideas, sorry. :( The only way I was able to
>>>>> >>>> break it was due to mistyping the extension as "netvis" several times
>>>>> >>>> for some reason.
>>>>> >>>>
>>>>> >>>> René
^ permalink raw reply
* Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Siddharth Kannan @ 2017-02-08 17:23 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Git List, Pranit Bauva, Jeff King, pclouds,
brian m. carlson
In-Reply-To: <vpqh944eof7.fsf@anie.imag.fr>
Hello Matthieu,
On 8 February 2017 at 20:10, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> In a previous discussion, I made an analogy with "cd -" (which is the
> source of inspiration of this shorthand AFAIK): "-" did not magically
> become "the last visited directory" for all Unix commands, just for
> "cd". And in this case, I'm happy with it. For example, I never need
> "mkdir -", and I'm happy I can't "rm -fr -" by mistake.
>
> So, it's debatable whether it's a good thing to have all commands
> support "-". For example, forcing users to explicitly type "git branch
> -d @{1}" and not providing them with a shortcut might be a good thing.
builtin/branch.c does not call setup_revisions and remains unaffected
by this patch :)
In my original patch post, I was not explicit about what files call
setup_revisions.
I would like to rectify that with this (grep-ed) list:
* builtin/blame.c
* builtin/diff.c
* builtin/diff-files.c
* builtin/diff-index.c
* builtin/diff-tree.c
* builtin/log.c
* builtin/rev-list.c
* builtin/shortlog.c
* builtin/fast-export.c
* builtin/fmt-merge-msg.c
builtin/add.c
builtin/checkout.c
builtin/commit.c
builtin/merge.c
builtin/pack-objects.c
builtin/revert.c
* marked commands only show information, and don't change anything.
As you might notice, in this list, most commands are not of the `rm` variety,
i.e. something that would delete stuff.
In the next version of this patch, I will definitely include the list
of commands
which are "rm-ish" and affected by this patch.
>
> I don't have strong opinion on this: I tend to favor consistency and
> supporting "-" everywhere goes in this direction, but I think the
> downsides should be considered too. A large part of the exercice here is
> to write a good commit message!
Yes, I prefer consistency very much as well! Having "-" mean the same thing
across a lot of commands is better than having that shorthand only in a few
commands, as it is now. Unless there is a specific confusion that might arise
because of this shorthand inclusion, I think that this shorthand
should be supported
across the board.
(I especially like typing `git checkout - <filename>` which is very handy!)
>
> Another issue with this is: - is also a common way to say "use stdin
> instead of a file", so before enabling - for "previous branch", we need
> to make sure it does not introduce any ambiguity. Git does not seem to
> use "- for stdin" much (most commands able to read from stdin have an
> explicit --stdin option for that), a quick grep in the docs shows only
> "git blame --contents -" which is OK because a revision wouldn't make
> sense here anyway.
Yes, just to jog your memory, this was discussed here [1]
Junio said:
As long as the addition is carefully prepared so that we know it
will not conflict (or be confused by users) with possible other uses
of "-", I do not think we would mind "git branch -D -" and other
commands to learn "-" as a synonym for @{-1}.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
Thanks a lot for the review on this patch, Matthieu!
--
Best Regards,
- Siddharth.
[1]: https://public-inbox.org/git/7vmwpitb6k.fsf@alter.siamese.dyndns.org/
^ permalink raw reply
* Re: [PATCH v4] tag: generate useful reflog message
From: Junio C Hamano @ 2017-02-08 23:44 UTC (permalink / raw)
To: Cornelius Weig; +Cc: git, karthik.188, peff, bitte.keine.werbung.einwerfen
In-Reply-To: <d0170d3a-3022-3bca-7c80-7ef0b1cf62a0@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
> A version with revised tests will follow.
Thanks; I think this is clean enough. Let's queue this one and
advance it to 'next' soonish.
^ permalink raw reply
* Re: "disabling bitmap writing, as some objects are not being packed"?
From: Junio C Hamano @ 2017-02-09 0:18 UTC (permalink / raw)
To: Jeff King; +Cc: David Turner, Duy Nguyen, Git Mailing List
In-Reply-To: <20170208230057.hking37uuynf4cgd@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> In my experience, auto-gc has never been a low-maintenance operation on
> the server side (and I do think it was primarily designed with clients
> in mind).
I do not think auto-gc was ever tweaked to help server usage, in its
history since it was invented strictly to help end-users (mostly new
ones).
> At GitHub we disable it entirely, and do our own gc based on a throttled
> job queue ...
> I wish regular Git were more turn-key in that respect. Maybe it is for
> smaller sites, but we certainly didn't find it so. And I don't know that
> it's feasible to really share the solution. It's entangled with our
> database (to store last-pushed and last-maintenance values for repos)
> and our job scheduler.
Thanks for sharing the insights from the trenches ;-)
> Yeah, I'm certainly open to improving Git's defaults. If it's not clear
> from the above, I mostly just gave up for a site the size of GitHub. :)
>
>> Idea 1: when gc --auto would issue this message, instead it could create
>> a file named gc.too-much-garbage (instead of gc.log), with this message.
>> If that file exists, and it is less than one day (?) old, then we don't
>> attempt to do a full gc; instead we just run git repack -A -d. (If it's
>> more than one day old, we just delete it and continue anyway).
>
> I kind of wonder if this should apply to _any_ error. I.e., just check
> the mtime of gc.log and forcibly remove it when it's older than a day.
> You never want to get into a state that will fail to resolve itself
> eventually. That might still happen (e.g., corrupt repo), but at the
> very least it won't be because Git is too dumb to try again.
;-)
>> Idea 2 : Like idea 1, but instead of repacking, just smash the existing
>> packs together into one big pack. In other words, don't consider
>> dangling objects, or recompute deltas. Twitter has a tool called "git
>> combine-pack" that does this:
>> https://github.com/dturner-tw/git/blob/dturner/journal/builtin/combine-pack.c
>
> We wrote something similar at GitHub, too, but we never ended up using
> it in production. We found that with a sane scheduler, it's not too big
> a deal to just do maintenance once in a while.
Thanks again for this. I've also been wondering about how effective
a "concatenate packs without paying reachability penalty" would be.
> I'm still not sure if it's worth making the fatal/non-fatal distinction.
> Doing so is perhaps safer, but it does mean that somebody has to decide
> which errors are important enough to block a retry totally, and which
> are not. In theory, it would be safe to always _try_ and then the gc
> process can decide when something is broken and abort. And all you've
> wasted is some processing power each day.
Yup, and somebody or something need to monitor so that repeated
failures can be dealt with.
^ permalink raw reply
* Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
From: Stefan Beller @ 2017-02-08 23:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, Linus Torvalds, Git Mailing List
In-Reply-To: <xmqq37fouvu3.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 8, 2017 at 3:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> - it hints that '!' is the more official spelling, making the
> output you showed above acceptable.
Long term , I'd rather have ^ as the "official" spelling as that is easier
to teach to people. ! being a historic mistake as it is hard to type?
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
From: Stefan Beller @ 2017-02-09 0:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqzihzymn3.fsf@gitster.mtv.corp.google.com>
> * sb/submodule-doc (2017-01-12) 3 commits
> - submodules: add a background story
> - submodule update documentation: don't repeat ourselves
> - submodule documentation: add options to the subcommand
>
> Needs review.
>
The first two commits are good to go IIRC as you seemed to be
positive about them at the time. Though I have a hard time finding
evidence of such.
I am currently reworking the 3/3 "add a background story" patch as it is
RFC-ish, so no need to review that any more.
Maybe we can get 1&2 merged and then 3 comes on its own?
Thanks,
Stefan
^ permalink raw reply
* Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)
From: Junio C Hamano @ 2017-02-09 0:26 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, peff
In-Reply-To: <20170207235350.10718-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> Usages of the list of remote refs or the remote-local ref
> map are updated as follows:
> - check_not_current_branch (which checks that the current branch is not
> affected by the fetch) is performed both on the original ref map (to
> die before the fetch if we can, as an optimization) and on the new
> ref map (since the new ref map is the one actually applied).
OK.
> - Pruning is done based on the new ref map.
OK. As that is what eventually gets "installed" on the local side,
it makes sense to become consistent with that set, not the set the
original server gave you.
> - backfill_tags (for tag following) is performed using the original
> list of remote refs because the new list of fetched refs is not
> guaranteed to contain tag information. (Since backfill_tags performs
> another fetch, it does not need to be fully consistent with the
> just-returned packfile.)
This smells correct but I need to think about this one a bit more.
Overall I think the strategy is agreeable.
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
From: Junio C Hamano @ 2017-02-09 1:01 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kZ6m1NGz9Lhhd5BDxayLW-UC8MqexMyhHBhHKrmg-tB+w@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> * sb/submodule-doc (2017-01-12) 3 commits
>> - submodules: add a background story
>> - submodule update documentation: don't repeat ourselves
>> - submodule documentation: add options to the subcommand
>
> The first two commits are good to go IIRC as you seemed to be
> positive about them at the time. Though I have a hard time finding
> evidence of such.
>
> I am currently reworking the 3/3 "add a background story" patch as it is
> RFC-ish, so no need to review that any more.
>
> Maybe we can get 1&2 merged and then 3 comes on its own?
Yeah, sorry, I keep forgetting this topic, it seems. Let's advance
the earlier two. Tentatively I'd drop the third one as you plan to
redo it separately.
Thanks for prodding.
^ permalink raw reply
* Re: "disabling bitmap writing, as some objects are not being packed"?
From: Jeff King @ 2017-02-09 1:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Turner, Duy Nguyen, Git Mailing List
In-Reply-To: <xmqqbmuctdwu.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 08, 2017 at 04:18:25PM -0800, Junio C Hamano wrote:
> > We wrote something similar at GitHub, too, but we never ended up using
> > it in production. We found that with a sane scheduler, it's not too big
> > a deal to just do maintenance once in a while.
>
> Thanks again for this. I've also been wondering about how effective
> a "concatenate packs without paying reachability penalty" would be.
For the sake of posterity, I'll include our patch at the end (sorry, not
chunked into nice readable commits; that never existed in the first
place).
> > I'm still not sure if it's worth making the fatal/non-fatal distinction.
> > Doing so is perhaps safer, but it does mean that somebody has to decide
> > which errors are important enough to block a retry totally, and which
> > are not. In theory, it would be safe to always _try_ and then the gc
> > process can decide when something is broken and abort. And all you've
> > wasted is some processing power each day.
>
> Yup, and somebody or something need to monitor so that repeated
> failures can be dealt with.
Yes. I think that part is probably outside the scope of Git. But if
auto-gc leaves gc.log lying around, it would be easy to visit each repo
and collect the various failures.
-- >8 --
This is the "pack-fast" patch, for reference. It applies on v2.6.5,
though I had to do some wiggling due to a few of our other custom
patches, so it's possible I introduced new bugs. It compiles, but I
didn't actually re-test the result. I _think_ the original at least
generated valid packs in all cases.
So I would certainly not recommend anybody run this. It's just a
possible base to work off of if anybody's interested in the topic. I
haven't looked at David's combine-packs at all to see if it is any less
gross. :)
---
Makefile | 1 +
builtin.h | 1 +
builtin/pack-fast.c | 618 +++++++++++++++++++++++++++++++++++
cache.h | 5 +
git.c | 1 +
pack-bitmap-write.c | 167 +++++++++-
pack-bitmap.c | 2 +-
pack-bitmap.h | 8 +
sha1_file.c | 4 +-
9 files changed, 792 insertions(+), 15 deletions(-)
diff --git a/Makefile b/Makefile
index 37e2d9e18..524b185ec 100644
--- a/Makefile
+++ b/Makefile
@@ -887,6 +887,7 @@ BUILTIN_OBJS += builtin/mv.o
BUILTIN_OBJS += builtin/name-rev.o
BUILTIN_OBJS += builtin/notes.o
BUILTIN_OBJS += builtin/pack-objects.o
+BUILTIN_OBJS += builtin/pack-fast.o
BUILTIN_OBJS += builtin/pack-redundant.o
BUILTIN_OBJS += builtin/pack-refs.o
BUILTIN_OBJS += builtin/patch-id.o
diff --git a/builtin.h b/builtin.h
index 79aaf0afe..df4e4d668 100644
--- a/builtin.h
+++ b/builtin.h
@@ -95,6 +95,7 @@ extern int cmd_mv(int argc, const char **argv, const char *prefix);
extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
extern int cmd_notes(int argc, const char **argv, const char *prefix);
extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
+extern int cmd_pack_fast(int argc, const char **argv, const char *prefix);
extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix);
extern int cmd_patch_id(int argc, const char **argv, const char *prefix);
extern int cmd_prune(int argc, const char **argv, const char *prefix);
diff --git a/builtin/pack-fast.c b/builtin/pack-fast.c
new file mode 100644
index 000000000..ad9f5e5f1
--- /dev/null
+++ b/builtin/pack-fast.c
@@ -0,0 +1,618 @@
+#include "builtin.h"
+#include "cache.h"
+#include "pack.h"
+#include "progress.h"
+#include "csum-file.h"
+#include "sha1-lookup.h"
+#include "parse-options.h"
+#include "tempfile.h"
+#include "pack-bitmap.h"
+#include "pack-revindex.h"
+
+static const char *pack_usage[] = {
+ N_("git pack-fast --quiet [options...] [base-name]"),
+ NULL
+};
+
+struct packwriter {
+ struct tempfile *tmp;
+ off_t total;
+ int fd;
+ uint32_t crc32;
+ unsigned do_crc;
+};
+
+static void packwriter_crc32_start(struct packwriter *w)
+{
+ w->crc32 = crc32(0, NULL, 0);
+ w->do_crc = 1;
+}
+
+static uint32_t packwriter_crc32_end(struct packwriter *w)
+{
+ w->do_crc = 0;
+ return w->crc32;
+}
+
+static void packwriter_write(struct packwriter *w, const void *buf, unsigned int count)
+{
+ if (w->do_crc)
+ w->crc32 = crc32(w->crc32, buf, count);
+ write_or_die(w->fd, buf, count);
+ w->total += count;
+}
+
+static off_t packwriter_total(struct packwriter *w)
+{
+ return w->total;
+}
+
+static void packwriter_init(struct packwriter *w)
+{
+ char tmpname[PATH_MAX];
+
+ w->fd = odb_mkstemp(tmpname, sizeof(tmpname), "pack/tmp_pack_XXXXXX");
+ w->total = 0;
+ w->do_crc = 0;
+ w->tmp = xcalloc(1, sizeof(*w->tmp));
+
+ register_tempfile(w->tmp, tmpname);
+}
+
+
+static int progress = 1;
+static struct progress *progress_state;
+static struct pack_idx_option pack_idx_opts;
+static const char *base_name = "pack-fast";
+static int skip_largest;
+static int write_bitmap_index = 1;
+
+static struct packed_git **all_packfiles;
+static unsigned int all_packfiles_nr;
+
+static struct pack_idx_entry **written_list;
+static unsigned int written_nr;
+
+struct write_slab {
+ struct write_slab *next;
+ unsigned int nr;
+
+ struct write_slab_entry {
+ struct pack_idx_entry idx;
+ enum object_type real_type;
+ } entries[];
+};
+
+static struct write_slab *written_slab_root;
+static struct write_slab *written_slab_current;
+
+static void add_to_write_list(
+ const unsigned char *sha1, off_t offset, uint32_t crc32,
+ enum object_type real_type)
+{
+ struct write_slab *slab = written_slab_current;
+ struct write_slab_entry *entry = &(slab->entries[slab->nr++]);
+
+ entry->real_type = real_type;
+ entry->idx.offset = offset;
+ entry->idx.crc32 = crc32;
+ hashcpy(entry->idx.sha1, sha1);
+}
+
+static void preallocate_write_slab(unsigned int num_entries)
+{
+ struct write_slab *slab = xmalloc(
+ sizeof(struct write_slab) +
+ num_entries * sizeof(struct write_slab_entry));
+
+ slab->next = NULL;
+ slab->nr = 0;
+
+ if (!written_slab_current) {
+ written_slab_current = slab;
+ written_slab_root = slab;
+ } else {
+ written_slab_current->next = slab;
+ written_slab_current = slab;
+ }
+}
+
+static struct skipped_object {
+ off_t skipped_offset;
+ off_t real_offset;
+} *skipped_list;
+static unsigned int skipped_nr;
+static unsigned int skipped_alloc;
+
+static void add_to_skipped_list(off_t skipped_offset, off_t real_offset)
+{
+ if (skipped_nr >= skipped_alloc) {
+ skipped_alloc = (skipped_alloc + 32) * 2;
+ REALLOC_ARRAY(skipped_list, skipped_alloc);
+ }
+
+ skipped_list[skipped_nr].skipped_offset = skipped_offset;
+ skipped_list[skipped_nr].real_offset = real_offset;
+ skipped_nr++;
+}
+
+static off_t find_real_offset_for_base(off_t skipped_offset)
+{
+ int lo = 0, hi = skipped_nr;
+ while (lo < hi) {
+ int mi = lo + ((hi - lo) / 2);
+ if (skipped_offset == skipped_list[mi].skipped_offset)
+ return skipped_list[mi].real_offset;
+ if (skipped_offset < skipped_list[mi].skipped_offset)
+ hi = mi;
+ else
+ lo = mi + 1;
+ }
+
+ return 0;
+}
+
+/*
+ * Record the offsets needed in our reused packfile chunks due to
+ * "gaps" where we omitted some objects.
+ */
+static struct reused_chunk {
+ off_t start;
+ off_t offset;
+} *reused_chunks;
+static int reused_chunks_nr;
+static int reused_chunks_alloc;
+
+static void record_reused_object(off_t where, off_t offset)
+{
+ if (reused_chunks_nr && reused_chunks[reused_chunks_nr-1].offset == offset)
+ return;
+
+ ALLOC_GROW(reused_chunks, reused_chunks_nr + 1,
+ reused_chunks_alloc);
+ reused_chunks[reused_chunks_nr].start = where;
+ reused_chunks[reused_chunks_nr].offset = offset;
+ reused_chunks_nr++;
+}
+
+/*
+ * Binary search to find the chunk that "where" is in. Note
+ * that we're not looking for an exact match, just the first
+ * chunk that contains it (which implicitly ends at the start
+ * of the next chunk.
+ */
+static off_t find_reused_offset(off_t where)
+{
+ int lo = 0, hi = reused_chunks_nr;
+ while (lo < hi) {
+ int mi = lo + ((hi - lo) / 2);
+ if (where == reused_chunks[mi].start)
+ return reused_chunks[mi].offset;
+ if (where < reused_chunks[mi].start)
+ hi = mi;
+ else
+ lo = mi + 1;
+ }
+
+ /*
+ * The first chunk starts at zero, so we can't have gone below
+ * there.
+ */
+ assert(lo);
+ return reused_chunks[lo-1].offset;
+}
+
+static uint32_t nth_packed_object_crc32(const struct packed_git *p, uint32_t nr)
+{
+ const uint32_t *index_crc = p->index_data;
+ index_crc += 2 + 256 + p->num_objects * (20/4) + nr;
+ return ntohl(*index_crc);
+}
+
+static void load_index_or_die(struct packed_git *p)
+{
+ if (open_pack_index(p) < 0)
+ die("failed to open index for '%s'", p->pack_name);
+
+ if (p->index_version != 2)
+ die("unsupported index version %d (fast-pack requires index v2)\n",
+ p->index_version);
+}
+
+static int sort_pack(const void *a_, const void *b_)
+{
+ struct packed_git *a = *((struct packed_git **)a_);
+ struct packed_git *b = *((struct packed_git **)b_);
+
+ if (a->mtime > b->mtime)
+ return 1;
+ else if (a->mtime == b->mtime)
+ return 0;
+ return -1;
+}
+
+static void find_packfiles(void)
+{
+ struct packed_git *p;
+ unsigned int n;
+
+ prepare_packed_git();
+
+ for (n = 0, p = packed_git; p; p = p->next) {
+ if (p->pack_local)
+ n++;
+ }
+
+ all_packfiles = xcalloc(n, sizeof(struct packed_git *));
+ all_packfiles_nr = n;
+
+ for (n = 0, p = packed_git; p; p = p->next) {
+ if (p->pack_local)
+ all_packfiles[n++] = p;
+ }
+
+ for (n = 1; n < all_packfiles_nr; ++n) {
+ if (all_packfiles[n]->pack_size > all_packfiles[0]->pack_size) {
+ struct packed_git *tmp = all_packfiles[0];
+ all_packfiles[0] = all_packfiles[n];
+ all_packfiles[n] = tmp;
+ }
+ }
+
+ qsort(all_packfiles + 1, all_packfiles_nr - 1, sizeof(struct packed_git *), sort_pack);
+}
+
+static int sha1_index__cmp(const void *a_, const void *b_)
+{
+ struct pack_idx_entry *a = *((struct pack_idx_entry **)a_);
+ struct pack_idx_entry *b = *((struct pack_idx_entry **)b_);
+ return hashcmp(a->sha1, b->sha1);
+}
+
+static const unsigned char *sha1_index__access(size_t pos, void *table)
+{
+ struct pack_idx_entry **index = table;
+ return index[pos]->sha1;
+}
+
+static void sha1_index_update(void)
+{
+ const unsigned int left_nr = written_nr;
+ const unsigned int right_nr = written_slab_current->nr;
+ const unsigned int total_nr = left_nr + right_nr;
+
+ struct pack_idx_entry **left = written_list;
+ struct pack_idx_entry **right = xmalloc(right_nr * sizeof(struct pack_idx_entry *));
+ struct pack_idx_entry **result = xmalloc(total_nr * sizeof(struct pack_idx_entry *));
+
+ unsigned int i, j, n;
+
+ for (j = 0; j < right_nr; ++j)
+ right[j] = (struct pack_idx_entry *)(&written_slab_current->entries[j]);
+
+ qsort(right, right_nr, sizeof(struct pack_idx_entry *), sha1_index__cmp);
+
+ for (i = j = n = 0; i < left_nr && j < right_nr; ++n) {
+ struct pack_idx_entry *a = left[i];
+ struct pack_idx_entry *b = right[j];
+
+ if (hashcmp(a->sha1, b->sha1) <= 0) {
+ result[n] = a;
+ i++;
+ } else {
+ result[n] = b;
+ j++;
+ }
+ }
+
+ for (; i < left_nr; ++n, ++i)
+ result[n] = left[i];
+
+ for (; j < right_nr; ++n, ++j)
+ result[n] = right[j];
+
+ free(written_list);
+ free(right);
+
+ written_list = result;
+ written_nr = total_nr;
+}
+
+static off_t sha1_index_find_offset(const unsigned char *sha1)
+{
+ int pos = sha1_pos(sha1, written_list, written_nr, sha1_index__access);
+ return (pos < 0) ? 0 : written_list[pos]->offset;
+}
+
+static void copy_pack_data(
+ struct packwriter *w,
+ struct packed_git *p,
+ struct pack_window **w_curs,
+ off_t offset,
+ off_t len)
+{
+ unsigned char *in;
+ unsigned long avail;
+
+ while (len) {
+ in = use_pack(p, w_curs, offset, &avail);
+ if (avail > len)
+ avail = (unsigned long)len;
+ packwriter_write(w, in, avail);
+ offset += avail;
+ len -= avail;
+ }
+}
+
+extern enum object_type packed_to_object_type(
+ struct packed_git *p, off_t obj_offset, enum object_type type,
+ struct pack_window **w_curs, off_t curpos);
+
+static int append_object_1(
+ struct revindex_entry *reventry,
+ struct packwriter *w,
+ struct packed_git *pack,
+ struct pack_window **w_curs,
+ enum object_type *real_type)
+{
+ const off_t offset = reventry[0].offset;
+ const off_t next = reventry[1].offset;
+
+ off_t cur;
+ enum object_type type;
+ unsigned long size;
+
+ record_reused_object(offset, offset - packwriter_total(w));
+
+ cur = offset;
+ type = unpack_object_header(pack, w_curs, &cur, &size);
+ assert(type >= 0);
+
+ if (write_bitmap_index)
+ *real_type = packed_to_object_type(pack, offset, type, w_curs, cur);
+
+ if (type == OBJ_OFS_DELTA) {
+ const off_t base_offset = get_delta_base(pack, w_curs, &cur, type, offset);
+ const off_t real_base_offset = find_real_offset_for_base(base_offset);
+ off_t fixed_offset = 0;
+
+ assert(base_offset != 0);
+
+ if (real_base_offset) {
+ fixed_offset = packwriter_total(w) - real_base_offset;
+ } else {
+ off_t fixup = find_reused_offset(offset) - find_reused_offset(base_offset);
+ if (fixup)
+ fixed_offset = offset - base_offset - fixup;
+ }
+
+ if (fixed_offset) {
+ unsigned char header[10], ofs_header[10];
+ unsigned i, len, ofs_len;
+
+ assert(fixed_offset > 0);
+ len = encode_in_pack_object_header(OBJ_OFS_DELTA, size, header);
+
+ i = sizeof(ofs_header) - 1;
+ ofs_header[i] = fixed_offset & 127;
+ while (fixed_offset >>= 7)
+ ofs_header[--i] = 128 | (--fixed_offset & 127);
+
+ ofs_len = sizeof(ofs_header) - i;
+
+ packwriter_write(w, header, len);
+ packwriter_write(w, ofs_header + sizeof(ofs_header) - ofs_len, ofs_len);
+ copy_pack_data(w, pack, w_curs, cur, next - cur);
+ return 1;
+ }
+
+ /* ...otherwise we have no fixup, and can write it verbatim */
+ }
+
+ copy_pack_data(w, pack, w_curs, offset, next - offset);
+ return 0;
+}
+
+static int copy_packfile(int from, struct packwriter *w)
+{
+ unsigned char buffer[8192];
+ struct stat st;
+ ssize_t to_read;
+
+ if (from < 0 || fstat(from, &st))
+ return -1;
+
+ posix_fadvise(from, 0, st.st_size, POSIX_FADV_SEQUENTIAL);
+ to_read = st.st_size - 20;
+
+ if (progress)
+ fprintf(stderr, "Copying main packfile...");
+
+ while (to_read) {
+ ssize_t r, cap = sizeof(buffer);
+
+ if (cap > to_read)
+ cap = to_read;
+
+ r = xread(from, buffer, cap);
+ if (r < 0)
+ return -1;
+
+ packwriter_write(w, buffer, r);
+ to_read -= r;
+ }
+
+ if (progress)
+ fprintf(stderr, " done.\n");
+ assert(to_read == 0);
+ return 0;
+}
+
+static void write_initial_packfile(struct packed_git *p, struct packwriter *w)
+{
+ unsigned int n;
+ int source_fd = git_open_noatime(p->pack_name);
+
+ if (copy_packfile(source_fd, w) < 0)
+ die_errno("failed to copy '%s'", p->pack_name);
+ close(source_fd);
+
+ load_index_or_die(p);
+ preallocate_write_slab(p->num_objects);
+
+ if (progress)
+ progress_state = start_progress("Indexing main packfile", p->num_objects);
+
+ for (n = 0; n < p->num_objects; ++n) {
+ const unsigned char *sha1 = nth_packed_object_sha1(p, n);
+ const off_t offset = nth_packed_object_offset(p, n);
+ const uint32_t crc32 = nth_packed_object_crc32(p, n);
+ add_to_write_list(sha1, offset, crc32, OBJ_BAD);
+ display_progress(progress_state, n + 1);
+ }
+
+ stop_progress(&progress_state);
+ close_pack_index(p);
+
+ written_list = xmalloc(p->num_objects * sizeof(struct packed_git *));
+ written_nr = p->num_objects;
+ for (n = 0; n < written_nr; ++n)
+ written_list[n] = (struct pack_idx_entry *)(&written_slab_current->entries[n]);
+}
+
+static void append_packfile(struct packed_git *p, struct packwriter *w)
+{
+ struct pack_window *w_curs = NULL;
+ struct pack_revindex *revidx;
+
+ unsigned int n;
+
+ load_index_or_die(p);
+ preallocate_write_slab(p->num_objects);
+ revidx = revindex_for_pack(p);
+
+ if (progress)
+ progress_state = start_progress("Appending packfile", p->num_objects);
+
+ for (n = 0; n < p->num_objects; ++n) {
+ struct revindex_entry *reventry = &revidx->revindex[n];
+ const unsigned char *sha1 = nth_packed_object_sha1(p, reventry[0].nr);
+ const off_t offset_in_pack = sha1_index_find_offset(sha1);
+
+ if (!offset_in_pack) {
+ const off_t offset = packwriter_total(w);
+
+ enum object_type real_type = OBJ_BAD;
+ uint32_t crc32;
+ int rewrite_header;
+
+ packwriter_crc32_start(w);
+ rewrite_header = append_object_1(reventry, w, p, &w_curs, &real_type);
+ crc32 = packwriter_crc32_end(w);
+
+ if (!rewrite_header && crc32 != nth_packed_object_crc32(p, reventry[0].nr))
+ die("crc32 check failed for %s", sha1_to_hex(sha1));
+
+ add_to_write_list(sha1, offset, crc32, real_type);
+ } else {
+ add_to_skipped_list(reventry[0].offset, offset_in_pack);
+ }
+
+ display_progress(progress_state, n + 1);
+ }
+
+ stop_progress(&progress_state);
+ unuse_pack(&w_curs);
+ close_pack_windows(p);
+ close_pack_index(p);
+
+ sha1_index_update();
+ skipped_nr = 0;
+ reused_chunks_nr = 0;
+}
+
+static void write_packs(void)
+{
+ struct packwriter w;
+ unsigned int i;
+
+ packwriter_init(&w);
+ write_initial_packfile(all_packfiles[0], &w);
+
+ for (i = 1; i < all_packfiles_nr; ++i)
+ append_packfile(all_packfiles[i], &w);
+
+ /* finalize pack */
+ {
+ unsigned char sha1[20];
+ struct strbuf tmpname = STRBUF_INIT;
+
+ fixup_pack_header_footer(w.fd, sha1, w.tmp->filename.buf, written_nr, NULL, 0);
+ close(w.fd);
+
+ strbuf_addf(&tmpname, "%s-", base_name);
+
+ finish_tmp_packfile(&tmpname, w.tmp->filename.buf,
+ written_list, written_nr,
+ &pack_idx_opts, sha1);
+
+ if (write_bitmap_index) {
+ strbuf_addf(&tmpname, "%s.bitmap", sha1_to_hex(sha1));
+ bitmap_rewrite_existing(
+ all_packfiles[0],
+ written_list, written_nr,
+ packwriter_total(&w),
+ sha1, tmpname.buf);
+ }
+
+ strbuf_release(&tmpname);
+ puts(sha1_to_hex(sha1));
+ }
+}
+
+void pack_fast_grow_typemaps(struct packed_git *p, struct ewah_bitmap **typemaps)
+{
+ uint32_t n;
+ size_t pos = p->num_objects;
+ struct write_slab *slab = written_slab_root;
+
+ assert(slab->nr == p->num_objects);
+ assert(slab->next);
+ slab = slab->next;
+
+ while (slab) {
+ for (n = 0; n < slab->nr; ++n) {
+ const enum object_type real_type = slab->entries[n].real_type;
+ assert(real_type >= OBJ_COMMIT && real_type <= OBJ_TAG);
+ ewah_set(typemaps[real_type - 1], pos++);
+ }
+ slab = slab->next;
+ }
+}
+
+int cmd_pack_fast(int argc, const char **argv, const char *prefix)
+{
+ struct option pack_fast_options[] = {
+ OPT_SET_INT('q', "quiet", &progress,
+ N_("do not show progress meter"), 0),
+ OPT_SET_INT(0, "progress", &progress,
+ N_("show progress meter"), 1),
+ OPT_BOOL(0, "skip-largest", &skip_largest,
+ N_("do not pack the largest packfile in the repository")),
+ OPT_END(),
+ };
+
+ reset_pack_idx_option(&pack_idx_opts);
+ progress = isatty(2);
+ argc = parse_options(argc, argv, prefix, pack_fast_options,
+ pack_usage, 0);
+
+ if (argc) {
+ base_name = argv[0];
+ argc--;
+ }
+
+ find_packfiles();
+ write_packs();
+ return 0;
+}
diff --git a/cache.h b/cache.h
index 6f53962bf..1a13961bd 100644
--- a/cache.h
+++ b/cache.h
@@ -1336,6 +1336,11 @@ extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsign
extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
+extern off_t get_delta_base(struct packed_git *p,
+ struct pack_window **w_curs,
+ off_t *curpos,
+ enum object_type type,
+ off_t delta_obj_offset);
/*
* Iterate over the files in the loose-object parts of the object
diff --git a/git.c b/git.c
index 40f9df089..d81bd4469 100644
--- a/git.c
+++ b/git.c
@@ -440,6 +440,7 @@ static struct cmd_struct commands[] = {
{ "name-rev", cmd_name_rev, RUN_SETUP },
{ "notes", cmd_notes, RUN_SETUP },
{ "pack-objects", cmd_pack_objects, RUN_SETUP },
+ { "pack-fast", cmd_pack_fast, RUN_SETUP },
{ "pack-redundant", cmd_pack_redundant, RUN_SETUP },
{ "pack-refs", cmd_pack_refs, RUN_SETUP },
{ "patch-id", cmd_patch_id },
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index c05d1386a..449715f02 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -505,23 +505,39 @@ void bitmap_writer_set_checksum(unsigned char *sha1)
hashcpy(writer.pack_checksum, sha1);
}
+static struct sha1file *bitmap_file_new(char *tmp_file, size_t len)
+{
+ int fd = odb_mkstemp(tmp_file, len, "pack/tmp_bitmap_XXXXXX");
+
+ if (fd < 0)
+ die_errno("unable to create '%s'", tmp_file);
+
+ return sha1fd(fd, tmp_file);
+}
+
+static void bitmap_file_close(struct sha1file *f, const char *tmp_file, const char *dest)
+{
+ sha1close(f, NULL, CSUM_FSYNC);
+
+ if (adjust_shared_perm(tmp_file))
+ die_errno("unable to make temporary bitmap file readable");
+
+ if (rename(tmp_file, dest))
+ die_errno("unable to rename temporary bitmap file to '%s'", dest);
+}
+
void bitmap_writer_finish(struct pack_idx_entry **index,
uint32_t index_nr,
const char *filename,
uint16_t options)
{
- static char tmp_file[PATH_MAX];
static uint16_t default_version = 1;
static uint16_t flags = BITMAP_OPT_FULL_DAG;
+ char tmp_file[PATH_MAX];
struct sha1file *f;
-
struct bitmap_disk_header header;
- int fd = odb_mkstemp(tmp_file, sizeof(tmp_file), "pack/tmp_bitmap_XXXXXX");
-
- if (fd < 0)
- die_errno("unable to create '%s'", tmp_file);
- f = sha1fd(fd, tmp_file);
+ f = bitmap_file_new(tmp_file, sizeof(tmp_file));
memcpy(header.magic, BITMAP_IDX_SIGNATURE, sizeof(BITMAP_IDX_SIGNATURE));
header.version = htons(default_version);
@@ -539,11 +555,138 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
if (options & BITMAP_OPT_HASH_CACHE)
write_hash_cache(f, index, index_nr);
- sha1close(f, NULL, CSUM_FSYNC);
+ bitmap_file_close(f, tmp_file, filename);
+}
- if (adjust_shared_perm(tmp_file))
- die_errno("unable to make temporary bitmap file readable");
+static void *try_load_bitmap(struct packed_git *p, size_t *_size_out)
+{
+ void *reused_bitmap;
+ size_t reused_bitmap_size;
+
+ int fd;
+ struct stat st;
+ char *idx_name;
+
+ idx_name = pack_bitmap_filename(p);
+ fd = git_open_noatime(idx_name);
+ free(idx_name);
+
+ if (fd < 0)
+ return NULL;
+
+ if (fstat(fd, &st)) {
+ close(fd);
+ return NULL;
+ }
+
+ reused_bitmap_size = xsize_t(st.st_size);
+ reused_bitmap = xmmap(NULL, reused_bitmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ close(fd);
+
+ *_size_out = reused_bitmap_size;
+ return reused_bitmap;
+}
+
+extern void pack_fast_grow_typemaps(struct packed_git *p, struct ewah_bitmap **typemaps);
+
+static size_t rewrite_type_maps(struct sha1file *f,
+ struct packed_git *p, unsigned char *original_map, size_t original_size, size_t pos)
+{
+ struct ewah_bitmap *typemaps[4];
+ int r, i;
+
+ for (i = 0; i < 4; ++i) {
+ typemaps[i] = ewah_pool_new();
+ r = ewah_read_mmap(typemaps[i], original_map + pos, original_size - pos);
+ if (r < 0)
+ die("failed to read bitmap index");
+ pos += r;
+ }
+
+ pack_fast_grow_typemaps(p, typemaps);
+
+ for (i = 0; i < 4; ++i) {
+ dump_bitmap(f, typemaps[i]);
+ ewah_pool_free(typemaps[i]);
+ }
+
+ return pos;
+}
+
+static size_t rewrite_bitmaps(struct sha1file *f,
+ struct packed_git *p, unsigned char *original_map, size_t original_size, size_t pos,
+ uint32_t entry_count, struct pack_idx_entry **index, uint32_t index_nr)
+{
+ uint32_t i;
+
+ for (i = 0; i < entry_count; ++i) {
+ const unsigned char *sha1;
+ uint32_t src_idx, src_buffer_len, total_len;
+ int new_idx;
+
+ src_idx = get_be32(original_map + pos);
+ pos += 4;
+
+ sha1 = nth_packed_object_sha1(p, src_idx);
+ new_idx = sha1_pos(sha1, index, index_nr, sha1_access);
+ sha1write_be32(f, (uint32_t)new_idx);
+
+ src_buffer_len = get_be32(original_map + pos + 2 + 4);
+ total_len = (3 * 4) + (src_buffer_len * 8);
+
+ sha1write(f, original_map + pos, 2 + total_len);
+ pos += 2 + total_len;
+
+ if (pos > original_size)
+ die("unexpected end of file");
+ }
+
+ return pos;
+}
+
+void bitmap_rewrite_existing(
+ struct packed_git *p,
+ struct pack_idx_entry **index,
+ uint32_t index_nr,
+ off_t pack_offset,
+ const unsigned char *pack_sha1,
+ const char *filename)
+{
+ char tmp_file[PATH_MAX];
+ struct sha1file *f;
+
+ unsigned char *original_map;
+ size_t original_size, pos = 0;
+ struct bitmap_disk_header header;
+
+ original_map = try_load_bitmap(p, &original_size);
+ if (!original_map || original_size < sizeof(header) + 20)
+ return;
+
+ memcpy(&header, original_map, sizeof(header));
+ hashcpy(header.checksum, pack_sha1);
+
+ if (memcmp(header.magic, BITMAP_IDX_SIGNATURE, sizeof(BITMAP_IDX_SIGNATURE)) != 0)
+ die("existing bitmap for '%s' is corrupted", p->pack_name);
+
+ if (ntohs(header.version) != 1)
+ die("existing bitmap for '%s' has an unsupported version", p->pack_name);
+
+ f = bitmap_file_new(tmp_file, sizeof(tmp_file));
+
+ sha1write(f, &header, sizeof(header));
+ pos = sizeof(header);
+ pos = rewrite_type_maps(f, p, original_map, original_size, pos);
+ pos = rewrite_bitmaps(f, p, original_map, original_size, pos,
+ ntohl(header.entry_count), index, index_nr);
+
+ if (ntohs(header.options) & BITMAP_OPT_HASH_CACHE) {
+ uint32_t i, zero = 0;
+ sha1write(f, original_map + pos, p->num_objects * 4);
+ for (i = p->num_objects; i < index_nr; ++i)
+ sha1write(f, &zero, 4);
+ pos += (p->num_objects * 4);
+ }
- if (rename(tmp_file, filename))
- die_errno("unable to rename temporary bitmap file to '%s'", filename);
+ bitmap_file_close(f, tmp_file, filename);
}
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 637770af8..ee361fa6a 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -250,7 +250,7 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
return 0;
}
-static char *pack_bitmap_filename(struct packed_git *p)
+char *pack_bitmap_filename(struct packed_git *p)
{
char *idx_name;
int len;
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 0adcef77b..398523dbb 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -34,6 +34,7 @@ typedef int (*show_reachable_fn)(
struct packed_git *found_pack,
off_t found_offset);
+char *pack_bitmap_filename(struct packed_git *p);
int prepare_bitmap_git(void);
void count_bitmap_commit_list(uint32_t *commits, uint32_t *trees, uint32_t *blobs, uint32_t *tags);
void traverse_bitmap_commit_list(show_reachable_fn show_reachable);
@@ -53,5 +54,12 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
uint32_t index_nr,
const char *filename,
uint16_t options);
+void bitmap_rewrite_existing(
+ struct packed_git *p,
+ struct pack_idx_entry **index,
+ uint32_t index_nr,
+ off_t pack_offset,
+ const unsigned char *pack_sha1,
+ const char *filename);
#endif
diff --git a/sha1_file.c b/sha1_file.c
index 72289696d..bcd447f16 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1821,7 +1821,7 @@ unsigned long get_size_from_delta(struct packed_git *p,
return get_delta_hdr_size(&data, delta_head+sizeof(delta_head));
}
-static off_t get_delta_base(struct packed_git *p,
+off_t get_delta_base(struct packed_git *p,
struct pack_window **w_curs,
off_t *curpos,
enum object_type type,
@@ -1936,7 +1936,7 @@ static int retry_bad_packed_offset(struct packed_git *p, off_t obj_offset)
#define POI_STACK_PREALLOC 64
-static enum object_type packed_to_object_type(struct packed_git *p,
+enum object_type packed_to_object_type(struct packed_git *p,
off_t obj_offset,
enum object_type type,
struct pack_window **w_curs,
^ permalink raw reply related
* [PATCH] gc: ignore old gc.log files
From: David Turner @ 2017-02-09 2:02 UTC (permalink / raw)
To: git; +Cc: peff, pclouds, David Turner
The intent of automatic gc is to have a git repository be relatively
low-maintenance from a server-operator perspective. Of course, large
operators like GitHub will need a more complicated management strategy,
but for ordinary usage, git should just work.
In this commit, git learns to ignore gc.log files which are older than
(by default) one day old. It also learns about a config, gc.maxLogAge
to manage this.
So git should never get itself into a state where it refuses to do any
maintenance, just because at some point some piece of the maintenance
didn't make progress. That might still happen (e.g. because the repo
is corrupt), but at the very least it won't be because Git is too dumb
to try again.
Signed-off-by: David Turner <dturner@twosigma.com>
Helped-by: Jeff King <peff@peff.net>
---
Documentation/config.txt | 5 +++++
builtin/gc.c | 15 ++++++++++++++-
t/t6500-gc.sh | 13 +++++++++++++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index fc5a28a32..6751371cf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1402,6 +1402,11 @@ gc.autoDetach::
Make `git gc --auto` return immediately and run in background
if the system supports it. Default is true.
+gc.maxLogAge::
+ If the file gc.log exists, then `git gc --auto` won't run
+ unless that file is more than maxLogAge seconds old. Default
+ is 86400, one day.
+
gc.packRefs::
Running `git pack-refs` in a repository renders it
unclonable by Git versions prior to 1.5.1.2 over dumb
diff --git a/builtin/gc.c b/builtin/gc.c
index 331f21926..62fc84815 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -33,6 +33,7 @@ static int aggressive_window = 250;
static int gc_auto_threshold = 6700;
static int gc_auto_pack_limit = 50;
static int detach_auto = 1;
+static int gc_max_log_age_seconds = 86400;
static const char *prune_expire = "2.weeks.ago";
static const char *prune_worktrees_expire = "3.months.ago";
@@ -111,6 +112,7 @@ static void gc_config(void)
git_config_get_int("gc.auto", &gc_auto_threshold);
git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
git_config_get_bool("gc.autodetach", &detach_auto);
+ git_config_get_int("gc.maxlogage", &gc_max_log_age_seconds);
git_config_date_string("gc.pruneexpire", &prune_expire);
git_config_date_string("gc.worktreepruneexpire", &prune_worktrees_expire);
git_config(git_default_config, NULL);
@@ -291,8 +293,19 @@ static int report_last_gc_error(void)
{
struct strbuf sb = STRBUF_INIT;
int ret;
+ struct stat st;
+ const char *gc_log_path = git_path("gc.log");
+
+ if (stat(gc_log_path, &st)) {
+ if (errno == ENOENT)
+ return 0;
+ return error(_("Can't read %s"), gc_log_path);
+ }
+
+ if (time(NULL) - st.st_mtime > gc_max_log_age_seconds)
+ return 0;
- ret = strbuf_read_file(&sb, git_path("gc.log"), 0);
+ ret = strbuf_read_file(&sb, gc_log_path, 0);
if (ret > 0)
return error(_("The last gc run reported the following. "
"Please correct the root cause\n"
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 1762dfa6a..b69c2c190 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,5 +67,18 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
test_line_count = 2 new # There is one new pack and its .idx
'
+test_expect_success 'background auto gc does not run if gc.log is present and recent but does if it is old' '
+ keep=$(ls .git/objects/pack/*.pack|head -1|sed -e "s/pack$/keep/") &&
+ test_commit foo &&
+ test_commit bar &&
+ git repack &&
+ test_config gc.autopacklimit 1 &&
+ test_config gc.autodetach true &&
+ echo fleem> .git/gc.log &&
+ test_must_fail git gc --auto 2>err &&
+ test_i18ngrep "^error:" err &&
+ test-chmtime =-86401 .git/gc.log &&
+ git gc --auto
+'
test_done
--
2.11.GIT
^ permalink raw reply related
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