* [PATCH] clear_delta_base_cache(): don't modify hashmap while iterating
From: Jeff King @ 2017-01-19 16:33 UTC (permalink / raw)
To: Ulrich Spörlein; +Cc: Ed Maste, git, Junio C Hamano
In-Reply-To: <CAJ9axoT1pUQc_jTKxO+RMw7emhA4ss1NCAU+hpnyG5LMwGD89w@mail.gmail.com>
On Thu, Jan 19, 2017 at 03:03:46PM +0100, Ulrich Spörlein wrote:
> > I suspect the patch below may fix things for you. It works around it by
> > walking over the lru list (either is fine, as they both contain all
> > entries, and since we're clearing everything, we don't care about the
> > order).
>
> Confirmed. With the patch applied, I can import the whole 55G in one go
> without any crashes or aborts. Thanks much!
Thanks. Here it is rolled up with a commit message.
-- >8 --
Subject: clear_delta_base_cache(): don't modify hashmap while iterating
Removing entries while iterating causes fast-import to
access an already-freed `struct packed_git`, leading to
various confusing errors.
What happens is that clear_delta_base_cache() drops the
whole contents of the cache by iterating over the hashmap,
calling release_delta_base_cache() on each entry. That
function removes the item from the hashmap. The hashmap code
may then shrink the table, but the hashmap_iter struct
retains an offset from the old table.
As a result, the next call to hashmap_iter_next() may claim
that the iteration is done, even though some items haven't
been visited.
The only caller of clear_delta_base_cache() is fast-import,
which wants to clear the cache because it is discarding the
packed_git struct for its temporary pack. So by failing to
remove all of the entries, we still have references to the
freed packed_git.
To make things even more confusing, this doesn't seem to
trigger with the test suite, because it depends on
complexities like the size of the hash table, which entries
got cleared, whether we try to access them before they're
evicted from the cache, etc.
So I've been able to identify the problem with large
imports like freebsd's svn import, or a fast-export of
linux.git. But nothing that would be reasonable to run as
part of the normal test suite.
We can fix this easily by iterating over the lru linked list
instead of the hashmap. They both contain the same entries,
and we can use the "safe" variant of the list iterator,
which exists for exactly this case.
Let's also add a warning to the hashmap API documentation to
reduce the chances of getting bit by this again.
Reported-by: Ulrich Spörlein <uqs@freebsd.org>
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/technical/api-hashmap.txt | 4 +++-
sha1_file.c | 9 ++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt
index 28f5a8b71..a3f020cd9 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -188,7 +188,9 @@ Returns the removed entry, or NULL if not found.
`void *hashmap_iter_next(struct hashmap_iter *iter)`::
`void *hashmap_iter_first(struct hashmap *map, struct hashmap_iter *iter)`::
- Used to iterate over all entries of a hashmap.
+ Used to iterate over all entries of a hashmap. Note that it is
+ not safe to add or remove entries to the hashmap while
+ iterating.
+
`hashmap_iter_init` initializes a `hashmap_iter` structure.
+
diff --git a/sha1_file.c b/sha1_file.c
index 1eb47f611..d20714d6b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2342,11 +2342,10 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
void clear_delta_base_cache(void)
{
- struct hashmap_iter iter;
- struct delta_base_cache_entry *entry;
- for (entry = hashmap_iter_first(&delta_base_cache, &iter);
- entry;
- entry = hashmap_iter_next(&iter)) {
+ struct list_head *lru, *tmp;
+ list_for_each_safe(lru, tmp, &delta_base_cache_lru) {
+ struct delta_base_cache_entry *entry =
+ list_entry(lru, struct delta_base_cache_entry, lru);
release_delta_base_cache(entry);
}
}
--
2.11.0.747.g2c5918130
^ permalink raw reply related
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Johannes Schindelin @ 2017-01-19 16:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Jeff King, git
In-Reply-To: <xmqqfukg9o7u.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Wed, 18 Jan 2017, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
>
> > Am 17.01.2017 um 20:17 schrieb Junio C Hamano:
> >> So... can we move this forward?
> >
> > I have no objects anymore.
>
> Alright, thanks.
>
> Dscho what's your assessment?
I still think it will be a problem. I'll address that problem when I get
bug reports, to unblock you.
Ciao,
Johannes
^ permalink raw reply
* Re: [RFC] stash --continue
From: Johannes Schindelin @ 2017-01-19 15:54 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Marc Branchaud, git
In-Reply-To: <e273c7c8-278a-061c-44fd-4808b53d0475@gmx.net>
Hi Stephan,
On Wed, 18 Jan 2017, Stephan Beyer wrote:
> PPS: Any opinions about the mentioned "backwards-compatibility" issue
> that people are then forced to finish their commits with "--continue"
> instead of "git reset" or "git commit"?
Maybe you could make it so that "git reset" and "git commit" would still
work as before, and reset the state so that "git stash pop --continue"
could complain that there is nothing to continue?
Similar to how we remove CHERRY_PICK_HEAD during a `git reset --hard`,
maybe?
Ciao,
Johannes
^ permalink raw reply
* Re: [RFC] stash --continue
From: Johannes Schindelin @ 2017-01-19 15:49 UTC (permalink / raw)
To: Marc Branchaud; +Cc: Stephan Beyer, git
In-Reply-To: <38d592b8-975c-1fd9-4c42-877e34a4ab70@xiplink.com>
Hi Marc,
On Wed, 18 Jan 2017, Marc Branchaud wrote:
> On 2017-01-18 11:34 AM, Johannes Schindelin wrote:
> >
> > On Wed, 18 Jan 2017, Marc Branchaud wrote:
> >
> > > On 2017-01-16 05:54 AM, Johannes Schindelin wrote:
> > >
> > > > On Mon, 16 Jan 2017, Stephan Beyer wrote:
> > > >
> > > > > a git-newbie-ish co-worker uses git-stash sometimes. Last time
> > > > > he used "git stash pop", he got into a merge conflict. After he
> > > > > resolved the conflict, he did not know what to do to get the
> > > > > repository into the wanted state. In his case, it was only "git
> > > > > add <resolved files>" followed by a "git reset" and a "git stash
> > > > > drop", but there may be more involved cases when your index is
> > > > > not clean before "git stash pop" and you want to have your index
> > > > > as before.
> > > > >
> > > > > This led to the idea to have something like "git stash
> > > > > --continue"[1]
> > > >
> > > > More like "git stash pop --continue". Without the "pop" command,
> > > > it does not make too much sense.
> > >
> > > Why not? git should be able to remember what stash command created
> > > the conflict. Why should I have to? Maybe the fire alarm goes off
> > > right when I run the stash command, and by the time I get back to it
> > > I can't remember which operation I did. It would be nice to be able
> > > to tell git to "just finish off (or abort) the stash operation,
> > > whatever it was".
> >
> > That reeks of a big potential for confusion.
> >
> > Imagine for example a total Git noob who calls `git stash list`,
> > scrolls two pages down, then hits `q` by mistake. How would you
> > explain to that user that `git stash --continue` does not continue
> > showing the list at the third page?
>
> Sorry, but I have trouble taking that example seriously. It assumes
> such a level of "noobness" that the user doesn't even understand how
> standard command output paging works, not just with git but with any
> shell command.
Yeah, well, I thought you understood what I meant.
The example was the best I could come up with quickly, and it only tried
to show that there are *other* stash operations that one might perceive
to happen at the same time as the "pop" operation, so your flimsical
comment "why not continue the latest operation" may very well be
ambiguous.
And if it is not ambiguous in "stash", it certainly will be in other Git
operations. And therefore, having a DWIM in "stash" to allow "--continue"
without any specific subcommand, but not having it in other Git commands,
is just a very poor user interface design. It is prone to confuse users,
which is always a hallmark of a bad user interface.
Hence my objection to "git stash --continue". No argument in favor of "git
stash --continue" I heard so far comes even close to being convincing.
> > Even worse: `git stash` (without arguments) defaults to the `save`
> > operation, so any user who does not read the documentation (and who
> > does?) would assume that `git stash --continue` *also* implies `save`.
>
> Like the first example, your user is trying to "continue" a command that
> is already complete.
Says who? You may understand the semantics better than other users, but
who are you to judge?
But that's besides the point.
My point (which you did not quite understand) was that it can be ambiguous
what to continue when looking at *all* Git commands. To special-case "git
stash"'s user interface makes things more confusing, and therefore less
usable for everyone.
And even with `git stash apply`, you could construct a very plausible
scenario (which does not work yet, but we may want to make it work): if
`git stash apply` causes conflicts, and `git stash apply stash@{1}`
conflicts in a *different* set of files, why don't we allow the second
operation to succeed (adding its conflicts)?
That example is like `git cherry-pick -n` with two different commits, both
of which conflict with the current worktree, but in different files. Both
cherry-picks would do their job if called after one another, and the
result is a worktree with the *combined* conflicts. That is a legitimate
use case (which I happened to *actually* perform just the other day).
If we fix "git stash" (and there is no reason we should not), it would
also allow "git stash pop; git stash pop" to work with two stashes that
both conflict with the current worktree, just in different files.
So I challenge you to get less hung up on the *exact* example I present,
and to try to see through the example what the issue is that I am trying
to get at.
> > If that was not enough, there would still be the overall design of
> > Git's user interface. You can call it confusing, inconsistent, with a
> > lot of room for improvement, and you would be correct. But none of
> > Git's commands has a `--continue` option that remembers the latest
> > subcommand and continues that. To introduce that behavior in `git
> > stash` would disimprove the situation.
>
> I think it's more the case that none of the current continuable commands
> have subcommands (though I can't think of all the continuable or
> abortable operations offhand, so maybe I'm wrong). I think we're
> discussing new UI ground here.
Nope, we are not entering new UI ground here. The principle is clear with
the existing --continue options: you pass them to the same operation that
you want to continue. By that reasoning, "git stash --continue" should
continue the (implicit) "save" operation. But that is not at all what you
want.
> And since the pattern is already "git foo --continue",
But foo *is the operation*! By that reasoning, you should agree that "git
stash --continue" is *wrong*!
> Think of it this way: All the currently continuable/abortable commands
> put the repository in a shaky state, where performing certain other
> operations would be ill advised. Attempting to start a rebase while a
> merge conflict is unresolved, for example. IIRC, git actually tries to
> stop users from shooting their feet in this way.
>
> And so it should be for the stash operation: If applying a stash yields
> a conflict, it has to be resolved or aborted before something like a
> rebase or merge is attempted.
That already happens, and I have no idea how you think this safe-guarding
has anything to do whether the "--continue" option makes sense in "git
stash", or only in "git stash pop".
> In the long run, I think there's even the possibility of generic "git
> continue" and "git abort" commands,
Wrong.
You can call "git cherry-pick" (and "git cherry-pick --continue") while
running a "git rebase -i".
You can run "git rebase", "git stash", "git cherry-pick" and many other
commands while running a "git bisect".
You can even run a "git rebase" or a "git cherry-pick" while resolving an
interrupted "git am".
Many, many examples that make it *impossible* for Git to know *what* you
want to continue, *what* you want to abort.
> > At least `git stash pop --continue` would be consistent with all other
> > `--continue` options in Git that I can think of...
>
> Alas, I disagree!
Sure, you are free to disagree.
And syntactially, you are even correct: "git <something> <something-else>
--continue" is inconsistent with "git <something> --continue".
But semantically, i.e. when you look at the *meaning* of those
"<something>"s, you are incorrect: the "--continue" option always goes
with the *operation* that needs to be continued. Always, always, always.
If you continue a rebase, it is "git rebase --continue", *not* "git
--continue". If you continue a revert, it is "git revert --continue". And
so it should be for popping a stash: "git stash pop --continue". Because
the operation is specified by "git stash pop", not by "git stash". There
is really not much you can argue sanely about that.
Ciao,
Johannes
^ permalink raw reply
* [RFC 1/2] grep: only add delimiter if there isn't one already
From: Stefan Hajnoczi @ 2017-01-19 15:03 UTC (permalink / raw)
To: git; +Cc: gitster, Stefan Hajnoczi
In-Reply-To: <20170119150347.3484-1-stefanha@redhat.com>
git-grep(1) output does not follow git's own syntax:
$ git grep malloc v2.9.3:
v2.9.3::Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc
$ git show v2.9.3::Makefile
fatal: Path ':Makefile' does not exist in 'v2.9.3'
This patch avoids emitting the unnecessary ':' delimiter if the name
already ends with ':' or '/':
$ git grep malloc v2.9.3:
v2.9.3:Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc
$ git show v2.9.3:Makefile
(succeeds)
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
builtin/grep.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 462e607..3643d8a 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -490,7 +490,11 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
strbuf_init(&base, PATH_MAX + len + 1);
if (len) {
strbuf_add(&base, name, len);
- strbuf_addch(&base, ':');
+
+ /* Add a delimiter if there isn't one already */
+ if (name[len - 1] != '/' && name[len - 1] != ':') {
+ strbuf_addch(&base, ':');
+ }
}
init_tree_desc(&tree, data, size);
hit = grep_tree(opt, pathspec, &tree, &base, base.len,
--
2.9.3
^ permalink raw reply related
* [RFC 2/2] grep: use '/' delimiter for paths
From: Stefan Hajnoczi @ 2017-01-19 15:03 UTC (permalink / raw)
To: git; +Cc: gitster, Stefan Hajnoczi
In-Reply-To: <20170119150347.3484-1-stefanha@redhat.com>
If the tree contains a sub-directory then git-grep(1) output contains a
colon character instead of a path separator:
$ git grep malloc v2.9.3:t
v2.9.3:t:test-lib.sh: setup_malloc_check () {
$ git show v2.9.3:t:test-lib.sh
fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
This patch attempts to use the correct delimiter:
$ git grep malloc v2.9.3:t
v2.9.3:t/test-lib.sh: setup_malloc_check () {
$ git show v2.9.3:t/test-lib.sh
(success)
This patch does not cope with @{1979-02-26 18:30:00} syntax and treats
it as a path because it contains colons.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
builtin/grep.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 3643d8a..06f8b47 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -493,7 +493,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
/* Add a delimiter if there isn't one already */
if (name[len - 1] != '/' && name[len - 1] != ':') {
- strbuf_addch(&base, ':');
+ /* rev: or rev:path/ */
+ strbuf_addch(&base, strchr(name, ':') ? '/' : ':');
}
}
init_tree_desc(&tree, data, size);
--
2.9.3
^ permalink raw reply related
* [RFC 0/2] grep: make output consistent with revision syntax
From: Stefan Hajnoczi @ 2017-01-19 15:03 UTC (permalink / raw)
To: git; +Cc: gitster, Stefan Hajnoczi
git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax.
This means you cannot take "rev:path/to/file.c: foo();" output from git-grep(1)
and expect "git show rev:path/to/file.c" to work. See the individual patches
for examples of command-lines that produce invalid output.
This series is an incomplete attempt at solving the issue. I'm not familiar
enough with the git codebase to propose a better solution. Perhaps someone is
interested in a proper fix?
Stefan Hajnoczi (2):
grep: only add delimiter if there isn't one already
grep: use '/' delimiter for paths
builtin/grep.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
--
2.9.3
^ permalink raw reply
* Re: The design of refs backends, linked worktrees and submodules
From: Michael Haggerty @ 2017-01-19 13:30 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8CHoroX2k9GqOFmXkvvPCPN4SBeCg+6aC2WSWNSKVmWQw@mail.gmail.com>
On 01/19/2017 12:55 PM, Duy Nguyen wrote:
> I've started working on fixing the "git gc" issue with multiple
> worktrees, which brings me back to this. Just some thoughts. Comments
> are really appreciated.
>
> In the current code, files backend has special cases for both
> submodules (explicitly) and linked worktrees (hidden behind git_path).
There is another terrible hack also needed to implement linked
worktrees, namely that the `refs/bisect/` hierarchy is manually inserted
into the `ref_cache`, because otherwise it wouldn't be noticed when
iterating over loose references via `readdir()`.
Other similar hacks would be required if other reference subtrees are
declared to be per-worktree.
> But if a backend has to handle this stuff, all future backends have to
> too. Which does not sound great. Imagine we have "something" in
> addition to worktrees and submodules in future, then all backends have
> to learn about it.
Agreed, the status quo is not pretty.
I kindof think that it would have been a better design to store the
references for all linked worktrees in the main repository's ref-store.
For example, the "bisect" refs for a worktree named "<name>" could have
been stored under "refs/worktrees/<name>/bisect/*". Then either:
* teach the associated tools to read/write references there directly
(probably with DWIM rules to make command-line use easier), or
* treat these references as if they were actually at a standard place
like `refs/worktree/bisect/*`; i.e., users would need to know that they
were per-worktree references, but wouldn't need to worry about the true
locations, or
* treat these references as if they were actually in their traditional
locations (though it is not obvious how this scheme could be expanded to
cover new per-worktree references).
> So how about we move worktree and submodule support back to front-end?
>
> The backend deals with refs, period. The file-based backend will be
> given a directory where refs live in and it work on that. Backends do
> not use git_path(). Backends do not care about $GIT_DIR. Access to odb
> (e.g. sha-1 validation) if needed is abstracted out via a set of
> callbacks. This allows submodules to give access to submodule's
> separate odb. And it's getting close to the "struct repository"
> mentioned somewhere in refs "TODO" comments, even though we are
> nowhere close to introducing that struct.
This is a topic that I have thought a lot about. I definitely like this
direction. In fact I've dabbled around with some first steps; see branch
`submodule-hash` in my fork on GitHub [1]. That branch associates a
`ref_store` more closely with the directory where the references are
stored, as opposed to having a 1:1 relationship between `ref_store`s and
submodules.
I would like to see the separation of a concept "iterate over all
reachability roots" that is independent of other ref iteration. Then it
wouldn't have to include reference names, except basically for use in
error messages. So for linked worktrees, in place of the reference name
it might emit a string like "<worktree>:<refname>". (Of course it would
get its information by iterating over all of the linked reference stores
using their reference iteration APIs.)
> For that to work, I'll probably need to add a "composite" ref_store
> that combines two file-based backends (for per-repo and per-worktree
> refs) to represent a unified ref store. I think your work on ref
> iterator makes way for that. A bit worried about transactions though,
> because I think per-repo and per-worktree updates will be separated in
> two transactions. But that's probably ok because future backends, like
> lmdb, will have to go through the same route anyway.
Yes, that was the main motivation for the ref-iterator work.
Regarding transactions, the commit pointed at by branch
`split-transaction` in my fork shows how I think the
`transaction_commit()` method could be split into two parts,
`transaction_prepare()` and `transaction_finish()`. The idea would be
that the driver function, `ref_transaction_commit()`, calls
`transaction_prepare()` for each `ref_store` involved in the
transaction, passing each one the reference updates for references that
live in that reference store. Those methods would verify that the part
of the transaction that lives in that ref-store "should" go through,
without actually committing anything. Then `transaction_finish()` would
be called on each ref store, and that method would commit the updates.
You probably couldn't get a bulletproof kind of compound transaction out
of this (e.g., if the computer's power goes out, one `ref_store`'s
updates might be committed but another one's not). But it would probably
be good enough to cover everyday reasons for transaction failures, like
pre-checksums not matching up.
Let me braindump some more information about this topic. A files backend
for a repository (ignoring submodules for the moment) currently consists
of five interacting parts, each of which looks a lot like a ref-store
itself:
* A loose reference ref-store for the main repo
* A loose reference ref-store for the per-subtree references
* A ref_cache in front of the two loose reference stores
* A packed ref-store
* A second ref_cache in front of the packed ref-store
But these ref-stores are currently coupled very tightly and have
peculiarities:
* The caching code is tightly coupled to the ref-store behind it.
* It is hard to imagine a packed refs-store that doesn't have some kind
of caching in front of it.
* There are tricky ordering constraints between writes to packed and
loose references to avoid races.
* The packed ref-store can't store symbolic refs, nor can it store
reflogs. It currently relies on the loose ref-store for those things.
* There is no packed-refs ref-store associated with per-worktree refs.
* Packed references are currently locked via `*.lock` files located near
the corresponding loose references.
* There are constraints that span refstores. For example, you aren't
allowed to create a packed ref that D/F conflicts with a loose ref or
vice versa.
* Symrefs, which are loose, can point at packed references.
I've taken some stabs at picking these apart into separate ref stores,
but haven't had time to make very satisfying progress. By the time of
GitMerge I might have a better feeling for whether I can devote some
time to this project.
Michael
[1] https://github.com/mhagger/git
^ permalink raw reply
* Re: merge maintaining history
From: David J. Bakeman @ 2017-01-19 13:12 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xoF8E55-XDnQT-GN1=hEwwq4pOsz7--P-SCy29C7ST3Hg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2031 bytes --]
On 01/14/2017 10:24 PM, Jacob Keller wrote:
> On Fri, Jan 13, 2017 at 6:01 PM, David J. Bakeman <nakuru@comcast.net> wrote:
>> History
>>
>> git cloned a remote repository and made many changes pushing them all to
>> said repository over many months.
>>
>> The powers that be then required me to move project to new repository
>> server did so by pushing local version to new remote saving all history!
>>
>> Now have to merge back to original repository(which has undergone many
>> changes since I split off) but how do I do that without loosing the
>> history of all the commits since the original move? Note I need to push
>> changes to files that are already in existence. I found on the web a
>> bunch of ways to insert a whole new directory structure into an existing
>> repository but as I said I need to do it on top of existing files. Of
>> course I can copy all the files from my local working repository to the
>> cloned remote repository and commit any changes but I loose all the
>> history that way.
>>
>> Thanks.
> If I understand it.. you have two remotes now:
>
> The "origin" remote, which was the original remote you started with.
>
> You have now a "new" remote which you created and pushed to.
>
> So you want to merge the "new" history into the original tree now, so
> you checkout the original tree, then "git merge <new-remote>/<branch>"
> and then fix up any conflicts, and then git commit to create a merge
> commit that has the new history. Then you could push that to both
> trees.
>
> I would want a bit more information about your setup before providing
> actual commands.
Thanks I think that's close but it's a little more complicated I think
:<( I don't know if this diagram will work but lets try.
original A->B->C->D->E->F
\
first branch b->c->d->e
new repo e->f->g->h
Now I need to merge h to F without loosing b through h hopefully. Yes e
was never merged back to the original repo and it's essentially gone now
so I can't just merge to F or can I?
>
> Thanks,
> Jake
>
[-- Attachment #2: nakuru.vcf --]
[-- Type: text/x-vcard, Size: 248 bytes --]
begin:vcard
fn:David J. Bakeman
n:Bakeman;David J.
org:Nakuru Software Inc.
adr:;;1504 North 57th Street;Seattle;WA;98103;USA
email;internet:nakuru@comcast.net
tel;work:(206)545-0609
tel;fax:(206)600-6957
x-mozilla-html:TRUE
version:2.1
end:vcard
^ permalink raw reply
* [PATCH] log: new option decorate reflog of remote refs
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 12:26 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This is most useful when you fork your branches off a remote ref and
rely on ref decoration to show your fork points in `git log`. Then you
do a "git fetch" and suddenly the remote decoration is gone because
remote refs are moved forward. With this, we can still see something
like "origin/foo@{1}"
This is for remote refs only because based on my experience, docorating
local reflog is just too noisy. You will most likely see HEAD@{1},
HEAD@{2} and so on. We can add that as a separate option in future if we
see a need for it.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I've been using this for many weeks and it has proven its usefulness
(to me). Looks like good material to send upstream.
Documentation/git-log.txt | 5 +++++
builtin/log.c | 10 +++++++++-
log-tree.c | 43 +++++++++++++++++++++++++++++++++++++++----
log-tree.h | 2 +-
pretty.c | 4 ++--
revision.c | 2 +-
6 files changed, 57 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 32246fd..f5ee575 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -38,6 +38,11 @@ OPTIONS
are shown as if 'short' were given, otherwise no ref names are
shown. The default option is 'short'.
+--decorate-remote-reflog[=<n>]::
+ Decorate `<n>` most recent reflog entries on remote refs, up
+ to the specified number of entries. By default, only the most
+ recent reflog entry is decorated.
+
--source::
Print out the ref name given on the command line by which each
commit was reached.
diff --git a/builtin/log.c b/builtin/log.c
index 55d20cc..c208703 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -36,6 +36,7 @@ static int default_follow;
static int default_show_signature;
static int decoration_style;
static int decoration_given;
+static int decorate_remote_reflog;
static int use_mailmap_config;
static const char *fmt_patch_subject_prefix = "PATCH";
static const char *fmt_pretty;
@@ -141,6 +142,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
OPT_BOOL(0, "use-mailmap", &mailmap, N_("Use mail map file")),
{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate options"),
PARSE_OPT_OPTARG, decorate_callback},
+ { OPTION_INTEGER, 0, "decorate-remote-reflog",
+ &decorate_remote_reflog, N_("n"),
+ N_("decorate the last <n> reflog entries of remote refs"),
+ PARSE_OPT_OPTARG | PARSE_OPT_NONEG, NULL, 1 },
OPT_CALLBACK('L', NULL, &line_cb, "n,m:file",
N_("Process line range n,m in file, counting from 1"),
log_line_range_callback),
@@ -195,9 +200,12 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
rev->abbrev_commit = 0;
}
+ if (decorate_remote_reflog > 0 && !decoration_style)
+ decoration_style = DECORATE_SHORT_REFS;
if (decoration_style) {
rev->show_decorations = 1;
- load_ref_decorations(decoration_style);
+ load_ref_decorations(decoration_style,
+ decorate_remote_reflog);
}
if (rev->line_level_traverse)
diff --git a/log-tree.c b/log-tree.c
index 8c24157..3d85ebc 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -88,14 +88,37 @@ const struct name_decoration *get_name_decoration(const struct object *obj)
return lookup_decoration(&name_decoration, obj);
}
+struct reflog_cb {
+ int type;
+ int count;
+ int nth;
+ const char *refname;
+};
+
+static int add_nth_reflog(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct reflog_cb *cb = cb_data;
+ struct commit *commit;
+
+ commit = lookup_commit(nsha1);
+ if (commit && cb->nth) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "%s@{%d}", cb->refname, cb->nth);
+ add_name_decoration(cb->type, sb.buf, &commit->object);
+ strbuf_release(&sb);
+ }
+ cb->nth++;
+ return cb->nth >= cb->count;
+}
+
static int add_ref_decoration(const char *refname, const struct object_id *oid,
int flags, void *cb_data)
{
struct object *obj;
enum decoration_type type = DECORATION_NONE;
- assert(cb_data == NULL);
-
if (starts_with(refname, git_replace_ref_base)) {
struct object_id original_oid;
if (!check_replace_refs)
@@ -135,6 +158,17 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
parse_object(obj->oid.hash);
add_name_decoration(DECORATION_REF_TAG, refname, obj);
}
+
+ if (cb_data && type == DECORATION_REF_REMOTE) {
+ struct reflog_cb cb;
+
+ memset(&cb, 0, sizeof(cb));
+ cb.refname = refname;
+ cb.type = type;
+ cb.count = *(int *)cb_data + 1 /* for @{0} */;
+
+ for_each_reflog_ent_reverse(refname, add_nth_reflog, &cb);
+ }
return 0;
}
@@ -147,13 +181,14 @@ static int add_graft_decoration(const struct commit_graft *graft, void *cb_data)
return 0;
}
-void load_ref_decorations(int flags)
+void load_ref_decorations(int flags, int remote_reflog)
{
if (!decoration_loaded) {
+ void *cb = remote_reflog ? &remote_reflog : NULL;
decoration_loaded = 1;
decoration_flags = flags;
- for_each_ref(add_ref_decoration, NULL);
+ for_each_ref(add_ref_decoration, cb);
head_ref(add_ref_decoration, NULL);
for_each_commit_graft(add_graft_decoration, NULL);
}
diff --git a/log-tree.h b/log-tree.h
index c8116e6..bb46c53 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -25,7 +25,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
const char **subject_p,
const char **extra_headers_p,
int *need_8bit_cte_p);
-void load_ref_decorations(int flags);
+void load_ref_decorations(int flags, int reflog);
#define FORMAT_PATCH_NAME_MAX 64
void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);
diff --git a/pretty.c b/pretty.c
index 5e68383..ec8e1cc 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1189,11 +1189,11 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
strbuf_addstr(sb, get_revision_mark(NULL, commit));
return 1;
case 'd':
- load_ref_decorations(DECORATE_SHORT_REFS);
+ load_ref_decorations(DECORATE_SHORT_REFS, 0);
format_decorations(sb, commit, c->auto_color);
return 1;
case 'D':
- load_ref_decorations(DECORATE_SHORT_REFS);
+ load_ref_decorations(DECORATE_SHORT_REFS, 0);
format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
return 1;
case 'g': /* reflog info */
diff --git a/revision.c b/revision.c
index b37dbec..4d5cbf5 100644
--- a/revision.c
+++ b/revision.c
@@ -1743,7 +1743,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->simplify_by_decoration = 1;
revs->limited = 1;
revs->prune = 1;
- load_ref_decorations(DECORATE_SHORT_REFS);
+ load_ref_decorations(DECORATE_SHORT_REFS, 0);
} else if (!strcmp(arg, "--date-order")) {
revs->sort_order = REV_SORT_BY_COMMIT_DATE;
revs->topo_order = 1;
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Duy Nguyen @ 2017-01-19 12:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, git, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <xmqqvatouwsh.fsf@gitster.mtv.corp.google.com>
On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Christian Couder <christian.couder@gmail.com> writes:
>>>
>>>> So what should we do if freshen_file() returns 0 which means that the
>>>> freshening failed?
>>>
>>> You tell me ;-) as you are the one who is proposing this feature.
>>
>> My answer is, we are not worse than freshening loose objects case
>> (especially since I took the idea from there).
>
> I do not think so, unfortunately. Loose object files with stale
> timestamps are not removed as long as objects are still reachable.
But there are plenty of unreachable loose objects, added in index,
then got replaced with new versions. cache-tree can create loose trees
too and it's been run more often, behind user's back, to take
advantage of the shortcut in unpack-trees.
> For the base/shared index file, the timestamp is the only thing that
> protects them from pruning, unless it is serving as the base file
> for the currently active $GIT_DIR/index that is split.
--
Duy
^ permalink raw reply
* Re: [PATCH v5 0/4] Per-worktree config file support
From: Duy Nguyen @ 2017-01-19 12:09 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Junio C Hamano, Michael J Gruber,
Jens Lehmann, Lars Schneider, Michael Haggerty, Max Kirillov
In-Reply-To: <CAGZ79ka+zkr83tSkg_kJWoN1u3fgu1O3u1-7USEoSM1tj-53vA@mail.gmail.com>
On Wed, Jan 11, 2017 at 12:01 AM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Jan 10, 2017 at 3:25 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> Let's get this rolling again. To refresh your memory because it's half
>> a year since v4 [1], this is about letting each worktree in multi
>> worktree setup has their own config settings. The most prominent ones
>> are core.worktree, used by submodules, and core.sparseCheckout.
>
> Thanks for getting this rolling again.
>
>>
>> This time I'm not touching submodules at all. I'm leaving it in the
>> good hands of "submodule people". All I'm providing is mechanism. How
>> you use it is up to you. So the series benefits sparse checkout users
>> only.
>
> As one of the "submodule people", I have no complaints here.
>
>>
>> Not much has changed from v4, except that the migration to new config
>> layout is done automatically _update_ a config variable with "git
>> config --worktree".
>>
>> I think this one is more or less ready. I have an RFC follow-up patch
>> about core.bare, but that could be handled separately.
>
> I have read through the series and think the design is sound for worktrees
> (though I have little knowledge about them).
Submodules and multi worktrees start to look very similar, the more I
think about it. Well, except that multi worktree does not separate odb
and config files, maybe. And we have already seen both have a need to
share code (like the moving .git dir operation). I suspect I'll learn
more about submodules along the way, and you worktrees ;-)
> Now even further:
>
> So to build on top of this series, I'd like to make submodules usable
> with worktrees (i.e. shared object store, only clone/fetch once and
> all worktrees
> benefit from it), the big question is how to get the underlying data
> model right.
>
> Would a submodule go into the superprojects
>
> .git/worktrees/<worktree-name>/modules/<submodule-name>/
>
> or rather
>
> .git/modules<submodule-name>/worktrees/<worktree-name>
>
> Or both (one of them being a gitlink file pointing at the other?)
>
> I have not made up my mind, as I haven't laid out all cases that are
> relevant here.
I would go with a conservative step first, keep submodules
per-worktree. After it's sorted out. You can change the layout (e.g.
as a config extension). The latter probably has some complication (but
yeah sharing would be a big plus).
--
Duy
^ permalink raw reply
* Re: [PATCH/RFC 5/4] Redefine core.bare in multiple working tree setting
From: Duy Nguyen @ 2017-01-19 12:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqh953j2q0.fsf@gitster.mtv.corp.google.com>
Thanks. I'll shelve this for now, maybe sleep on it for a while. The
series is complete without this patch by the way, if you want to pick
it up.
On Fri, Jan 13, 2017 at 6:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> With per-worktree configuration in place, core.bare is moved to main
>> worktree's private config file. But it does not really make sense
>> because this is about _repository_. Instead we could leave core.bare in
>> the per-repo config and change/extend its definition from:
>>
>> If true this repository is assumed to be 'bare' and has no working
>> directory associated with it.
>>
>> to
>>
>> If true this repository is assumed to be 'bare' and has no _main_
>> working directory associated with it.
>>
>> In other words, linked worktrees are not covered by core.bare. This
>> definition is the same as before when it comes to single worktree setup.
>
> Up to this point, I think it is not _wrong_ per-se, but it does not
> say anything about secondary worktrees. Some may have their own
> working tree, others may be bare, and there is no way for programs
> to discover if a particular secondary worktree has or lacks its own
> working tree.
>
> Granted, "git worktree" porcelain may be incapable of creating a
> secondary worktree without a working tree, but I think the
> underlying repository layout still is capable of expressing such a
> secondary worktree.
>
> So there still is something else necessary, I suspect, to make the
> definition complete. Perhaps core.bare should be set in
> per-worktree configuration for all worktrees including the primary
> one, and made the definition/explanation of core.bare to be
> "definition of this variable, if done, must be done in per-worktree
> config file. If set to true, the worktree is 'bare' and has no
> working directory associated with it"? That makes things even more
> equal, as there is truly no "special one" at that point.
>
> I dunno.
--
Duy
^ permalink raw reply
* The design of refs backends, linked worktrees and submodules
From: Duy Nguyen @ 2017-01-19 11:55 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Git Mailing List
I've started working on fixing the "git gc" issue with multiple
worktrees, which brings me back to this. Just some thoughts. Comments
are really appreciated.
In the current code, files backend has special cases for both
submodules (explicitly) and linked worktrees (hidden behind git_path).
But if a backend has to handle this stuff, all future backends have to
too. Which does not sound great. Imagine we have "something" in
addition to worktrees and submodules in future, then all backends have
to learn about it.
So how about we move worktree and submodule support back to front-end?
The backend deals with refs, period. The file-based backend will be
given a directory where refs live in and it work on that. Backends do
not use git_path(). Backends do not care about $GIT_DIR. Access to odb
(e.g. sha-1 validation) if needed is abstracted out via a set of
callbacks. This allows submodules to give access to submodule's
separate odb. And it's getting close to the "struct repository"
mentioned somewhere in refs "TODO" comments, even though we are
nowhere close to introducing that struct.
How does that sound? In particular, is there anything wrong, or
unrealistic, with that?
For that to work, I'll probably need to add a "composite" ref_store
that combines two file-based backends (for per-repo and per-worktree
refs) to represent a unified ref store. I think your work on ref
iterator makes way for that. A bit worried about transactions though,
because I think per-repo and per-worktree updates will be separated in
two transactions. But that's probably ok because future backends, like
lmdb, will have to go through the same route anyway.
--
Duy
^ permalink raw reply
* [PATCH v5 3/3] log --graph: customize the graph lines with config log.graphColors
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>
If you have a 256 colors terminal (or one with true color support), then
the predefined 12 colors seem limited. On the other hand, you don't want
to draw graph lines with every single color in this mode because the two
colors could look extremely similar. This option allows you to hand pick
the colors you want.
Even with standard terminal, if your background color is neither black
or white, then the graph line may match your background and become
hidden. You can exclude your background color (or simply the colors you
hate) with this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 4 ++++
graph.c | 42 +++++++++++++++++++++++++++++++++++++++---
t/t4202-log.sh | 22 ++++++++++++++++++++++
3 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2003,6 +2003,10 @@ log.follow::
i.e. it cannot be used to follow multiple files and does not work well
on non-linear history.
+log.graphColors::
+ A list of colors, separated by commas, that can be used to draw
+ history lines in `git log --graph`.
+
log.showRoot::
If true, the initial commit will be shown as a big creation event.
This is equivalent to a diff against an empty tree.
diff --git a/graph.c b/graph.c
index dd17201..822d40a 100644
--- a/graph.c
+++ b/graph.c
@@ -4,6 +4,7 @@
#include "graph.h"
#include "diff.h"
#include "revision.h"
+#include "argv-array.h"
/* Internal API */
@@ -62,6 +63,26 @@ enum graph_state {
static const char **column_colors;
static unsigned short column_colors_max;
+static void parse_graph_colors_config(struct argv_array *colors, const char *string)
+{
+ const char *end, *start;
+
+ start = string;
+ end = string + strlen(string);
+ while (start < end) {
+ const char *comma = strchrnul(start, ',');
+ char color[COLOR_MAXLEN];
+
+ if (!color_parse_mem(start, comma - start, color))
+ argv_array_push(colors, color);
+ else
+ warning(_("ignore invalid color '%.*s' in log.graphColors"),
+ (int)(comma - start), start);
+ start = comma + 1;
+ }
+ argv_array_push(colors, GIT_COLOR_RESET);
+}
+
void graph_set_column_colors(const char **colors, unsigned short colors_max)
{
column_colors = colors;
@@ -207,9 +228,24 @@ struct git_graph *graph_init(struct rev_info *opt)
{
struct git_graph *graph = xmalloc(sizeof(struct git_graph));
- if (!column_colors)
- graph_set_column_colors(column_colors_ansi,
- column_colors_ansi_max);
+ if (!column_colors) {
+ struct argv_array ansi_colors = {
+ column_colors_ansi,
+ column_colors_ansi_max + 1
+ };
+ struct argv_array *colors = &ansi_colors;
+ char *string;
+
+ if (!git_config_get_string("log.graphcolors", &string)) {
+ static struct argv_array custom_colors = ARGV_ARRAY_INIT;
+ argv_array_clear(&custom_colors);
+ parse_graph_colors_config(&custom_colors, string);
+ free(string);
+ colors = &custom_colors;
+ }
+ /* graph_set_column_colors takes a max-index, not a count */
+ graph_set_column_colors(colors->argv, colors->argc - 1);
+ }
graph->commit = NULL;
graph->revs = opt;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..0aeabed 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
test_cmp expect actual
'
+cat > expect.colors <<\EOF
+* Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+ test_config log.graphColors ",, blue,invalid-color, cyan, red , " &&
+ git log --color=always --graph --date-order --pretty=tformat:%s |
+ test_decode_color | sed "s/ *\$//" >actual &&
+ test_cmp expect.colors actual
+'
+
test_expect_success 'log --raw --graph -m with merge' '
git log --raw --graph --oneline -m master | head -n 500 >actual &&
grep "initial" actual
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 2/3] color.c: trim leading spaces in color_parse_mem()
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>
Normally color_parse_mem() is called from config parser which trims the
leading spaces already. The new caller in the next patch won't. Let's be
tidy and trim leading spaces too (we already trim trailing spaces before
comma).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
color.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/color.c b/color.c
index a9eadd1..7bb4a96 100644
--- a/color.c
+++ b/color.c
@@ -207,10 +207,15 @@ int color_parse_mem(const char *value, int value_len, char *dst)
struct color fg = { COLOR_UNSPECIFIED };
struct color bg = { COLOR_UNSPECIFIED };
+ while (len > 0 && isspace(*ptr)) {
+ ptr++;
+ len--;
+ }
+
if (!len)
return -1;
- if (!strncasecmp(value, "reset", len)) {
+ if (!strncasecmp(ptr, "reset", len)) {
xsnprintf(dst, end - dst, GIT_COLOR_RESET);
return 0;
}
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 1/3] color.c: fix color_parse_mem() with value_len == 0
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>
In this code we want to match the word "reset". If len is zero,
strncasecmp() will return zero and we incorrectly assume it's "reset" as
a result.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
color.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/color.c b/color.c
index 81c2676..a9eadd1 100644
--- a/color.c
+++ b/color.c
@@ -207,6 +207,9 @@ int color_parse_mem(const char *value, int value_len, char *dst)
struct color fg = { COLOR_UNSPECIFIED };
struct color bg = { COLOR_UNSPECIFIED };
+ if (!len)
+ return -1;
+
if (!strncasecmp(value, "reset", len)) {
xsnprintf(dst, end - dst, GIT_COLOR_RESET);
return 0;
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 0/3] nd/log-graph-configurable-colors
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170109103258.25341-1-pclouds@gmail.com>
v5 moves space trimming to color_parse_mem() from read_graph_colors_config,
which is renamed to parse_graph... because the config reading is moved
back to graph_init.
I think it looks better, but we may be pushing the limits of
argv_array's abuse.
Nguyễn Thái Ngọc Duy (3):
color.c: fix color_parse_mem() with value_len == 0
color.c: trim leading spaces in color_parse_mem()
log --graph: customize the graph lines with config log.graphColors
Documentation/config.txt | 4 ++++
color.c | 10 +++++++++-
graph.c | 42 +++++++++++++++++++++++++++++++++++++++---
t/t4202-log.sh | 22 ++++++++++++++++++++++
4 files changed, 74 insertions(+), 4 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* Re: [PATCH 0/6] loose-object fsck fixes/tightening
From: John Szakmeister @ 2017-01-19 11:18 UTC (permalink / raw)
To: Jeff King; +Cc: Dennis Kaarsemaker, git
In-Reply-To: <20170113175258.e66taigy4wpokohk@sigill.intra.peff.net>
On Fri, Jan 13, 2017 at 12:52 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 13, 2017 at 04:15:42AM -0500, John Szakmeister wrote:
>
>> > I did notice another interesting case when looking at this. Fsck ends up
>> > in fsck_loose(), which has the sha1 and path of the loose object. It
>> > passes the sha1 to fsck_sha1(), and ignores the path entirely!
>> >
>> > So if you have a duplicate copy of the object in a pack, we'd actually
>> > find and check the duplicate. This can happen, e.g., if you had a loose
>> > object and fetched a thin-pack which made a copy of the loose object to
>> > complete the pack).
>> >
>> > Probably fsck_loose() should be more picky about making sure we are
>> > reading the data from the loose version we found.
>>
>> Interesting find! Thanks for the information Peff!
>
> So I figured I would knock this out as a fun morning exercise. But
> sheesh, it turned out to be a slog, because most of the functions rely
> on map_sha1_file() to convert the sha1 to an object path at the lowest
> level.
Yeah, I discovered the same thing when I took a look at it a week or so ago. :-(
> But I finally got something working, so here it is. I found another bug
> on the way, along with a few cleanups. And then I did the trailing
> garbage detection at the end, because by that point I knew right where
> it needed to go. :)
I don't know if my opinion counts for much, but the changes look good to me.
-John
^ permalink raw reply
* Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: jean-christophe manciot @ 2017-01-19 8:42 UTC (permalink / raw)
To: git
In-Reply-To: <CAKcFC3ZN+=o_2t4AawCE0c4Tw+t_XJKTHtEq9d7bv-ht5euPbw@mail.gmail.com>
Also some context information:
Ubuntu 16.10 4.8
git 2.11.0
On Thu, Jan 19, 2017 at 9:32 AM, jean-christophe manciot
<actionmystique@gmail.com> wrote:
> In case you were wondering whether these files were tracked or not:
>
> git-Games# git ls-files Ubuntu/16.04
> Ubuntu/16.04/residualvm_0.3.0~git-1_amd64.deb
> Ubuntu/16.04/scummvm-data_1.8.0_all.deb
> Ubuntu/16.04/scummvm-data_1.9.0_all.deb
> Ubuntu/16.04/scummvm-dbgsym_1.8.2~git20160821.bad86050_amd64.deb
> Ubuntu/16.04/scummvm-dbgsym_1.9.0_amd64.deb
> Ubuntu/16.04/scummvm_1.8.0_amd64.deb
> Ubuntu/16.04/scummvm_1.9.0_amd64.deb
>
> On Tue, Jan 17, 2017 at 4:30 PM, jean-christophe manciot
> <actionmystique@gmail.com> wrote:
>> Hi there,
>>
>> I'm trying to purge a complete folder and its files from the
>> repository history with:
>>
>> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
>> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
>> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
>>
>> git does not find the folder although it's there:
>>
>> git-Games# ll Ubuntu/16.04/
>> total 150316
>> drwxr-x--- 2 actionmystique actionmystique 4096 Nov 19 13:40 ./
>> drwxr-x--- 4 actionmystique actionmystique 4096 Oct 30 14:02 ../
>> -rwxr-x--- 1 actionmystique actionmystique 2190394 May 9 2016
>> residualvm_0.3.0~git-1_amd64.deb*
>> ...
>> -rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
>> scummvm-dbgsym_1.9.0_amd64.deb
>>
>> What is going on?
>>
>> --
>> Jean-Christophe
>
>
>
> --
> Jean-Christophe
--
Jean-Christophe
^ permalink raw reply
* Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: jean-christophe manciot @ 2017-01-19 8:32 UTC (permalink / raw)
To: git
In-Reply-To: <CAKcFC3aqjLNUNKPDZ08rO_SBkY84uvHBerCxnSchAe8rH0dnMg@mail.gmail.com>
In case you were wondering whether these files were tracked or not:
git-Games# git ls-files Ubuntu/16.04
Ubuntu/16.04/residualvm_0.3.0~git-1_amd64.deb
Ubuntu/16.04/scummvm-data_1.8.0_all.deb
Ubuntu/16.04/scummvm-data_1.9.0_all.deb
Ubuntu/16.04/scummvm-dbgsym_1.8.2~git20160821.bad86050_amd64.deb
Ubuntu/16.04/scummvm-dbgsym_1.9.0_amd64.deb
Ubuntu/16.04/scummvm_1.8.0_amd64.deb
Ubuntu/16.04/scummvm_1.9.0_amd64.deb
On Tue, Jan 17, 2017 at 4:30 PM, jean-christophe manciot
<actionmystique@gmail.com> wrote:
> Hi there,
>
> I'm trying to purge a complete folder and its files from the
> repository history with:
>
> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
>
> git does not find the folder although it's there:
>
> git-Games# ll Ubuntu/16.04/
> total 150316
> drwxr-x--- 2 actionmystique actionmystique 4096 Nov 19 13:40 ./
> drwxr-x--- 4 actionmystique actionmystique 4096 Oct 30 14:02 ../
> -rwxr-x--- 1 actionmystique actionmystique 2190394 May 9 2016
> residualvm_0.3.0~git-1_amd64.deb*
> ...
> -rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
> scummvm-dbgsym_1.9.0_amd64.deb
>
> What is going on?
>
> --
> Jean-Christophe
--
Jean-Christophe
^ permalink raw reply
* Re: Git: new feature suggestion
From: Konstantin Khomoutov @ 2017-01-19 6:33 UTC (permalink / raw)
To: Joao Pinto; +Cc: git, Linus Torvalds, CARLOS.PALMINHA@synopsys.com
In-Reply-To: <4817eb00-6efc-e3c0-53d7-46f2509350d3@synopsys.com>
On Wed, 18 Jan 2017 10:40:52 +0000
Joao Pinto <Joao.Pinto@synopsys.com> wrote:
[...]
> I have seen a lot of Linux developers avoid this re-organization
> operations because they would lose the renamed file history, because
> a new log is created for the new file, even if it is a renamed
> version of itself. I am sending you this e-mail to suggest the
> creation of a new feature in Git: when renamed, a file or folder
> should inherit his parent’s log and a “rename: …” would be
> automatically created or have some kind of pointer to its “old” form
> to make history analysis easier.
Git does not record renames because of its stance that what matters is
code _of the whole project_ as opposed to its location in a particular
file.
Hence with regard to renames Git "works backwards" by detecting them
dynamically while traversing the history (such as with `git log`
etc). This detection uses certain heuristics which can be controlled
with knobs pointed to by Stefan Beller.
Still, I welcome you to read the sort-of "reference" post by Linus
Torvalds [1] in which he explains the reasoning behind this approach
implemented in Git. IMO, understanding the reasoning behind the idea
is much better than just mechanically learning how to use it.
The whole thread (esp. Torvalds' replies) is worth reading, but that
particular mail summarizes the whole thing very well.
(The reference link to it used to be [2], but Gmane is not fully
recovered to be able to display it.)
1. http://public-inbox.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/
2. http://thread.gmane.org/gmane.comp.version-control.git/27/focus=217
^ permalink raw reply
* [PATCHv3 3/4] cache.h: document add_[file_]to_index
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/cache.h b/cache.h
index 929474d7a9..12394eb541 100644
--- a/cache.h
+++ b/cache.h
@@ -614,8 +614,18 @@ extern int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_IGNORE_ERRORS 4
#define ADD_CACHE_IGNORE_REMOVAL 8
#define ADD_CACHE_INTENT 16
+/*
+ * These two are used to add the contents of the file at path
+ * to the index, marking the working tree up-to-date by storing
+ * the cached stat info in the resulting cache entry. A caller
+ * that has already run lstat(2) on the path can call
+ * add_to_index(), and all others can call add_file_to_index();
+ * the latter will do necessary lstat(2) internally before
+ * calling the former.
+ */
extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
extern int add_file_to_index(struct index_state *, const char *path, int flags);
+
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* [PATCHv3 4/4] documentation: retire unfinished documentation
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>
When looking for documentation for a specific function, you may be tempted
to run
git -C Documentation grep index_name_pos
only to find the file technical/api-in-core-index.txt, which doesn't
help for understanding the given function. It would be better to not find
these functions in the documentation, such that people directly dive into
the code instead.
In the previous patches we have documented
* index_name_pos()
* remove_index_entry_at()
* add_[file_]to_index()
in cache.h
We already have documentation for:
* add_index_entry()
* read_index()
Which leaves us with a TODO for:
* cache -> the_index macros
* refresh_index()
* discard_index()
* ie_match_stat() and ie_modified(); how they are different and when to
use which.
* write_index() that was renamed to write_locked_index
* cache_tree_invalidate_path()
* cache_tree_update()
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/technical/api-in-core-index.txt | 21 ---------------------
1 file changed, 21 deletions(-)
delete mode 100644 Documentation/technical/api-in-core-index.txt
diff --git a/Documentation/technical/api-in-core-index.txt b/Documentation/technical/api-in-core-index.txt
deleted file mode 100644
index adbdbf5d75..0000000000
--- a/Documentation/technical/api-in-core-index.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-in-core index API
-=================
-
-Talk about <read-cache.c> and <cache-tree.c>, things like:
-
-* cache -> the_index macros
-* read_index()
-* write_index()
-* ie_match_stat() and ie_modified(); how they are different and when to
- use which.
-* index_name_pos()
-* remove_index_entry_at()
-* remove_file_from_index()
-* add_file_to_index()
-* add_index_entry()
-* refresh_index()
-* discard_index()
-* cache_tree_invalidate_path()
-* cache_tree_update()
-
-(JC, Linus)
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* [PATCHv3 2/4] cache.h: document remove_index_entry_at
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>
Do this by moving the existing documentation from
read-cache.c to cache.h.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 3 +++
read-cache.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index 1469ddeafe..929474d7a9 100644
--- a/cache.h
+++ b/cache.h
@@ -603,7 +603,10 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
#define ADD_CACHE_KEEP_CACHE_TREE 32 /* Do not invalidate cache-tree */
extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
+
+/* Remove entry, return true if there are more entries to go. */
extern int remove_index_entry_at(struct index_state *, int pos);
+
extern void remove_marked_cache_entries(struct index_state *istate);
extern int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 2eca639cce..63a414cdb5 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -503,7 +503,6 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel
return index_name_stage_pos(istate, name, namelen, 0);
}
-/* Remove entry, return true if there are more entries to go.. */
int remove_index_entry_at(struct index_state *istate, int pos)
{
struct cache_entry *ce = istate->cache[pos];
--
2.11.0.299.g762782ba8a
^ 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