Re: [PATCH] help: allow redirecting to help for aliased command
On Sat, Sep 29, 2018 at 10:27:15PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > And I think this has to be stderr. We're polluting the output of the > > aliased command with our extra message, so we have two choices: > > > > 1. Pollute stderr, and risk copious stdout (or a pager) scrolling it > > off the screen. > > > > 2. Pollute stdout, at which point our message may be confused as part > > of the actual output of the command (and that may not even be > > immediately noticed if it is passed through a shell pipeline or > > into a file). > > > > Choice (2) seems like a regression to me. Choice (1) is unfortunate in > > some cases, but is no worse than today's behavior. > > I think the output of "git foo -h" changing (i.e. has "aliased > to..." message in front) is about the same degree of regression as > "git foo --help" no longer giving "aliased to..." information > anywhere, though. Hmm. They seem quite different to me. Changing "--help" output is something that's only going to impact what the user sees (a manpage versus the alias message). And the user can follow-up by asking for what they wanted. Whereas if I have an alias that currently understands "-h", and I do something like: git foo -h | wc -l if we output to stdout, that's going to produce subtly broken results. But if we output to stderr instead, then they may see the extra message, but it's obvious what's happening, and it's probably an annoyance at worst). > > Yeah. I think if "git foo -h" produces a bunch of output you didn't > > expect, then "git help foo" or "git foo --help" may be the next thing > > you reach for. That's not so different than running the command even > > without any aliases involved. > > Hmmm. With the "teach 'git foo -h' to output 'foo is aliased to > bar' to the standard error before running 'git bar -h'", plus "'git > foo --help' now goes straight to 'git bar --help'", "git foo --help" > no longer tells us that foo is aliased to bar. Presumably "git help > foo" will still give "foo is bar" and stop? Yes, that was the intent in the behavior I laid out earlier. -Peff
Re: [PATCH v2 8/8] reflog expire: cover reflog from all worktrees
On Sat, Sep 29, 2018 at 3:11 PM Nguyễn Thái Ngọc Duy wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
> @@ -72,6 +72,11 @@ Options for `expire`
> +--single-worktree::
> + By default when `--all` is specified, reflogs from all working
> + trees are processed. This option limits the processing to reflogs
> + from the current working tree only.
Bikeshedding: I wonder if this should be named "--this-worktree" or
"--this-worktree-only" or if it should somehow be orthogonal to --all
rather than modifying it. (Genuine questions. I don't have the
answers.)
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> @@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char
> **argv, const char *prefix)
> if (do_all) {
> struct collect_reflog_cb collected;
> + struct worktree **worktrees, **p;
> int i;
>
> memset(, 0, sizeof(collected));
> - for_each_reflog(collect_reflog, );
> + worktrees = get_worktrees(0);
> + for (p = worktrees; *p; p++) {
> + if (!all_worktrees && !(*p)->is_current)
> + continue;
> + collected.wt = *p;
> + for_each_reflog(collect_reflog, );
> + }
> + free_worktrees(worktrees);
Should this have a test in the test suite?
Re: [PATCH] help: allow redirecting to help for aliased command
Jeff King writes: > And I think this has to be stderr. We're polluting the output of the > aliased command with our extra message, so we have two choices: > > 1. Pollute stderr, and risk copious stdout (or a pager) scrolling it > off the screen. > > 2. Pollute stdout, at which point our message may be confused as part > of the actual output of the command (and that may not even be > immediately noticed if it is passed through a shell pipeline or > into a file). > > Choice (2) seems like a regression to me. Choice (1) is unfortunate in > some cases, but is no worse than today's behavior. I think the output of "git foo -h" changing (i.e. has "aliased to..." message in front) is about the same degree of regression as "git foo --help" no longer giving "aliased to..." information anywhere, though. >> Even the first two "better" cases share the same glitch if the "foo >> ... >> thing to do is to >> >> $ git unknown-command -h >&2 | less >> >> And at that point, it does not matter which between the standard >> output and the standard error streams we write "unknown-command is >> aliased to ...". > > Yeah. I think if "git foo -h" produces a bunch of output you didn't > expect, then "git help foo" or "git foo --help" may be the next thing > you reach for. That's not so different than running the command even > without any aliases involved. Hmmm. With the "teach 'git foo -h' to output 'foo is aliased to bar' to the standard error before running 'git bar -h'", plus "'git foo --help' now goes straight to 'git bar --help'", "git foo --help" no longer tells us that foo is aliased to bar. Presumably "git help foo" will still give "foo is bar" and stop?
Re: [PATCH v2 5/8] revision.c: better error reporting on ref from different worktrees
On Sat, Sep 29, 2018 at 3:11 PM Nguyễn Thái Ngọc Duy wrote:
> Make use of the new ref aliases to pass refs from another worktree
> around and access them from the current ref store instead. This does
> not change any functionality, but when a problem arises, we would like
> the reported messages to mention full ref aliases, like this:
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
> +void strbuf_worktree_ref(const struct worktree *wt,
> +struct strbuf *sb,
> +const char *refname)
> +{
> + if (wt && !wt->is_current) {
> + if (is_main_worktree(wt))
> + strbuf_addstr(sb, "main/");
I think this needs to be "main-worktree/", doesn't it?
> + else
> + strbuf_addf(sb, "worktrees/%s/", wt->id);
> + }
> + strbuf_addstr(sb, refname);
> +}
> diff --git a/worktree.h b/worktree.h
> @@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct
> worktree *wt,
> +/*
> + * Return a refname suitable for access from the current ref
> + * store. The result may be destroyed at the next call.
s/may/will/
Re: [PATCH] Improvement to only call Git Credential Helper once
On Sat, Sep 29, 2018 at 12:06:38PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > Wow, what's old is new again. Here's more or less the same patch from > > 2012: > > > > https://public-inbox.org/git/20120407033417.ga13...@sigill.intra.peff.net/ > > > > Unfortunately, some people seem to rely on this multi-helper behavior. I > > recommend reading the whole thread, as there are some interesting bits > > in it (that I had always meant to return to, but somehow 6 years went > > by). > > After reading that thread again, my version of summary is that > > - storing the credential obtained from a helper back to the same >helper may be pointless at best and may incur end-user >interaction (e.g. asking for symmetric encryption key before the >data hits the disk), but it can be used as a "we used what you >gave us successfully" ping-back signal. We may not have designed >approve() to do "store" back to the same helper by default and >instead to do so only when asked by the helper, if we knew >better. But an unconditional change of behaviour will break >existing users and helpers. > > - storing the credential obtained from a helper to a different >helper may have security implications, and we may not have >designed approve() to do "store" by default to all helpers if we >knew better. But an unconditional change of behaviour will break >existing users and helpers. Yeah, I agree with that summary, except I want to pick a nit with the very last sentence. You're not wrong that it will break those existing users, but there are security implications. If you're expecting and relying on that behavior, then all is well with the current code, and you'd be annoyed by a change. But if you're not expecting it, the system is not just broken: it may be leaking credentials from a higher-security first-choice helper to a lower-security second-choice one. And that's why I wonder if it might be the right thing to break compatibility in this case. > Assuming that the above summary is accurate, I think the former is > easier to solve. In the ideal end game state, we would also have > "accepted" in the protocol and call the helper that gave us the > accepted credential material with an earlier "get" (if the helper is > updated to understand "accepted"). The "accepted" may not even need > to receive the credential material as the payload. I think you can really solve both by adding a "retrieved" key to the store step of the protocol (which can be done in a backwards-compatible way, since all sides are supposed to ignore keys they don't understand). Then a helper which understands "retrieved" can say "Oh, this came from another helper; I don't need to store it". And that includes things that might have come from itself! Likewise, helpers might take options to tell them how to behave. So the "do some https thing and cache it" combination from that earlier thread would be: [credential] helper = magic-https-thing helper = cache and the safe version of "try high-security thing, and then fall back to caching" would be: [credential] helper = high-security-thing helper = cache --no-retrieved (that keeps the default matching the current behavior, but obviously we could flip it if we wanted to have a safer default but leave the existing case with an escape hatch). > The latter is trickier, as there are design considerations. > > - We may want to allow the helper that gives the credential back >from the outside world upon "get" request to say "you can 'store' >this to other helpers of this kind but not of that kind". If we >want to do so, we'd need to extend what is returned from the >helper upon "get" (e.g. "get2" will give more than what "get" >does back). I don't think you need a "get2". The helpers should be free to pass back extra keys (but old versions of Git just won't do anything with them). > - We may want to allow the helper that receives the credential >material from others to behave differently depending on where it >came from, and what other helpers done to it (e.g. even a new >credential the end user just typed may not want to go to all >helpers; an on-disk helper may feel "I'd take it if the only >other helpers that responded to 'store' are the transient >'in-core' kind, but otherwise I'd ignore"). I am not offhand >sure what kind of flexibility and protocol extension is >necessary. If we have a "retrieved" flag, it could be "retrieved=some-helper" to show which helper it came from. One tricky thing, though, is that helpers do not always have a simple name (they can be arbitrary shell expressions). > - We may want to be able to override the preference the helper >makes in the above two. The user may want to say "Only use this >on-disk helper as a read-only source and never call 'store' on it >to add new entries (or update an existing one)." Yes. I think if we go this
Re: [PATCH v2 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
On Sat, Sep 29, 2018 at 3:10 PM Nguyễn Thái Ngọc Duy wrote: > The main worktree has to be treated specially because well.. it's Nit: s/well../well.../ > special from the beginning. So HEAD from the main worktree is > acccessible via the name "main-worktree/HEAD" instead of > "worktrees/main/HEAD" because "main" could be just another secondary > worktree. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt > @@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs. > There are one > +Refs that are per working tree can still be accessed from another > +working tree via two special paths main-worktree and worktrees. The s/paths/paths,/ > +former gives access to per-worktree refs of the main working tree, > +while the former to all linked working trees. s/former/latter/ > diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh > @@ -30,4 +30,50 @@ test_expect_success 'refs/worktree are per-worktree' ' > +test_expect_success 'ambiguous main-worktree/HEAD' ' > + mkdir -p .git/refs/heads/main-worktree && > + test_when_finished rm .git/refs/heads/main-worktree/HEAD && > + cp .git/HEAD .git/refs/heads/main-worktree/HEAD && Better to use "rm -f" for cleanup in case this 'cp' fails for some reason. > + git rev-parse main-worktree/HEAD 2>warn >/dev/null && You could probably omit the /dev/null redirect. > + grep "main-worktree/HEAD.*ambiguous" warn > +' > + > +test_expect_success 'ambiguous worktrees/xx/HEAD' ' > + mkdir -p .git/refs/heads/worktrees/wt1 && > + test_when_finished rm .git/refs/heads/worktrees/wt1/HEAD && Ditto "rm -f". > + cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD && > + git rev-parse worktrees/wt1/HEAD 2>warn >/dev/null && Ditto /dev/null. > + grep "worktrees/wt1/HEAD.*ambiguous" warn > +'
Re: [PATCH] fetch: replace string-list used as a look-up table with a hashmap
On Sat, Sep 29, 2018 at 11:31:08AM -0700, Junio C Hamano wrote:
> > The only funny thing is that you have to "dereference" the iterator like
> > this:
> >
> > struct list_head *pos;
> > struct actual_thing *item;
> > ...
> > item = list_entry(pos, struct actual_thing, list_member);
> >
> > which is a minor pain, but it's reasonably hard to get it wrong.
> >
> > I wonder if we could do the same here. The hashmap would only ever see
> > the "struct hashmap_entry", and then the caller would convert that back
> > to the actual type.
>
> Hmph, how would hashmap_cmp_fn look like with that scheme? It would
> get one entry, another entry (or just the skeleton of it) and
> optionally a separate keydata (if the second one is skeleton), and
> the first two points at the embedded hashmap struct, not the
> surrounding one. The callback function is now responsible for
> calling a hashmap_entry() macro that adjusts for the negative offset
> like list_entry() does?
Exactly. The comparison functions currently look something like this:
int foo_hash_cmp(const void *data,
const void *e1, const void *e2,
const void *keydata)
{
const struct foo *f1 = (const struct foo *)e1;
const struct foo *f2 = (const struct foo *)e2;
return strcmp(foo->field, keydata ? keydata : foo->field);
}
(this is using the correct function signature; you can drop the casts if
you violate that, but IMHO this style is more maintainable in the long
run). With the hashmap_entry at an arbitrary position, the body is:
const struct foo *f1 = hash_entry(e1, struct foo, hash_field);
const struct foo *f2 = hash_entry(e2, struct foo, hash_field);
return strcmp(foo->field, keydata ? keydata : foo->field);
where hash_field is the member of "struct foo" that holds the "struct
hashmap_entry". So that's not so different, but there is an interesting
implication here. The comparison callback has to know which
hashmap_entry name to use! So if you have a struct that can be in two
hashes, like:
struct foo {
char *name;
struct hashmap_entry h1;
struct hashmap_entry h2;
};
then you cannot use a single foo_hash_cmp() function. You have to use a
different one for each field. Which seems kind of nasty and error-prone.
So I dunno. Maybe this is not a good direction. I have to admit that I'm
not wild about our hashmap implementation in general:
- the way it handles allocations is often awkward, or causes you to
write confusing boilerplate
- it's not type-safe
- it seems slower than open-addressed alternatives (e.g., over in [1]
we determined that oidset can be made almost twice as fast using
khash)
- the chained implementation in hashmap.c is probably better for cases
where there's a lot of deletions, because removing from the hashmap
truly removes everything (whereas with open-addressed schemes, we
either didn't implement deletion at all, or it sets a marker which
allows the item to be dropped next time the hash is resized)
[1] https://public-inbox.org/git/20180814015842.ga27...@sigill.intra.peff.net/
> Was it ever a consideration, when allowing struct list-head anywhere
> in the enclosing struct, that it would allow an element to be on
> more than one list?
I don't know the history of that list code in that kernel, but I always
assumed that was one of the goals. We don't use it yet, but there are
places where we could (e.g., packed_git is part of the regular packed
list, as well as the mru; the former is implemented as a singly-linked
list, but that's mostly historical).
It also allows us to have an item in a list and a hashmap (since if they
both needed to be at the start of the struct, that wouldn't work). We do
use that ability for delta_base_cache_entry.
> Would it benefit us to be able to place an
> element in multiple hashmaps because we do not have to have the
> embedded hashmap_entry always at the beginning if we did this
> change?
I'm not sure. I think it would allow us to more aggressively stick the
hashmap_entry inside existing structs. For instance, in the patch from
this thread, could we actually shove the hashmap_entry structs into
"struct ref", and save having to allocate the extra refname_hash struct?
I guess that pollutes "struct ref", though. Unless we generically say
"it can be a part of up to 3 hashes" and provide struct hashmap_entry
h1, struct hashmap_entry h2, etc. And then your new code does:
struct hashmap existing_refs;
struct hashmap remote_refs;
/*
* These need to be different compare functions to access the
* h1 and h2 fields in "struct ref"!
*/
hashmap_init(_refs, ref_hash_cmp_h1);
hashmap_init(_refs, ref_hash_cmp_h2);
...
hashmap_add(_refs, _ref->h1);
But that seems pretty error-prone (not just confusing h1 and h2 here,
but how do we know that some other part of the code isn't using "h1" for
its own hash already?)
So again, maybe this is just a bad direction.
At
Re: [PATCH v2 1/1] roll wt_status_state into wt_status and populate in the collect phase
On Sat, Sep 29, 2018 at 2:55 PM Stephen P. Smith wrote: > Status variables were initialized in the collect phase and some > variables were later freed in the print functions. > > A "struct wt_status" used to be sufficient for the output phase to > work. It was designed to be filled in the collect phase and consumed > in the output phase, but over time some fields were added and output > phase started filling the fields. > > A "struct wt_status_state" that was used in other codepaths turned out > to be useful in the "git status" output. This is not tied to "struct > wt_status", so filling in the collect phase was not consistently > followed. > > Move the status state structure variables into the status state > structure and populate them in the collect functions. > > Create a new funciton to free the buffers that were being freed in the s/funciton/function/ > print function. Call this new function in commit.c where both the > collect and print functions were being called. > > Signed-off-by: Stephen P. Smith
Re: [PATCH 1/1] roll wt_status_state into wt_status and populate in the collect phase
On Fri, Sep 28, 2018 at 9:55 AM Taylor Blau wrote: > On Thu, Sep 27, 2018 at 09:49:36PM -0700, Stephen P. Smith wrote: > > When updating the collect and print functions, it was found that > > status variables were initialized in the collect phase and some > > variables were later freed in the print functions. > > Nit: I think that in the past Eric Sunshine has recommended that I use > active voice in patches, but "it was found" is passive. > > I tried to find the message that I was thinking of, but couldn't, so > perhaps I'm inventing it myself ;-). > > I'm CC-ing Eric to check my judgement. You're probably thinking of "imperative mood" (and perhaps [1]), which this commit message already uses when it says "Move the..." and "Create a new function..." (in the couple paragraphs following the part you quoted). > > Move the status state structure variables into the status state > > structure and populate them in the collect functions. > > > > Create a new funciton to free the buffers that were being freed in the s/funciton/function/ > > print function. Call this new function in commit.c where both the > > collect and print functions were being called. [1]: https://public-inbox.org/git/capig+ctozduqsaxh+w4h85m7en72yo09asdx+1kstswqbnb...@mail.gmail.com/
Re: [PATCH v2 2/2] worktree: add per-worktree config files
On Sat, Sep 29, 2018 at 11:30 AM Nguyễn Thái Ngọc Duy wrote:
> A new repo extension is added, worktreeConfig. When it is present:
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -2,8 +2,9 @@ CONFIGURATION FILE
> The Git configuration file contains a number of variables that affect
> +the Git commands' behavior. The files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) in each
> +repository is used to store the configuration for that repository, and
s/is used/are/used/
> `$HOME/.gitconfig` is used to store a per-user configuration as
> fallback values for the `.git/config` file. The file `/etc/gitconfig`
> can be used to store a system-wide default configuration.
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -204,6 +204,36 @@ working trees, it can be used to identify worktrees. For
> example if
> +CONFIGURATION FILE
> +--
> +In this mode, specific configuration stays in the path pointed by `git
> +rev-parse --git-path config.worktree`. You can add or update
> +configuration in this file with `git config --worktree`. Older Git
> +versions may will refuse to access repositories with this extension.
s/may will/will/
> diff --git a/Documentation/gitrepository-layout.txt
> b/Documentation/gitrepository-layout.txt
> @@ -275,6 +280,9 @@ worktrees//locked::
> +worktrees//config.worktree::
> + Working directory specific configuration file.
I wonder if this deserves a quick mention in
Documentation/git-worktree.txt since other worktree-related files,
such as "worktrees//locked", are mentioned there.
> diff --git a/builtin/config.c b/builtin/config.c
> @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char
> *prefix)
> + else if (use_worktree_config) {
> + struct worktree **worktrees = get_worktrees(0);
> + if (repository_format_worktree_config)
> + given_config_source.file =
> git_pathdup("config.worktree");
> + else if (worktrees[0] && worktrees[1])
> + die(_("--worktree cannot be used with multiple "
> + "working trees unless the config\n"
> + "extension worktreeConfig is enabled. "
> + "Please read \"CONFIGURATION FILE\"\n"
> + "section in \"git help worktree\" for
> details"));
> + else
> + given_config_source.file = git_pathdup("config");
I'm not sure I understand the purpose of allowing --worktree when only
a single worktree is present and extensions.worktreeConfig is not set.
Can you talk about it a bit more?
Re: [PATCH] help: allow redirecting to help for aliased command
On Sat, Sep 29, 2018 at 10:39:54AM -0700, Junio C Hamano wrote: > Suppose "git foo" is aliased to a command "git bar". > > The best case is when "git bar -h" knows that it is asked to give us > a short usage. We get "foo is aliased to bar" followed by the short > usage for "bar" and everything is visible above the shell prompt > after all that happens. > > The second best case is when "git bar" simply does not support "-h" > but actively notices an unknown option on the command line to give > the usage message. We see "foo is aliased to bar" followed by "-h > is an unknown option; supported options are ..." and everything is > visible above the shell prompt after all that happens. Right, these are the ones we hope for. > The worst case is when "git bar" supports or ignores "-h" and > produces reams of output. Sending the "aliased to" message to the > standard error means that it is scrolled out when the output is > done, or lost even when "git foo -h | less" attempts to let the > reader read before the early part of the output scrolls away. This is the "who-knows-what" case I meant here: >> It is a little funny, I guess, if you have a script which doesn't >> respond to "-h", because you'd get our "foo is aliased to git-bar" >> message to stderr, followed by who-knows-what. But as long as it's to >> stderr (and not stdout), I think it's not likely to _break_ anything. And I think this has to be stderr. We're polluting the output of the aliased command with our extra message, so we have two choices: 1. Pollute stderr, and risk copious stdout (or a pager) scrolling it off the screen. 2. Pollute stdout, at which point our message may be confused as part of the actual output of the command (and that may not even be immediately noticed if it is passed through a shell pipeline or into a file). Choice (2) seems like a regression to me. Choice (1) is unfortunate in some cases, but is no worse than today's behavior. (Obviously I'm not including choices like not running the sub-command at all, but I think that would be even worse). > Even the first two "better" cases share the same glitch if the "foo > is aliased to bar" goes to the standard error output. Parse-options > enabled commands tend to show a long "-h" output that you would need > to say "git grep -h | less", losing the "aliased to" message. > > At least it seems to me an improvement to use standard output, > instead of standard error, for the alias information. ...so I'd disagree with this. > In practice, however, what the command that "git foo" is aliased to > does when given "-h" is probably unknown (because the user is asking > what "git foo" is in the first place), so perhaps I am worried too > much. When the user does not know if the usage text comes to the > standard output or to the standard error, and if the usage text is > very long or not, they probably would learn quickly that the safest > thing to do is to > > $ git unknown-command -h >&2 | less > > And at that point, it does not matter which between the standard > output and the standard error streams we write "unknown-command is > aliased to ...". Yeah. I think if "git foo -h" produces a bunch of output you didn't expect, then "git help foo" or "git foo --help" may be the next thing you reach for. That's not so different than running the command even without any aliases involved. -Peff
Re: [PATCH v2 1/2] t1300: extract and use test_cmp_config()
On Sat, Sep 29, 2018 at 11:30 AM Nguyễn Thái Ngọc Duy wrote:
> In many config-related tests it's common to check if a config variable
> has expected value and we want to print the differences when the test
> fails. Doing it the normal way is three lines of shell code. Let's add
> a function do to all this (and a little more).
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -747,6 +747,30 @@ test_cmp() {
> +# similar to test_cmp but $2 is a config key instead of actual value
> +# it can also accept -C to read from a different repo, e.g.
Minor: maybe say that "-C " changes to for the git-config invocation.
> +# test_cmp_config -C xyz foo core.bar
> +#
> +# is sort of equivalent of
> +#
> +# test "foo" = "$(git -C xyz core.bar)"
Should be: $(git -C xyz config core.bar)
> +test_cmp_config() {
> + if [ "$1" = "-C" ]
> + then
Style:
if test "$1" = "-C"
then
...
> + shift &&
> + GD="-C $1" &&
> + shift
> + else
> + GD=
> + fi &&
> + echo "$1" >expected &&
If $1 starts with a hyphen, 'echo' might try interpreting it as an
option. Use printf instead:
printf "%s\n" "$1" &&
> + shift &&
> + git $GD config "$@" >actual &&
> + test_cmp expected actual
Please choose names other than "actual" and "expected" since those are
likely to clobber files of the same name which the test has set up
itself. (Or, at the very least, document that this function clobbers
those files -- but using better filenames is preferable.)
> +}
Re: [PATCH] fetch: replace string-list used as a look-up table with a hashmap
Jeff King writes:
> So yet another alternative here is to just define a single hashmap that
> stores void pointers. That also throws away some type safety, but is
> maybe conceptually simpler. I dunno.
I was tempted to try that route, which would essentially give us "if
you are abusing string-list as a look-up table, and if either you do
not need to iterate over all the elements, or you do not care about
the order in which such an interation is made, then use this
instead" API that can more easily substitute string-list without
boilerplate.
But I am not sure if that is worth it. Perhaps we may end up doing
so when we find the second thing to move from string-list to hashmap,
but not with this patch (and yes, there is another string-list user
in the same source file, which I think can reuse what I added with
this patch).
In any case, I expect to be offline more than half of the next week,
so before I forget, here is an updated patch. Two changes since the
version posted earlier are:
- remote_refs_list (not remote_refs which is a hashmap) is passed
to string_list_clear() at the end. This is a fix for an outright
bug Ramsay noticed.
- The callback function now takes (const void *) and casts its
parameters to correct type before they are used, instead of
casting the pointer to a function whose signature is wrong in the
call to hashmap_init(). This was noticed by Stefan and I agree
that doing it this way makes more sense.
-- >8 --
Subject: [PATCH v2] fetch: replace string-list used as a look-up table with a
hashmap
In find_non_local_tags() helper function (used to implement the
"follow tags"), we use string_list_has_string() on two string lists
as a way to see if a refname has already been processed, etc.
All this code predates more modern in-core lookup API like hashmap;
replace them with two hashmaps and one string list---the hashmaps
are used for look-ups and the string list is to keep the order of
items in the returned result stable (which is the only single thing
hashmap does worse than lookups on string-list).
Signed-off-by: Junio C Hamano
---
builtin/fetch.c | 121 +---
1 file changed, 94 insertions(+), 27 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 0696abfc2a..489531ec93 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -246,16 +246,76 @@ static int will_fetch(struct ref **head, const unsigned
char *sha1)
return 0;
}
+struct refname_hash_entry {
+ struct hashmap_entry ent; /* must be the first member */
+ struct object_id oid;
+ char refname[FLEX_ARRAY];
+};
+
+static int refname_hash_entry_cmp(const void *hashmap_cmp_fn_data,
+ const void *e1_,
+ const void *e2_,
+ const void *keydata)
+{
+ const struct refname_hash_entry *e1 = e1_;
+ const struct refname_hash_entry *e2 = e2_;
+
+ return strcmp(e1->refname, keydata ? keydata : e2->refname);
+}
+
+static struct refname_hash_entry *refname_hash_add(struct hashmap *map,
+ const char *refname,
+ const struct object_id *oid)
+{
+ struct refname_hash_entry *ent;
+ size_t len = strlen(refname);
+
+ FLEX_ALLOC_MEM(ent, refname, refname, len);
+ hashmap_entry_init(ent, memhash(refname, len));
+ oidcpy(>oid, oid);
+ hashmap_add(map, ent);
+ return ent;
+}
+
+static int add_one_refname(const char *refname,
+ const struct object_id *oid,
+ int flag, void *cbdata)
+{
+ struct hashmap *refname_map = cbdata;
+
+ (void) refname_hash_add(refname_map, refname, oid);
+ return 0;
+}
+
+static void refname_hash_init(struct hashmap *map)
+{
+ hashmap_init(map, refname_hash_entry_cmp, NULL, 0);
+}
+
+static int refname_hash_exists(struct hashmap *map, const char *refname)
+{
+ struct hashmap_entry key;
+ size_t len = strlen(refname);
+ hashmap_entry_init(, memhash(refname, len));
+
+ return !!hashmap_get(map, , refname);
+}
+
static void find_non_local_tags(const struct ref *refs,
struct ref **head,
struct ref ***tail)
{
- struct string_list existing_refs = STRING_LIST_INIT_DUP;
- struct string_list remote_refs = STRING_LIST_INIT_NODUP;
+ struct hashmap existing_refs;
+ struct hashmap remote_refs;
+ struct string_list remote_refs_list = STRING_LIST_INIT_NODUP;
+ struct string_list_item *remote_ref_item;
const struct ref *ref;
- struct string_list_item *item = NULL;
+ struct refname_hash_entry *item = NULL;
- for_each_ref(add_existing, _refs);
+ refname_hash_init(_refs);
+ refname_hash_init(_refs);
+
+ for_each_ref(add_one_refname, _refs);
Re: Git Evolve
Stefan Xenos writes: > What is the evolve command? > ... > - Systems like gerrit would no longer need to rely on "change-id" tags > in commit comments to associate commits with the change that they > edit, since git itself would have that information. > ... > Is anyone else interested in this? Please email me directly or on this > list. Let's chat: I want to make sure that whatever we come up with is > at least as good as any similar technology that has come before. As you listed in the related technologies section, I think the underlying machinery that supports "rebase -i", especially with the recent addition of redoing the existing merges (i.e. "rebase -i -r"), may be enough to rewrite the histories that were built on top of a commit that has been obsoleted by amending. I would imagine that the main design effort you would need to make is to figure out a good way to (1) keep track of which commits are obsoleted by which other ones [*1*], and (2) to figure out what histories are still to be rebuilt in what order on top of what commit efficiently. Once these are done, you should be able to write out the sequence of instructions to feed the same sequencer machinery used by the "rebase -i" command. [Side note] *1* It is very desirable to keep track of the evolution of a change without polluting the commit object with things like Change-Id: and other cruft, either in the body or in the header. If we lose the Change-Id: footer without adding any new cruft in the commit object header, that would be a great success. It would be a failure if we end up touching the object header.
Git Evolve
Hello, List! I'm interested in porting something like Mercurial's evolve command to Git. I'll be following up with a formal proposal shortly, but before I do I thought I'd introduce myself to the list and find out if anyone else is interested in this topic. What is the evolve command? Imagine you have three dependent changes up for review and you receive feedback that requires editing all three changes. While you're editing one, more feedback arrives on one of the others. What do you do? The evolve command is a convenient way to work with chains of commits that are under review. Whenever you rebase or amend a commit, the repository remembers that the old commit is obsolete and has been replaced by the new one. Then, at some point in the future, you can run "git evolve" and the correct sequence of rebases will occur in the correct order such that no commit has an obsolete parent. Part of making the "evolve" command work involves tracking the edits to a commit over time, which could provide a lot of other benefits: - Systems like gerrit would no longer need to rely on "change-id" tags in commit comments to associate commits with the change that they edit, since git itself would have that information. - You could directly view the history of a commit over time (ie: the sequence of amends and rebases that occurred with that commit, orthogonal to the history of the branch it is on). If you've used mercurial, this would be a git equivalent to "hg obslog". If you've used gerrit, this would be like the gerrit "change log" but it would work for all commits and work offline. - You can easily list all the changes that you have as works-in progress. If you've used gerrit, this would be an offline equivalent to the gerrit dashboard. - You could choose to share the history of a commit with others, or collaborate on and merge different variants of the same change. Some information about Mercurial's evolve command can be found here: https://www.mercurial-scm.org/doc/evolution/ Other similar technologies: rebase -i can be used to solve the same problem, but you can't easily switch tasks midway through an interactive rebase or have more than one interactive rebase going on at the same time. It also can't handle the case where you have multiple changes sharing the same parent that needs to be rebased and won't let you collaborate with others on resolving a complicated interactive rebase. patch queues (topgit, stgit, quilt) address a very similar problem, however since they're built on top of git rather than integrated with it, most of them end up managing extra state that can get easily damaged whenever you run a native git command that doesn't know about the patch queue. Most of them also have various workflow problems that aren't present in hg evolve. Is anyone else interested in this? Please email me directly or on this list. Let's chat: I want to make sure that whatever we come up with is at least as good as any similar technology that has come before.
Re: [PATCH] fetch-pack: approximate no_dependents with filter
Jonathan Tan writes: > This was prompted by a user at $DAY_JOB who had a partial clone > excluding trees, and had a workflow that only required tree objects (and > not blobs). > > This will hopefully make partial clones excluding trees (with the > "tree:0" filter) a bit better, in that if an operation requires only > trees to be inspected, the required download is much smaller. This seems to break 5520 and 5616 when merged to 'pu'. It seems that merging master to md/filter-trees and then applying this is sufficient to break 5616.
Re: [PATCH] grep: provide a noop --recursive option
Re: Junio C Hamano 2018-09-29 > I also expect folks who are used to "git grep --re" to summon > the only option of the command that begins with that prefix to start > complaining that they now have to type "--recurs" instead. Fwiw I was just asking about -r. There doesn't have to be a corresponding long option if the short one is a no-op. Christoph
[PATCH v2 8/8] reflog expire: cover reflog from all worktrees
Reported-by: Jeff King
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git-reflog.txt | 7 ++-
builtin/reflog.c | 22 +++---
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index 472a6808cd..ff487ff77d 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -20,7 +20,7 @@ depending on the subcommand:
'git reflog' ['show'] [log-options] []
'git reflog expire' [--expire=] [--expire-unreachable=]
[--rewrite] [--updateref] [--stale-fix]
- [--dry-run | -n] [--verbose] [--all | ...]
+ [--dry-run | -n] [--verbose] [--all [--single-worktree] | ...]
'git reflog delete' [--rewrite] [--updateref]
[--dry-run | -n] [--verbose] ref@\{specifier\}...
'git reflog exists'
@@ -72,6 +72,11 @@ Options for `expire`
--all::
Process the reflogs of all references.
+--single-worktree::
+ By default when `--all` is specified, reflogs from all working
+ trees are processed. This option limits the processing to reflogs
+ from the current working tree only.
+
--expire=::
Prune entries older than the specified time. If this option is
not specified, the expiration time is taken from the
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 3acef5a0ab..eed956851e 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -10,6 +10,7 @@
#include "diff.h"
#include "revision.h"
#include "reachable.h"
+#include "worktree.h"
/* NEEDSWORK: switch to using parse_options */
static const char reflog_expire_usage[] =
@@ -52,6 +53,7 @@ struct collect_reflog_cb {
struct collected_reflog **e;
int alloc;
int nr;
+ struct worktree *wt;
};
/* Remember to update object flag allocation in object.h */
@@ -388,8 +390,12 @@ static int collect_reflog(const char *ref, const struct
object_id *oid, int unus
{
struct collected_reflog *e;
struct collect_reflog_cb *cb = cb_data;
+ struct strbuf newref = STRBUF_INIT;
+
+ strbuf_worktree_ref(cb->wt, , ref);
+ FLEX_ALLOC_STR(e, reflog, newref.buf);
+ strbuf_release();
- FLEX_ALLOC_STR(e, reflog, ref);
oidcpy(>oid, oid);
ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);
cb->e[cb->nr++] = e;
@@ -512,7 +518,7 @@ static int cmd_reflog_expire(int argc, const char **argv,
const char *prefix)
{
struct expire_reflog_policy_cb cb;
timestamp_t now = time(NULL);
- int i, status, do_all;
+ int i, status, do_all, all_worktrees = 1;
int explicit_expiry = 0;
unsigned int flags = 0;
@@ -549,6 +555,8 @@ static int cmd_reflog_expire(int argc, const char **argv,
const char *prefix)
flags |= EXPIRE_REFLOGS_UPDATE_REF;
else if (!strcmp(arg, "--all"))
do_all = 1;
+ else if (!strcmp(arg, "--single-worktree"))
+ all_worktrees = 0;
else if (!strcmp(arg, "--verbose"))
flags |= EXPIRE_REFLOGS_VERBOSE;
else if (!strcmp(arg, "--")) {
@@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv,
const char *prefix)
if (do_all) {
struct collect_reflog_cb collected;
+ struct worktree **worktrees, **p;
int i;
memset(, 0, sizeof(collected));
- for_each_reflog(collect_reflog, );
+ worktrees = get_worktrees(0);
+ for (p = worktrees; *p; p++) {
+ if (!all_worktrees && !(*p)->is_current)
+ continue;
+ collected.wt = *p;
+ for_each_reflog(collect_reflog, );
+ }
+ free_worktrees(worktrees);
for (i = 0; i < collected.nr; i++) {
struct collected_reflog *e = collected.e[i];
set_reflog_expiry_param(, explicit_expiry,
e->reflog);
--
2.19.0.341.g3acb95d729
[PATCH v2 2/8] Add a place for (not) sharing stuff between worktrees
When multiple worktrees are used, we need rules to determine if
something belongs to one worktree or all of them. Instead of keeping
adding rules when new stuff comes (*), have a generic rule:
- Inside $GIT_DIR, which is per-worktree by default, add
$GIT_DIR/common which is always shared. New features that want to
share stuff should put stuff under this directory.
- Inside refs/, which is shared by default except refs/bisect, add
refs/worktree/ which is per-worktree. We may eventually move
refs/bisect to this new location and remove the exception in refs
code.
(*) And it may also include stuff from external commands which will
have no way to modify common/per-worktree rules.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git-worktree.txt | 19 ++-
Documentation/gitrepository-layout.txt | 11 +++--
path.c | 2 ++
refs.c | 1 +
refs/files-backend.c | 14 ---
t/t0060-path-utils.sh | 2 ++
t/t1415-worktree-refs.sh | 33 ++
7 files changed, 76 insertions(+), 6 deletions(-)
create mode 100755 t/t1415-worktree-refs.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..a50fbf8094 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,22 @@ working trees, it can be used to identify worktrees. For
example if
you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
then "ghi" or "def/ghi" is enough to point to the former working tree.
+REFS
+
+In multiple working trees, some refs may be shared between all working
+trees, some refs are local. One example is HEAD is different for all
+working trees. This section is about the sharing rules.
+
+In general, all pseudo refs are per working tree and all refs starting
+with "refs/" are shared. Pseudo refs are ones like HEAD which are
+directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
+exception to this: refs inside refs/bisect and refs/worktree is not
+shared.
+
+To access refs, it's best not to look inside GIT_DIR directly. Instead
+use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
+which will handle refs correctly.
+
DETAILS
---
Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +244,8 @@ linked working tree `git rev-parse --git-path HEAD` returns
`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git
rev-parse --git-path refs/heads/master` uses
$GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`,
-since refs are shared across all working trees.
+since refs are shared across all working trees, except refs/bisect and
+refs/worktree.
See linkgit:gitrepository-layout[5] for more information. The rule of
thumb is do not make any assumption about whether a path belongs to
diff --git a/Documentation/gitrepository-layout.txt
b/Documentation/gitrepository-layout.txt
index e85148f05e..89b616e049 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -95,8 +95,10 @@ refs::
References are stored in subdirectories of this
directory. The 'git prune' command knows to preserve
objects reachable from refs found in this directory and
- its subdirectories. This directory is ignored if $GIT_COMMON_DIR
- is set and "$GIT_COMMON_DIR/refs" will be used instead.
+ its subdirectories.
+ This directory is ignored (except refs/bisect and
+ refs/worktree) if $GIT_COMMON_DIR is set and
+ "$GIT_COMMON_DIR/refs" will be used instead.
refs/heads/`name`::
records tip-of-the-tree commit objects of branch `name`
@@ -165,6 +167,11 @@ hooks::
each hook. This directory is ignored if $GIT_COMMON_DIR is set
and "$GIT_COMMON_DIR/hooks" will be used instead.
+common::
+ When multiple working trees are used, most of files in
+ $GIT_DIR are per-worktree with a few known exceptions. All
+ files under 'common' however will be shared between all
+ working trees.
index::
The current index file for the repository. It is
diff --git a/path.c b/path.c
index 34f0f98349..bf4bb02a27 100644
--- a/path.c
+++ b/path.c
@@ -108,6 +108,7 @@ struct common_dir {
static struct common_dir common_list[] = {
{ 0, 1, 0, "branches" },
+ { 0, 1, 0, "common" },
{ 0, 1, 0, "hooks" },
{ 0, 1, 0, "info" },
{ 0, 0, 1, "info/sparse-checkout" },
@@ -118,6 +119,7 @@ static struct common_dir common_list[] = {
{ 0, 1, 0, "objects" },
{ 0, 1, 0, "refs" },
{ 0, 1, 1, "refs/bisect" },
+ { 0, 1, 1, "refs/worktree" },
{ 0, 1, 0, "remotes" },
{ 0, 1, 0, "worktrees" },
{ 0, 1, 0, "rr-cache" },
diff --git a/refs.c b/refs.c
index 9f7268d5fe..1bc4ed301b 100644
--- a/refs.c
+++
[PATCH v2 7/8] fsck: check HEAD and reflog from other worktrees
fsck is a repo-wide operation and should check all references no
matter which worktree they are associated to.
Reported-by: Jeff King
Helped-by: Elijah Newren
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fsck.c | 55 ++---
t/t1450-fsck.sh | 35 +++
2 files changed, 73 insertions(+), 17 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 24f8a09a3c..71492c158d 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -19,6 +19,7 @@
#include "packfile.h"
#include "object-store.h"
#include "run-command.h"
+#include "worktree.h"
#define REACHABLE 0x0001
#define SEEN 0x0002
@@ -444,7 +445,11 @@ static int fsck_handle_reflog_ent(struct object_id *ooid,
struct object_id *noid
static int fsck_handle_reflog(const char *logname, const struct object_id *oid,
int flag, void *cb_data)
{
- for_each_reflog_ent(logname, fsck_handle_reflog_ent, (void *)logname);
+ struct strbuf refname = STRBUF_INIT;
+
+ strbuf_worktree_ref(cb_data, , logname);
+ for_each_reflog_ent(refname.buf, fsck_handle_reflog_ent, refname.buf);
+ strbuf_release();
return 0;
}
@@ -482,20 +487,34 @@ static int fsck_handle_ref(const char *refname, const
struct object_id *oid,
return 0;
}
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+ const char **head_points_at,
struct object_id *head_oid);
static void get_default_heads(void)
{
+ struct worktree **worktrees, **p;
const char *head_points_at;
struct object_id head_oid;
- fsck_head_link(_points_at, _oid);
- if (head_points_at && !is_null_oid(_oid))
- fsck_handle_ref("HEAD", _oid, 0, NULL);
for_each_rawref(fsck_handle_ref, NULL);
- if (include_reflogs)
- for_each_reflog(fsck_handle_reflog, NULL);
+
+ worktrees = get_worktrees(0);
+ for (p = worktrees; *p; p++) {
+ struct worktree *wt = *p;
+ struct strbuf ref = STRBUF_INIT;
+
+ strbuf_worktree_ref(wt, , "HEAD");
+ fsck_head_link(ref.buf, _points_at, _oid);
+ if (head_points_at && !is_null_oid(_oid))
+ fsck_handle_ref(ref.buf, _oid, 0, NULL);
+ strbuf_release();
+
+ if (include_reflogs)
+ refs_for_each_reflog(get_worktree_ref_store(wt),
+fsck_handle_reflog, wt);
+ }
+ free_worktrees(worktrees);
/*
* Not having any default heads isn't really fatal, but
@@ -584,34 +603,36 @@ static void fsck_object_dir(const char *path)
stop_progress();
}
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+ const char **head_points_at,
struct object_id *head_oid)
{
int null_is_error = 0;
if (verbose)
- fprintf(stderr, "Checking HEAD link\n");
+ fprintf(stderr, "Checking %s link\n", head_ref_name);
- *head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+ *head_points_at = resolve_ref_unsafe(head_ref_name, 0, head_oid, NULL);
if (!*head_points_at) {
errors_found |= ERROR_REFS;
- return error("Invalid HEAD");
+ return error("Invalid %s", head_ref_name);
}
- if (!strcmp(*head_points_at, "HEAD"))
+ if (!strcmp(*head_points_at, head_ref_name))
/* detached HEAD */
null_is_error = 1;
else if (!starts_with(*head_points_at, "refs/heads/")) {
errors_found |= ERROR_REFS;
- return error("HEAD points to something strange (%s)",
-*head_points_at);
+ return error("%s points to something strange (%s)",
+head_ref_name, *head_points_at);
}
if (is_null_oid(head_oid)) {
if (null_is_error) {
errors_found |= ERROR_REFS;
- return error("HEAD: detached HEAD points at nothing");
+ return error("%s: detached HEAD points at nothing",
+head_ref_name);
}
- fprintf(stderr, "notice: HEAD points to an unborn branch
(%s)\n",
- *head_points_at + 11);
+ fprintf(stderr, "notice: %s points to an unborn branch (%s)\n",
+ head_ref_name, *head_points_at + 11);
}
return 0;
}
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0f2dd26f74..e97e6a7c6d 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -101,6 +101,41 @@ test_expect_success 'HEAD link pointing at a funny place' '
[PATCH v2 6/8] fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
From: Elijah Newren
This will make it easier to check the HEAD of other worktrees from fsck.
Signed-off-by: Elijah Newren
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fsck.c | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 63c8578cc1..24f8a09a3c 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -36,8 +36,6 @@ static int check_strict;
static int keep_cache_objects;
static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
-static struct object_id head_oid;
-static const char *head_points_at;
static int errors_found;
static int write_lost_and_found;
static int verbose;
@@ -484,8 +482,15 @@ static int fsck_handle_ref(const char *refname, const
struct object_id *oid,
return 0;
}
+static int fsck_head_link(const char **head_points_at,
+ struct object_id *head_oid);
+
static void get_default_heads(void)
{
+ const char *head_points_at;
+ struct object_id head_oid;
+
+ fsck_head_link(_points_at, _oid);
if (head_points_at && !is_null_oid(_oid))
fsck_handle_ref("HEAD", _oid, 0, NULL);
for_each_rawref(fsck_handle_ref, NULL);
@@ -579,33 +584,34 @@ static void fsck_object_dir(const char *path)
stop_progress();
}
-static int fsck_head_link(void)
+static int fsck_head_link(const char **head_points_at,
+ struct object_id *head_oid)
{
int null_is_error = 0;
if (verbose)
fprintf(stderr, "Checking HEAD link\n");
- head_points_at = resolve_ref_unsafe("HEAD", 0, _oid, NULL);
- if (!head_points_at) {
+ *head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+ if (!*head_points_at) {
errors_found |= ERROR_REFS;
return error("Invalid HEAD");
}
- if (!strcmp(head_points_at, "HEAD"))
+ if (!strcmp(*head_points_at, "HEAD"))
/* detached HEAD */
null_is_error = 1;
- else if (!starts_with(head_points_at, "refs/heads/")) {
+ else if (!starts_with(*head_points_at, "refs/heads/")) {
errors_found |= ERROR_REFS;
return error("HEAD points to something strange (%s)",
-head_points_at);
+*head_points_at);
}
- if (is_null_oid(_oid)) {
+ if (is_null_oid(head_oid)) {
if (null_is_error) {
errors_found |= ERROR_REFS;
return error("HEAD: detached HEAD points at nothing");
}
fprintf(stderr, "notice: HEAD points to an unborn branch
(%s)\n",
- head_points_at + 11);
+ *head_points_at + 11);
}
return 0;
}
@@ -720,7 +726,6 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
git_config(fsck_config, NULL);
- fsck_head_link();
if (connectivity_only) {
for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
--
2.19.0.341.g3acb95d729
[PATCH v2 5/8] revision.c: better error reporting on ref from different worktrees
Make use of the new ref aliases to pass refs from another worktree
around and access them from the current ref store instead. This does
not change any functionality, but when a problem arises, we would like
the reported messages to mention full ref aliases, like this:
fatal: bad object worktrees/ztemp/HEAD
warning: reflog of 'main-worktree/HEAD' references pruned commits
instead of
fatal: bad object HEAD
warning: reflog of 'HEAD' references pruned commits
which does not really tell where the refs are from.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
revision.c | 21 +
worktree.c | 32 +---
worktree.h | 14 ++
3 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/revision.c b/revision.c
index 63aae722c1..8ce660e3b1 100644
--- a/revision.c
+++ b/revision.c
@@ -1177,7 +1177,7 @@ struct all_refs_cb {
int warned_bad_reflog;
struct rev_info *all_revs;
const char *name_for_errormsg;
- struct ref_store *refs;
+ struct worktree *wt;
};
int ref_excluded(struct string_list *ref_excludes, const char *path)
@@ -1214,7 +1214,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb,
struct rev_info *revs,
cb->all_revs = revs;
cb->all_flags = flags;
revs->rev_input_given = 1;
- cb->refs = NULL;
+ cb->wt = NULL;
}
void clear_ref_exclusion(struct string_list **ref_excludes_p)
@@ -1277,15 +1277,20 @@ static int handle_one_reflog_ent(struct object_id
*ooid, struct object_id *noid,
return 0;
}
-static int handle_one_reflog(const char *refname,
+static int handle_one_reflog(const char *refname_in_wt,
const struct object_id *oid,
int flag, void *cb_data)
{
struct all_refs_cb *cb = cb_data;
+ struct strbuf refname = STRBUF_INIT;
+
cb->warned_bad_reflog = 0;
- cb->name_for_errormsg = refname;
- refs_for_each_reflog_ent(cb->refs, refname,
+ strbuf_worktree_ref(cb->wt, , refname_in_wt);
+ cb->name_for_errormsg = refname.buf;
+ refs_for_each_reflog_ent(get_main_ref_store(the_repository),
+refname.buf,
handle_one_reflog_ent, cb_data);
+ strbuf_release();
return 0;
}
@@ -1300,8 +1305,8 @@ static void add_other_reflogs_to_pending(struct
all_refs_cb *cb)
if (wt->is_current)
continue;
- cb->refs = get_worktree_ref_store(wt);
- refs_for_each_reflog(cb->refs,
+ cb->wt = wt;
+ refs_for_each_reflog(get_worktree_ref_store(wt),
handle_one_reflog,
cb);
}
@@ -1314,7 +1319,7 @@ void add_reflogs_to_pending(struct rev_info *revs,
unsigned flags)
cb.all_revs = revs;
cb.all_flags = flags;
- cb.refs = get_main_ref_store(the_repository);
+ cb.wt = NULL;
for_each_reflog(handle_one_reflog, );
if (!revs->single_worktree)
diff --git a/worktree.c b/worktree.c
index b0d0b5426d..ec1a5bc511 100644
--- a/worktree.c
+++ b/worktree.c
@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
return ret;
}
+void strbuf_worktree_ref(const struct worktree *wt,
+struct strbuf *sb,
+const char *refname)
+{
+ if (wt && !wt->is_current) {
+ if (is_main_worktree(wt))
+ strbuf_addstr(sb, "main/");
+ else
+ strbuf_addf(sb, "worktrees/%s/", wt->id);
+ }
+ strbuf_addstr(sb, refname);
+}
+
+const char *worktree_ref(const struct worktree *wt, const char *refname)
+{
+ static struct strbuf sb = STRBUF_INIT;
+
+ strbuf_reset();
+ strbuf_worktree_ref(wt, , refname);
+ return sb.buf;
+}
+
int other_head_refs(each_ref_fn fn, void *cb_data)
{
struct worktree **worktrees, **p;
@@ -495,13 +517,17 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
worktrees = get_worktrees(0);
for (p = worktrees; *p; p++) {
struct worktree *wt = *p;
- struct ref_store *refs;
+ struct object_id oid;
+ int flag;
if (wt->is_current)
continue;
- refs = get_worktree_ref_store(wt);
- ret = refs_head_ref(refs, fn, cb_data);
+ if (!refs_read_ref_full(get_main_ref_store(the_repository),
+ worktree_ref(wt, "HEAD"),
+ RESOLVE_REF_READING,
+ , ))
+ ret = fn(worktree_ref(wt, "HEAD"), , flag, cb_data);
if (ret)
break;
}
diff --git a/worktree.h b/worktree.h
index df3fc30f73..0016eb9e88 100644
[PATCH v2 1/8] refs.c: indent with tabs, not spaces
Signed-off-by: Nguyễn Thái Ngọc Duy --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index a7a75b4cc0..9f7268d5fe 100644 --- a/refs.c +++ b/refs.c @@ -646,7 +646,7 @@ enum ref_type ref_type(const char *refname) return REF_TYPE_PER_WORKTREE; if (is_pseudoref_syntax(refname)) return REF_TYPE_PSEUDOREF; - return REF_TYPE_NORMAL; + return REF_TYPE_NORMAL; } long get_files_ref_lock_timeout_ms(void) -- 2.19.0.341.g3acb95d729
[PATCH v2 0/8] fix per-worktree ref iteration in fsck/reflog expire
v2 changes
- more documentation
- main/ prefix is renamed to main-worktree/ to reduce ambiguation
chances and make it clearer
- refs/local is renamed to refs/worktree
- bug fix in is_main_pseudoref_syntax() and
is_other_pseudoref_syntax()
Interdiff
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..58415f9207 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,35 @@ working trees, it can be used to identify worktrees. For
example if
you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
then "ghi" or "def/ghi" is enough to point to the former working tree.
+REFS
+
+In multiple working trees, some refs may be shared between all working
+trees, some refs are local. One example is HEAD is different for all
+working trees. This section is about the sharing rules and how to access
+refs of one working tree from another.
+
+In general, all pseudo refs are per working tree and all refs starting
+with "refs/" are shared. Pseudo refs are ones like HEAD which are
+directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
+exception to this: refs inside refs/bisect and refs/worktree is not
+shared.
+
+Refs that are per working tree can still be accessed from another
+working tree via two special paths main-worktree and worktrees. The
+former gives access to per-worktree refs of the main working tree,
+while the former to all linked working trees.
+
+For example, main-worktree/HEAD or main-worktree/refs/bisect/good
+resolve to the same value as the main working tree's HEAD and
+refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or
+worktrees/bar/refs/bisect/bad are the same as
+GIT_COMMON_DIR/worktrees/foo/HEAD and
+GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad.
+
+To access refs, it's best not to look inside GIT_DIR directly. Instead
+use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
+which will handle refs correctly.
+
DETAILS
---
Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +257,8 @@ linked working tree `git rev-parse --git-path HEAD` returns
`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git
rev-parse --git-path refs/heads/master` uses
$GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`,
-since refs are shared across all working trees.
+since refs are shared across all working trees, except refs/bisect and
+refs/worktree.
See linkgit:gitrepository-layout[5] for more information. The rule of
thumb is do not make any assumption about whether a path belongs to
diff --git a/Documentation/gitrepository-layout.txt
b/Documentation/gitrepository-layout.txt
index fad404ed7c..89b616e049 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -96,9 +96,9 @@ refs::
directory. The 'git prune' command knows to preserve
objects reachable from refs found in this directory and
its subdirectories.
- This directory is ignored (except refs/bisect and refs/local)
- if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/refs" will be
- used instead.
+ This directory is ignored (except refs/bisect and
+ refs/worktree) if $GIT_COMMON_DIR is set and
+ "$GIT_COMMON_DIR/refs" will be used instead.
refs/heads/`name`::
records tip-of-the-tree commit objects of branch `name`
diff --git a/path.c b/path.c
index 7eb61bf31b..bf4bb02a27 100644
--- a/path.c
+++ b/path.c
@@ -119,6 +119,7 @@ static struct common_dir common_list[] = {
{ 0, 1, 0, "objects" },
{ 0, 1, 0, "refs" },
{ 0, 1, 1, "refs/bisect" },
+ { 0, 1, 1, "refs/worktree" },
{ 0, 1, 0, "remotes" },
{ 0, 1, 0, "worktrees" },
{ 0, 1, 0, "rr-cache" },
diff --git a/refs.c b/refs.c
index 90b73c7334..2378b2e7fc 100644
--- a/refs.c
+++ b/refs.c
@@ -624,7 +624,7 @@ int dwim_log(const char *str, int len, struct object_id
*oid, char **log)
static int is_per_worktree_ref(const char *refname)
{
return !strcmp(refname, "HEAD") ||
- starts_with(refname, "refs/local/") ||
+ starts_with(refname, "refs/worktree/") ||
starts_with(refname, "refs/bisect/") ||
starts_with(refname, "refs/rewritten/");
}
@@ -643,7 +643,8 @@ static int is_pseudoref_syntax(const char *refname)
static int is_main_pseudoref_syntax(const char *refname)
{
- return skip_prefix(refname, "main/", ) &&
+ return skip_prefix(refname, "main-worktree/", ) &&
+ *refname &&
is_pseudoref_syntax(refname);
}
@@ -652,7 +653,7 @@ static int is_other_pseudoref_syntax(const char *refname)
if (!skip_prefix(refname, "worktrees/", ))
return 0;
refname = strchr(refname, '/');
- if (!refname)
+ if (!refname || !refname[1])
return 0;
return
[PATCH v2 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
One of the problems with multiple worktree is accessing per-worktree
refs of one worktree from another worktree. This was sort of solved by
multiple ref store, where the code can open the ref store of another
worktree and has access to the ref space of that worktree.
The problem with this is reporting. "HEAD" in another ref space is
also called "HEAD" like in the current ref space. In order to
differentiate them, all the code must somehow carry the ref store
around and print something like "HEAD from this ref store".
But that is not feasible (or possible with a _lot_ of work). With the
current design, we pass a reference around as a string (so called
"refname"). Extending this design to pass a string _and_ a ref store
is a nightmare, especially when handling extended SHA-1 syntax.
So we do it another way. Instead of entering a separate ref space, we
make refs from other worktrees available in the current ref space. So
"HEAD" is always HEAD of the current worktree, but then we can have
"worktrees/blah/HEAD" to denote HEAD from a worktree named
"blah". This syntax coincidentally matches the underlying directory
structure which makes implementation a bit easier.
The main worktree has to be treated specially because well.. it's
special from the beginning. So HEAD from the main worktree is
acccessible via the name "main-worktree/HEAD" instead of
"worktrees/main/HEAD" because "main" could be just another secondary
worktree.
This patch also makes it possible to specify refs from one worktree in
another one, e.g.
git log worktrees/foo/HEAD
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git-worktree.txt | 15 ++-
refs.c | 21
refs.h | 8 +++---
refs/files-backend.c | 28 +
t/t1415-worktree-refs.sh | 46 ++
5 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index a50fbf8094..58415f9207 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -208,7 +208,8 @@ REFS
In multiple working trees, some refs may be shared between all working
trees, some refs are local. One example is HEAD is different for all
-working trees. This section is about the sharing rules.
+working trees. This section is about the sharing rules and how to access
+refs of one working tree from another.
In general, all pseudo refs are per working tree and all refs starting
with "refs/" are shared. Pseudo refs are ones like HEAD which are
@@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs.
There are one
exception to this: refs inside refs/bisect and refs/worktree is not
shared.
+Refs that are per working tree can still be accessed from another
+working tree via two special paths main-worktree and worktrees. The
+former gives access to per-worktree refs of the main working tree,
+while the former to all linked working trees.
+
+For example, main-worktree/HEAD or main-worktree/refs/bisect/good
+resolve to the same value as the main working tree's HEAD and
+refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or
+worktrees/bar/refs/bisect/bad are the same as
+GIT_COMMON_DIR/worktrees/foo/HEAD and
+GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad.
+
To access refs, it's best not to look inside GIT_DIR directly. Instead
use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
which will handle refs correctly.
diff --git a/refs.c b/refs.c
index 1bc4ed301b..2378b2e7fc 100644
--- a/refs.c
+++ b/refs.c
@@ -641,12 +641,33 @@ static int is_pseudoref_syntax(const char *refname)
return 1;
}
+static int is_main_pseudoref_syntax(const char *refname)
+{
+ return skip_prefix(refname, "main-worktree/", ) &&
+ *refname &&
+ is_pseudoref_syntax(refname);
+}
+
+static int is_other_pseudoref_syntax(const char *refname)
+{
+ if (!skip_prefix(refname, "worktrees/", ))
+ return 0;
+ refname = strchr(refname, '/');
+ if (!refname || !refname[1])
+ return 0;
+ return is_pseudoref_syntax(refname + 1);
+}
+
enum ref_type ref_type(const char *refname)
{
if (is_per_worktree_ref(refname))
return REF_TYPE_PER_WORKTREE;
if (is_pseudoref_syntax(refname))
return REF_TYPE_PSEUDOREF;
+ if (is_main_pseudoref_syntax(refname))
+ return REF_TYPE_MAIN_PSEUDOREF;
+ if (is_other_pseudoref_syntax(refname))
+ return REF_TYPE_OTHER_PSEUDOREF;
return REF_TYPE_NORMAL;
}
diff --git a/refs.h b/refs.h
index bd52c1bbae..9b53dbeae8 100644
--- a/refs.h
+++ b/refs.h
@@ -704,9 +704,11 @@ int parse_hide_refs_config(const char *var, const char
*value, const char *);
int ref_is_hidden(const char *, const char *);
enum ref_type {
- REF_TYPE_PER_WORKTREE,
- REF_TYPE_PSEUDOREF,
[PATCH v2 4/8] revision.c: correct a parameter name
This function is a callback of for_each_reflog() which will pass a ref
name as the first argument, not a path (to a reflog file).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
revision.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/revision.c b/revision.c
index e18bd530e4..63aae722c1 100644
--- a/revision.c
+++ b/revision.c
@@ -1277,13 +1277,14 @@ static int handle_one_reflog_ent(struct object_id
*ooid, struct object_id *noid,
return 0;
}
-static int handle_one_reflog(const char *path, const struct object_id *oid,
+static int handle_one_reflog(const char *refname,
+const struct object_id *oid,
int flag, void *cb_data)
{
struct all_refs_cb *cb = cb_data;
cb->warned_bad_reflog = 0;
- cb->name_for_errormsg = path;
- refs_for_each_reflog_ent(cb->refs, path,
+ cb->name_for_errormsg = refname;
+ refs_for_each_reflog_ent(cb->refs, refname,
handle_one_reflog_ent, cb_data);
return 0;
}
--
2.19.0.341.g3acb95d729
Re: [PATCH] Improvement to only call Git Credential Helper once
Jeff King writes: > Wow, what's old is new again. Here's more or less the same patch from > 2012: > > https://public-inbox.org/git/20120407033417.ga13...@sigill.intra.peff.net/ > > Unfortunately, some people seem to rely on this multi-helper behavior. I > recommend reading the whole thread, as there are some interesting bits > in it (that I had always meant to return to, but somehow 6 years went > by). After reading that thread again, my version of summary is that - storing the credential obtained from a helper back to the same helper may be pointless at best and may incur end-user interaction (e.g. asking for symmetric encryption key before the data hits the disk), but it can be used as a "we used what you gave us successfully" ping-back signal. We may not have designed approve() to do "store" back to the same helper by default and instead to do so only when asked by the helper, if we knew better. But an unconditional change of behaviour will break existing users and helpers. - storing the credential obtained from a helper to a different helper may have security implications, and we may not have designed approve() to do "store" by default to all helpers if we knew better. But an unconditional change of behaviour will break existing users and helpers. Assuming that the above summary is accurate, I think the former is easier to solve. In the ideal end game state, we would also have "accepted" in the protocol and call the helper that gave us the accepted credential material with an earlier "get" (if the helper is updated to understand "accepted"). The "accepted" may not even need to receive the credential material as the payload. The latter is trickier, as there are design considerations. - We may want to allow the helper that gives the credential back from the outside world upon "get" request to say "you can 'store' this to other helpers of this kind but not of that kind". If we want to do so, we'd need to extend what is returned from the helper upon "get" (e.g. "get2" will give more than what "get" does back). - We may want to allow the helper that receives the credential material from others to behave differently depending on where it came from, and what other helpers done to it (e.g. even a new credential the end user just typed may not want to go to all helpers; an on-disk helper may feel "I'd take it if the only other helpers that responded to 'store' are the transient 'in-core' kind, but otherwise I'd ignore"). I am not offhand sure what kind of flexibility and protocol extension is necessary. - We may want to be able to override the preference the helper makes in the above two. The user may want to say "Only use this on-disk helper as a read-only source and never call 'store' on it to add new entries (or update an existing one)."
[PATCH v2 0/1] wt-status-state-cleanup
Junio suggested a cleanup patch, jc/wt-status-state-cleanup, which is the basis for this patch. This patch uses ss/wt-status-committable. Updated commit comment and removed one extra blank line insertion. Stephen P. Smith (1): roll wt_status_state into wt_status and populate in the collect phase builtin/commit.c | 3 ++ wt-status.c | 134 +-- wt-status.h | 38 +++--- 3 files changed, 82 insertions(+), 93 deletions(-) -- 2.19.0
[PATCH v2 1/1] roll wt_status_state into wt_status and populate in the collect phase
Status variables were initialized in the collect phase and some
variables were later freed in the print functions.
A "struct wt_status" used to be sufficient for the output phase to
work. It was designed to be filled in the collect phase and consumed
in the output phase, but over time some fields were added and output
phase started filling the fields.
A "struct wt_status_state" that was used in other codepaths turned out
to be useful in the "git status" output. This is not tied to "struct
wt_status", so filling in the collect phase was not consistently
followed.
Move the status state structure variables into the status state
structure and populate them in the collect functions.
Create a new funciton to free the buffers that were being freed in the
print function. Call this new function in commit.c where both the
collect and print functions were being called.
Based on a patch suggestion by Junio C Hamano. [1]
[1] https://public-inbox.org/git/xmqqr2i5ueg4@gitster-ct.c.googlers.com/
Signed-off-by: Stephen P. Smith
---
builtin/commit.c | 3 ++
wt-status.c | 134 +--
wt-status.h | 38 +++---
3 files changed, 82 insertions(+), 93 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d6efd996f8..c91af2fe38 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -508,6 +508,7 @@ static int run_status(FILE *fp, const char *index_file,
const char *prefix, int
wt_status_collect(s);
wt_status_print(s);
+ wt_status_collect_free_buffers(s);
return s->committable;
}
@@ -1390,6 +1391,8 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
s.prefix = prefix;
wt_status_print();
+ wt_status_collect_free_buffers();
+
return 0;
}
diff --git a/wt-status.c b/wt-status.c
index 1822e95f65..cc3e84b997 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -744,21 +744,25 @@ static int has_unmerged(struct wt_status *s)
void wt_status_collect(struct wt_status *s)
{
- struct wt_status_state state;
wt_status_collect_changes_worktree(s);
-
if (s->is_initial)
wt_status_collect_changes_initial(s);
else
wt_status_collect_changes_index(s);
wt_status_collect_untracked(s);
- memset(, 0, sizeof(state));
- wt_status_get_state(, s->branch && !strcmp(s->branch, "HEAD"));
- if (state.merge_in_progress && !has_unmerged(s))
+ wt_status_get_state(>state, s->branch && !strcmp(s->branch, "HEAD"));
+ if (s->state.merge_in_progress && !has_unmerged(s))
s->committable = 1;
}
+void wt_status_collect_free_buffers(struct wt_status *s)
+{
+ free(s->state.branch);
+ free(s->state.onto);
+ free(s->state.detached_from);
+}
+
static void wt_longstatus_print_unmerged(struct wt_status *s)
{
int shown_header = 0;
@@ -1087,8 +1091,7 @@ static void wt_longstatus_print_tracking(struct wt_status
*s)
}
static void show_merge_in_progress(struct wt_status *s,
- struct wt_status_state *state,
- const char *color)
+ const char *color)
{
if (has_unmerged(s)) {
status_printf_ln(s, color, _("You have unmerged paths."));
@@ -1109,16 +1112,15 @@ static void show_merge_in_progress(struct wt_status *s,
}
static void show_am_in_progress(struct wt_status *s,
- struct wt_status_state *state,
const char *color)
{
status_printf_ln(s, color,
_("You are in the middle of an am session."));
- if (state->am_empty_patch)
+ if (s->state.am_empty_patch)
status_printf_ln(s, color,
_("The current patch is empty."));
if (s->hints) {
- if (!state->am_empty_patch)
+ if (!s->state.am_empty_patch)
status_printf_ln(s, color,
_(" (fix conflicts and then run \"git am
--continue\")"));
status_printf_ln(s, color,
@@ -1242,10 +1244,9 @@ static int read_rebase_todolist(const char *fname,
struct string_list *lines)
}
static void show_rebase_information(struct wt_status *s,
- struct wt_status_state *state,
- const char *color)
+ const char *color)
{
- if (state->rebase_interactive_in_progress) {
+ if (s->state.rebase_interactive_in_progress) {
int i;
int nr_lines_to_show = 2;
@@ -1296,28 +1297,26 @@ static void show_rebase_information(struct wt_status *s,
}
static void print_rebase_state(struct wt_status *s,
- struct wt_status_state *state,
- const char *color)
+
Re: [PATCH 7/8] fsck: check HEAD and reflog from other worktrees
On Sun, Sep 23, 2018 at 04:41:32AM -0400, Eric Sunshine wrote: > > + grep "main/HEAD: detached HEAD points" out > > This message doesn't get localized, so no need for > test_i18ngrep(). Okay. Don't remind me. I started to mark all strings in fsck for translation and faced the reality that I would need to change a lot to test_i18ngrep :( -- Duy
Re: [PATCH] fetch: fix compilation warning
Ramsay Jones writes: > You probably already know, but I had to add this on top of the 'pu' > branch to get a clean compile tonight (your 'jc/war-on-string-list' > branch). It was not just about squelching a warning but simply broken code that deserved to be warned/errored. I think what we have in 'pu' now is already fixed. Thanks. > } > hashmap_free(_refs, 1); > - string_list_clear(_refs, 0); > + string_list_clear(_refs_list, 0); > } > > static struct ref *get_ref_map(struct remote *remote,
Re: [PATCH] fetch: replace string-list used as a look-up table with a hashmap
Jeff King writes:
> On Wed, Sep 26, 2018 at 03:59:08PM -0700, Stefan Beller wrote:
>
>> > +struct refname_hash_entry {
>> > + struct hashmap_entry ent; /* must be the first member */
>>
>> $ git grep "struct hashmap_entry" reveals that we have another
>> convention that we follow but not document, which is to stress
>> the importance of putting the hashmap_entry first. ;-)
>
> One thing I've liked about the list.h implementation is that you can
> store the list pointer anywhere in the struct, or even have the same
> struct in multiple lists.
>
> The only funny thing is that you have to "dereference" the iterator like
> this:
>
> struct list_head *pos;
> struct actual_thing *item;
> ...
> item = list_entry(pos, struct actual_thing, list_member);
>
> which is a minor pain, but it's reasonably hard to get it wrong.
>
> I wonder if we could do the same here. The hashmap would only ever see
> the "struct hashmap_entry", and then the caller would convert that back
> to the actual type.
Hmph, how would hashmap_cmp_fn look like with that scheme? It would
get one entry, another entry (or just the skeleton of it) and
optionally a separate keydata (if the second one is skeleton), and
the first two points at the embedded hashmap struct, not the
surrounding one. The callback function is now responsible for
calling a hashmap_entry() macro that adjusts for the negative offset
like list_entry() does?
> I think we could even get away with not converting
> existing callers; if the hashmap _is_ at the front, then that
> list_entry() really just devolves to a cast.
Yes.
> So as long as the struct
> definition and the users of the struct agree, it would just work.
Yes, too.
Was it ever a consideration, when allowing struct list-head anywhere
in the enclosing struct, that it would allow an element to be on
more than one list? Would it benefit us to be able to place an
element in multiple hashmaps because we do not have to have the
embedded hashmap_entry always at the beginning if we did this
change?
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
On Tue, Sep 25, 2018 at 11:16 PM Junio C Hamano wrote: > > Nguyễn Thái Ngọc Duy writes: > > > The main worktree has to be treated specially because well.. it's > > special from the beginning. So HEAD from the main worktree is > > acccessible via the name "main/HEAD" (we can't use > > "worktrees/main/HEAD" because "main" under "worktrees" is not > > reserved). > > I do not quite follow. So with this, both refs/heads/master and > main/refs/heads/master are good names for the master branch (even > though the local branch names are not per worktree), because > in the main worktree, refs/bisect/bad and main/refs/bisect/bad ought > to mean the same thing. True. I think the ambiguation here is about the main worktree versus a secondary worktree that is accidentally named "main". Then suddenly we have to worktrees of the same name, and accessing them both via worktrees//HEAD will not work, and there is no other way to disambiguate them. > side note: Or is this only for pseudo-refs > (i.e. $GIT_DIR/$name where $name consists of all caps or > underscore and typically ends with HEAD)? Right now, due to implementation limitations, only pseudo refs (or loose refs in the case of refs/bisect) are accessible. But I don't see why main/refs/heads/master should not work. > The disambiguation rule has always been: if you have a confusingly > named ref, you can spell it out fully to avoid any ambiguity, e.g. > refs/heads/refs/heads/foo can be given to "git rev-parse" and will > mean the tip of the branch whose name is "refs/heads/foo", even when > another branch whose name is "foo" exists. > > Would we have a reasonable disambiguation rules that work well with > the main/ and worktrees/* prefixes? When somebody has main/HEAD branch > and writes "git rev-parse main/HEAD", does it find refs/heads/main/HEAD > or $GIT_DIR/HEAD, if the user is in the main worktree? The rules are not touched. But it looks like everything still works as expected (I'm adding tests to verify this) -- Duy
Re: [PATCH v6 3/7] eoie: add End of Index Entry (EOIE) extension
Duy Nguyen writes:
> On Wed, Sep 26, 2018 at 03:54:38PM -0400, Ben Peart wrote:
>> +
>> +#define EOIE_SIZE (4 + GIT_SHA1_RAWSZ) /* <4-byte offset> + <20-byte hash>
>> */
>> +#define EOIE_SIZE_WITH_HEADER (4 + 4 + EOIE_SIZE) /* <4-byte signature> +
>> <4-byte length> + EOIE_SIZE */
>
> If you make these variables instead of macros, you can use
> the_hash_algo, which makes this code sha256-friendlier and probably
> can explain less, e.g. ...
>
>> +
>> +static size_t read_eoie_extension(const char *mmap, size_t mmap_size)
>> +{
>> +/*
>> + * The end of index entries (EOIE) extension is guaranteed to be last
>> + * so that it can be found by scanning backwards from the EOF.
>> + *
>> + * "EOIE"
>> + * <4-byte length>
>> + * <4-byte offset>
>> + * <20-byte hash>
20? ;-)
>> + */
>
> uint32_t EOIE_SIZE = 4 + the_hash_algo->rawsz;
> uint32_t EOIE_SIZE_WITH_HEADER = 4 + 4 + EOIE_SIZE;
>
>> +const char *index, *eoie;
>> +uint32_t extsize;
>> +size_t offset, src_offset;
>> +unsigned char hash[GIT_MAX_RAWSZ];
>> +git_hash_ctx c;
> --
> Duy
Re: [PATCH] help: allow redirecting to help for aliased command
Jeff King writes: > Right, I'm proposing only to add the extra message and then continue as > usual. > > It is a little funny, I guess, if you have a script which doesn't > respond to "-h", because you'd get our "foo is aliased to git-bar" > message to stderr, followed by who-knows-what. But as long as it's to > stderr (and not stdout), I think it's not likely to _break_ anything. > >> > - "git cp --help" opens the manpage for cherry-pick. We don't bother >> > with the alias definition, as it's available through other means >> > (and thus we skip the obliteration/timing thing totally). >> >> It sounds like you suggest doing this unconditionally, and without any >> opt-in via config option or a short wait? That would certainly work for >> me. It is, in fact, how I expect 'git cp --help' to work, until I get >> reminded that it does not... Also, as Junio noted, is consistent with >> --help generally providing more information than -h - except that one >> loses the 'is an alias for' part for --help. > > Yes, I'd suggest doing it always. No config, no wait. While I do think your suggestion is the best among various ones floated in the thread, I just realized there is one potential glitch even with that approach. Suppose "git foo" is aliased to a command "git bar". The best case is when "git bar -h" knows that it is asked to give us a short usage. We get "foo is aliased to bar" followed by the short usage for "bar" and everything is visible above the shell prompt after all that happens. The second best case is when "git bar" simply does not support "-h" but actively notices an unknown option on the command line to give the usage message. We see "foo is aliased to bar" followed by "-h is an unknown option; supported options are ..." and everything is visible above the shell prompt after all that happens. The worst case is when "git bar" supports or ignores "-h" and produces reams of output. Sending the "aliased to" message to the standard error means that it is scrolled out when the output is done, or lost even when "git foo -h | less" attempts to let the reader read before the early part of the output scrolls away. Even the first two "better" cases share the same glitch if the "foo is aliased to bar" goes to the standard error output. Parse-options enabled commands tend to show a long "-h" output that you would need to say "git grep -h | less", losing the "aliased to" message. At least it seems to me an improvement to use standard output, instead of standard error, for the alias information. In practice, however, what the command that "git foo" is aliased to does when given "-h" is probably unknown (because the user is asking what "git foo" is in the first place), so perhaps I am worried too much. When the user does not know if the usage text comes to the standard output or to the standard error, and if the usage text is very long or not, they probably would learn quickly that the safest thing to do is to $ git unknown-command -h >&2 | less And at that point, it does not matter which between the standard output and the standard error streams we write "unknown-command is aliased to ...". So I dunno.
Re: [PATCH] grep: provide a noop --recursive option
Ævar Arnfjörð Bjarmason writes: > This --recursive (-r) option does nothing, and is purely here to > appease people who have "grep -r ..." burned into their muscle memory. > > Requested-by: Christoph Berg > Signed-off-by: Ævar Arnfjörð Bjarmason > --- I personally am not all that sympathetic to the "'git grep' and 'grep' sound similar, so even though it won't do anything useful just add a synonym to noop" line of reasoning. That will lead to sloppy noop, and will invite unbound amount of busywork to deal with future complaints like "oh, but 'grep GNU COPYING' does not give useless filename in front like the same command line with 'git' prefixed; please fix 'git grep'" (which we'd have to say "no", wasting our time). I however do not mind if we added "--recursive" with matching "--no-recursive", and - made "--recursive" the default (obviously) - made "--no-recursive" a synonym to setting the recursion limit to "never recurse" - and made "--recursive" a synonym to setting the recursion limit to "infinity". That would be more work than this patch. But if I see "--recursive" advertised as a feature, and the command by default goes recursive, I do expect to be able to tell it not to recurse. I also expect folks who are used to "git grep --re" to summon the only option of the command that begins with that prefix to start complaining that they now have to type "--recurs" instead. I am not solving that with the above suggestion to improve the suggested "noop".
Re: [BUG] Segfault in "git submodule"
I have a repo, but it appears to be specific to staging area state. It only segfaults when I have a certain file deleted. Where do you want me to upload it? On Sat, Sep 29, 2018 at 8:34 AM Duy Nguyen wrote: > > On Sat, Sep 29, 2018 at 5:31 PM Ævar Arnfjörð Bjarmason > wrote: > > > #1 refs_resolve_ref_unsafe (refs=0x0, > > > refname=refname@entry=0x55e863062253 "HEAD", > > > resolve_flags=resolve_flags@entry=1, oid=oid@entry=0x7ffdc834b1c0, > > > flags=flags@entry=0x7ffdc834b1bc) at refs.c:1493 > > refs is NULL. It looks like somebody fails to get the right submodule > ref store (or tries to get it but fails to check if it may return > NULL) > -- > Duy
Re: [PATCH] grep: provide a noop --recursive option
On Sat, Sep 29, 2018 at 5:25 PM Ævar Arnfjörð Bjarmason
wrote:
>
>
> On Sat, Sep 29 2018, Duy Nguyen wrote:
>
> > On Sat, Sep 29, 2018 at 4:58 PM Ævar Arnfjörð Bjarmason
> > wrote:
> >>
> >> This --recursive (-r) option does nothing, and is purely here to
> >> appease people who have "grep -r ..." burned into their muscle memory.
> >
> > GNU grep -r recurses infinitely but Git grep also has --max-depth. How
> > do these interact? My knee-jerk reaction is -r equals --max-depth=-1
> > (i.e. overriding previous --mex-depth options on command line, or from
> > alias)
>
> I didn't know about --max-depth, we could squash the following in to
> deal with that, but that still leaves --max-depth=123 --no-recursive as
> using depth 123, and we'd need different options parsing than OPT_BOOL
> to deal with that.
I think if you initialize recursive variable below to -1, then you can
detect both --recursive (1) and --no-recursive (0).
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 02d4384225..2e048d9b49 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -785,7 +785,7 @@ int cmd_grep(int argc, const char **argv, const char
> *prefix)
> int use_index = 1;
> int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
> int allow_revs;
> - int unused_recursive; /* this is never used */
> + int recursive = 0;
>
> struct option options[] = {
> OPT_BOOL(0, "cached", ,
> @@ -803,7 +803,7 @@ int cmd_grep(int argc, const char **argv, const char
> *prefix)
> N_("show non-matching lines")),
> OPT_BOOL('i', "ignore-case", _case,
> N_("case insensitive matching")),
> - OPT_BOOL('r', "recursive", _recursive,
> + OPT_BOOL('r', "recursive", ,
> N_("does nothing, git-grep is always recursive, for
> grep(1) muscle memory compatibility")),
> OPT_BOOL('w', "word-regexp", _regexp,
> N_("match patterns only at word boundaries")),
> @@ -926,6 +926,8 @@ int cmd_grep(int argc, const char **argv, const char
> *prefix)
> PARSE_OPT_STOP_AT_NON_OPTION);
> grep_commit_pattern_type(pattern_type_arg, );
>
> + if (opt.max_depth != -1 && recursive)
> + die(_("The --max-depth and --recursive options are
> incompatible"));
> if (use_index && !startup_info->have_repository) {
> int fallback = 0;
> git_config_get_bool("grep.fallbacktonoindex", );
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index c48d1fa34b..ad25cd50f5 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -1697,4 +1697,8 @@ test_expect_success 'grep does not report i-t-a and
> assume unchanged with -L' '
> test_cmp expected actual
> '
>
> +test_expect_success 'grep --recursive is incompatible with --max-depth' '
> + test_must_fail git grep --recursive --max-depth=1
> +'
> +
> test_done
--
Duy
Re: [BUG] Segfault in "git submodule"
On Sat, Sep 29, 2018 at 5:31 PM Ævar Arnfjörð Bjarmason wrote: > > #1 refs_resolve_ref_unsafe (refs=0x0, > > refname=refname@entry=0x55e863062253 "HEAD", > > resolve_flags=resolve_flags@entry=1, oid=oid@entry=0x7ffdc834b1c0, > > flags=flags@entry=0x7ffdc834b1bc) at refs.c:1493 refs is NULL. It looks like somebody fails to get the right submodule ref store (or tries to get it but fails to check if it may return NULL) -- Duy
[PATCH v2 2/2] worktree: add per-worktree config files
A new repo extension is added, worktreeConfig. When it is present: - Repository config reading by default includes $GIT_DIR/config _and_ $GIT_DIR/config.worktree. "config" file remains shared in multiple worktree setup. - The special treatment for core.bare and core.worktree, to stay effective only in main worktree, is gone. These config settings are supposed to be in config.worktree. This extension is most useful in multiple worktree setup because you now have an option to store per-worktree config (which is either .git/config.worktree for main worktree, or .git/worktrees/xx/config.worktree for linked ones). This extension can be used in single worktree mode, even though it's pretty much useless (but this can happen after you remove all linked worktrees and move back to single worktree). "git config" reads from both "config" and "config.worktree" by default (i.e. without either --user, --file...) when this extension is present. Default writes still go to "config", not "config.worktree". A new option --worktree is added for that (*). Since a new repo extension is introduced, existing git binaries should refuse to access to the repo (both from main and linked worktrees). So they will not misread the config file (i.e. skip the config.worktree part). They may still accidentally write to the config file anyway if they use with "git config --file ". This design places a bet on the assumption that the majority of config variables are shared so it is the default mode. A safer move would be default writes go to per-worktree file, so that accidental changes are isolated. (*) "git config --worktree" points back to "config" file when this extension is not present and there is only one worktree so that it works in any both single and multiple worktree setups. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/config.txt | 12 +++- Documentation/git-config.txt | 26 ++--- Documentation/git-worktree.txt | 30 ++ Documentation/gitrepository-layout.txt | 8 +++ builtin/config.c | 19 ++- cache.h| 2 + config.c | 11 environment.c | 1 + setup.c| 40 ++--- t/t2029-worktree-config.sh | 79 ++ 10 files changed, 210 insertions(+), 18 deletions(-) create mode 100755 t/t2029-worktree-config.sh diff --git a/Documentation/config.txt b/Documentation/config.txt index 8d85d1a324..44407e69db 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2,8 +2,9 @@ CONFIGURATION FILE -- The Git configuration file contains a number of variables that affect -the Git commands' behavior. The `.git/config` file in each repository -is used to store the configuration for that repository, and +the Git commands' behavior. The files `.git/config` and optionally +`config.worktree` (see `extensions.worktreeConfig` below) in each +repository is used to store the configuration for that repository, and `$HOME/.gitconfig` is used to store a per-user configuration as fallback values for the `.git/config` file. The file `/etc/gitconfig` can be used to store a system-wide default configuration. @@ -371,6 +372,13 @@ advice.*:: editor input from the user. -- +extensions.worktreeConfig:: + If set, by default "git config" reads from both "config" and + "config.worktree" file from GIT_DIR in that order. In + multiple working directory mode, "config" file is shared while + "config.worktree" is per-working directory (i.e., it's in + GIT_COMMON_DIR/worktrees//config.worktree) + core.fileMode:: Tells Git if the executable bit of files in the working tree is to be honored. diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 8e240435be..4870e00b89 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -45,13 +45,15 @@ unset an existing `--type` specifier with `--no-type`. When reading, the values are read from the system, global and repository local configuration files by default, and options -`--system`, `--global`, `--local` and `--file ` can be -used to tell the command to read from only that location (see <>). +`--system`, `--global`, `--local`, `--worktree` and +`--file ` can be used to tell the command to read from only +that location (see <>). When writing, the new value is written to the repository local configuration file by default, and options `--system`, `--global`, -`--file ` can be used to tell the command to write to -that location (you can say `--local` but that is the default). +`--worktree`, `--file ` can be used to tell the command to +write to that location (you can say `--local` but that is the +default). This command will fail with non-zero status upon error. Some exit codes are: @@ -131,6 +133,11 @@ from
[PATCH v2 1/2] t1300: extract and use test_cmp_config()
In many config-related tests it's common to check if a config variable has expected value and we want to print the differences when the test fails. Doing it the normal way is three lines of shell code. Let's add a function do to all this (and a little more). This function has uses outside t1300 as well but I'm not going to convert them all. And it will be used in the next commit where per-worktree config feature is introduced. Signed-off-by: Nguyễn Thái Ngọc Duy --- t/t1300-config.sh | 79 ++--- t/test-lib-functions.sh | 24 + 2 files changed, 43 insertions(+), 60 deletions(-) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index cdf1fed5d1..00c2b0f0eb 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -76,15 +76,11 @@ EOF test_expect_success 'non-match result' 'test_cmp expect .git/config' test_expect_success 'find mixed-case key by canonical name' ' - echo Second >expect && - git config cores.whatever >actual && - test_cmp expect actual + test_cmp_config Second cores.whatever ' test_expect_success 'find mixed-case key by non-canonical name' ' - echo Second >expect && - git config CoReS.WhAtEvEr >actual && - test_cmp expect actual + test_cmp_config Second CoReS.WhAtEvEr ' test_expect_success 'subsections are not canonicalized by git-config' ' @@ -94,12 +90,8 @@ test_expect_success 'subsections are not canonicalized by git-config' ' [section "SubSection"] key = two EOF - echo one >expect && - git config section.subsection.key >actual && - test_cmp expect actual && - echo two >expect && - git config section.SubSection.key >actual && - test_cmp expect actual + test_cmp_config one section.subsection.key && + test_cmp_config two section.SubSection.key ' cat > .git/config <<\EOF @@ -212,9 +204,7 @@ test_expect_success 'really really mean test' ' ' test_expect_success 'get value' ' - echo alpha >expect && - git config beta.haha >actual && - test_cmp expect actual + test_cmp_config alpha beta.haha ' cat > expect << EOF @@ -251,15 +241,11 @@ test_expect_success 'non-match' ' ' test_expect_success 'non-match value' ' - echo wow >expect && - git config --get nextsection.nonewline !for >actual && - test_cmp expect actual + test_cmp_config wow --get nextsection.nonewline !for ' test_expect_success 'multi-valued get returns final one' ' - echo "wow2 for me" >expect && - git config --get nextsection.nonewline >actual && - test_cmp expect actual + test_cmp_config "wow2 for me" --get nextsection.nonewline ' test_expect_success 'multi-valued get-all returns all' ' @@ -520,21 +506,11 @@ test_expect_success 'editing stdin is an error' ' test_expect_success 'refer config from subdirectory' ' mkdir x && - ( - cd x && - echo strasse >expect && - git config --get --file ../other-config ein.bahn >actual && - test_cmp expect actual - ) - + test_cmp_config -C x strasse --get --file ../other-config ein.bahn ' test_expect_success 'refer config from subdirectory via --file' ' - ( - cd x && - git config --file=../other-config --get ein.bahn >actual && - test_cmp expect actual - ) + test_cmp_config -C x strasse --file=../other-config --get ein.bahn ' cat > expect << EOF @@ -688,16 +664,13 @@ test_expect_success numbers ' test_expect_success '--int is at least 64 bits' ' git config giga.watts 121g && - echo 129922760704 >expect && - git config --int --get giga.watts >actual && - test_cmp expect actual + echo >expect && + test_cmp_config 129922760704 --int --get giga.watts ' test_expect_success 'invalid unit' ' git config aninvalid.unit "1auto" && - echo 1auto >expect && - git config aninvalid.unit >actual && - test_cmp expect actual && + test_cmp_config 1auto aninvalid.unit && test_must_fail git config --int --get aninvalid.unit 2>actual && test_i18ngrep "bad numeric config value .1auto. for .aninvalid.unit. in file .git/config: invalid unit" actual ' @@ -1039,9 +1012,7 @@ test_expect_success '--null --get-regexp' ' test_expect_success 'inner whitespace kept verbatim' ' git config section.val "foo bar" && - echo "foo bar" >expect && - git config section.val >actual && - test_cmp expect actual + test_cmp_config "foo bar" section.val ' test_expect_success SYMLINKS 'symlinked configuration' ' @@ -1808,21 +1779,15 @@ big = 1M EOF test_expect_success 'identical modern --type specifiers are allowed' ' - git config --type=int --type=int core.big >actual && - echo 1048576 >expect && - test_cmp expect actual +
[PATCH v2 0/2] Per-worktree config files
Besides various doc/test changes. v2 has two code changes - "git config --worktree" on a multiple worktree setup _without_ the new extension will now barf. The user is supposed to RTFM and enable the extension manually. - I failed to update setup code to pick up config from per-worktree files. Granted, core.bare and core.worktree don't have much use in this context (except perhaps submodules). But let's make sure we have proper code path to handle it. config.worktree should have a new scope, CONFIG_SCOPE_WORKTREE, on top of CONFIG_SCOPE_REPO. But we would need to go over scope-sensitive code (upload-pack and remote) to update if necessary. I don't think it's worth getting into that just yet. Range diff -: -- > 1: 8f334e0020 t1300: extract and use test_cmp_config() 1: 12a999e517 ! 2: 99337a52f2 worktree: add per-worktree config files @@ -9,7 +9,7 @@ worktree setup. - The special treatment for core.bare and core.worktree, to stay - effective only in main worktree, is gone. These config files are + effective only in main worktree, is gone. These config settings are supposed to be in config.worktree. This extension is most useful in multiple worktree setup because you @@ -38,7 +38,8 @@ isolated. (*) "git config --worktree" points back to "config" file when this -extension is not present so that it works in any setup. +extension is not present and there is only one worktree so that it +works in any both single and multiple worktree setups. diff --git a/Documentation/config.txt b/Documentation/config.txt --- a/Documentation/config.txt @@ -50,7 +51,7 @@ -the Git commands' behavior. The `.git/config` file in each repository -is used to store the configuration for that repository, and +the Git commands' behavior. The files `.git/config` and optionally -+`config.worktree` (see `extensions.worktreeConfig` below) are each ++`config.worktree` (see `extensions.worktreeConfig` below) in each +repository is used to store the configuration for that repository, and `$HOME/.gitconfig` is used to store a per-user configuration as fallback values for the `.git/config` file. The file `/etc/gitconfig` @@ -61,9 +62,10 @@ +extensions.worktreeConfig:: + If set, by default "git config" reads from both "config" and -+ "config.worktree" file in that order. In multiple working -+ directory mode, "config" file is shared while -+ "config.worktree" is per-working directory. ++ "config.worktree" file from GIT_DIR in that order. In ++ multiple working directory mode, "config" file is shared while ++ "config.worktree" is per-working directory (i.e., it's in ++ GIT_COMMON_DIR/worktrees//config.worktree) + core.fileMode:: Tells Git if the executable bit of files in the working tree @@ -140,12 +142,12 @@ +CONFIGURATION FILE +-- +By default, the repository "config" file is shared across all working -+directories. If the config variables `core.bare` or `core.worktree` -+are already present in the config file, they will be applied to the -+main working directory only. ++trees. If the config variables `core.bare` or `core.worktree` are ++already present in the config file, they will be applied to the main ++working trees only. + -+In order to have configuration specific to working directories, you -+can turn on "worktreeConfig" extension, e.g.: ++In order to have configuration specific to working trees, you can turn ++on "worktreeConfig" extension, e.g.: + + +$ git config extensions.worktreeConfig true @@ -153,26 +155,19 @@ + +In this mode, specific configuration stays in the path pointed by `git +rev-parse --git-path config.worktree`. You can add or update -+configuration in this file with `git config --worktree`. Git before -+version 2.20.0 will refuse to access repositories with this extension. ++configuration in this file with `git config --worktree`. Older Git ++versions may will refuse to access repositories with this extension. + -+Note that in this file, the exception for `core.bare` and -+`core.worktree` is gone. If you have them before, you need to move -+them to the `config.worktree` of the main working directory. You may -+also take this opportunity to move other configuration that you do not -+want to share to all working directories: ++Note that in this file, the exception for `core.bare` and `core.worktree` ++is gone. If you have them in $GIT_DIR/config before, you must move ++them to the `config.worktree` of the main working tree. You may also ++take this opportunity to review and move other configuration that you ++do not want to share to all
Re: [BUG] Segfault in "git submodule"
On Sat, Sep 29 2018, Raymond Jennings wrote: > [New LWP 19644] > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/lib64/libthread_db.so.1". > Core was generated by `git submodule--helper status'. > Program terminated with signal SIGSEGV, Segmentation fault. > #0 refs_read_raw_ref (type=, referent=, > oid=, refname=, ref_store= out>) at refs.c:1451 > 1451 return ref_store->be->read_raw_ref(ref_store, refname, oid, > referent, type); > #0 refs_read_raw_ref (type=, referent=, > oid=, refname=, ref_store= out>) at refs.c:1451 > #1 refs_resolve_ref_unsafe (refs=0x0, > refname=refname@entry=0x55e863062253 "HEAD", > resolve_flags=resolve_flags@entry=1, oid=oid@entry=0x7ffdc834b1c0, > flags=flags@entry=0x7ffdc834b1bc) at refs.c:1493 > #2 0x55e862fcad5c in refs_read_ref_full (flags=0x7ffdc834b1bc, > oid=0x7ffdc834b1c0, resolve_flags=1, refname=0x55e863062253 "HEAD", > refs=) at refs.c:224 > #3 refs_head_ref (refs=, fn=fn@entry=0x55e862f25fb0 > , cb_data=cb_data@entry=0x7ffdc834b300) at > refs.c:1314 > #4 0x55e862f292a2 in status_submodule (flags=0, prefix= out>, ce_flags=0, ce_oid=0x55e86468d4d4, path=0x55e86468d4e8 > "lpc-doc") at builtin/submodule--helper.c:624 > #5 status_submodule_cb (cb_data=0x7ffdc834b240, > list_item=0x55e86468d490) at builtin/submodule--helper.c:665 > #6 for_each_listed_submodule (cb_data=, fn= out>, list=) at builtin/submodule--helper.c:404 > #7 module_status (argc=, argv=, > prefix=) at builtin/submodule--helper.c:698 > #8 0x55e862eaec95 in run_builtin (argv=, > argc=, p=) at git.c:346 > #9 handle_builtin (argc=, argv=) at git.c:554 > #10 0x55e862eaf985 in run_argv (argv=0x7ffdc834be20, > argcp=0x7ffdc834be2c) at git.c:606 > #11 cmd_main (argc=, argv=) at git.c:683 > #12 0x55e862eae96a in main (argc=3, argv=0x7ffdc834c078) at > common-main.c:43 Can you consistently reproduce this? Maybe Stefan can make some sense of this, but it would be really useful if you could compile git from the master branch with CFLAGS="-O0 -g" and run gdb with that and paste that backtrace, and even better if you're in a position to share a copy of the repository where this happens.
Re: [PATCH] grep: provide a noop --recursive option
On Sat, Sep 29 2018, Duy Nguyen wrote:
> On Sat, Sep 29, 2018 at 4:58 PM Ævar Arnfjörð Bjarmason
> wrote:
>>
>> This --recursive (-r) option does nothing, and is purely here to
>> appease people who have "grep -r ..." burned into their muscle memory.
>
> GNU grep -r recurses infinitely but Git grep also has --max-depth. How
> do these interact? My knee-jerk reaction is -r equals --max-depth=-1
> (i.e. overriding previous --mex-depth options on command line, or from
> alias)
I didn't know about --max-depth, we could squash the following in to
deal with that, but that still leaves --max-depth=123 --no-recursive as
using depth 123, and we'd need different options parsing than OPT_BOOL
to deal with that.
diff --git a/builtin/grep.c b/builtin/grep.c
index 02d4384225..2e048d9b49 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -785,7 +785,7 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
int use_index = 1;
int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
int allow_revs;
- int unused_recursive; /* this is never used */
+ int recursive = 0;
struct option options[] = {
OPT_BOOL(0, "cached", ,
@@ -803,7 +803,7 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
N_("show non-matching lines")),
OPT_BOOL('i', "ignore-case", _case,
N_("case insensitive matching")),
- OPT_BOOL('r', "recursive", _recursive,
+ OPT_BOOL('r', "recursive", ,
N_("does nothing, git-grep is always recursive, for
grep(1) muscle memory compatibility")),
OPT_BOOL('w', "word-regexp", _regexp,
N_("match patterns only at word boundaries")),
@@ -926,6 +926,8 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
PARSE_OPT_STOP_AT_NON_OPTION);
grep_commit_pattern_type(pattern_type_arg, );
+ if (opt.max_depth != -1 && recursive)
+ die(_("The --max-depth and --recursive options are
incompatible"));
if (use_index && !startup_info->have_repository) {
int fallback = 0;
git_config_get_bool("grep.fallbacktonoindex", );
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index c48d1fa34b..ad25cd50f5 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1697,4 +1697,8 @@ test_expect_success 'grep does not report i-t-a and
assume unchanged with -L' '
test_cmp expected actual
'
+test_expect_success 'grep --recursive is incompatible with --max-depth' '
+ test_must_fail git grep --recursive --max-depth=1
+'
+
test_done
[BUG] Segfault in "git submodule"
[New LWP 19644] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". Core was generated by `git submodule--helper status'. Program terminated with signal SIGSEGV, Segmentation fault. #0 refs_read_raw_ref (type=, referent=, oid=, refname=, ref_store=) at refs.c:1451 1451 return ref_store->be->read_raw_ref(ref_store, refname, oid, referent, type); #0 refs_read_raw_ref (type=, referent=, oid=, refname=, ref_store=) at refs.c:1451 #1 refs_resolve_ref_unsafe (refs=0x0, refname=refname@entry=0x55e863062253 "HEAD", resolve_flags=resolve_flags@entry=1, oid=oid@entry=0x7ffdc834b1c0, flags=flags@entry=0x7ffdc834b1bc) at refs.c:1493 #2 0x55e862fcad5c in refs_read_ref_full (flags=0x7ffdc834b1bc, oid=0x7ffdc834b1c0, resolve_flags=1, refname=0x55e863062253 "HEAD", refs=) at refs.c:224 #3 refs_head_ref (refs=, fn=fn@entry=0x55e862f25fb0 , cb_data=cb_data@entry=0x7ffdc834b300) at refs.c:1314 #4 0x55e862f292a2 in status_submodule (flags=0, prefix=, ce_flags=0, ce_oid=0x55e86468d4d4, path=0x55e86468d4e8 "lpc-doc") at builtin/submodule--helper.c:624 #5 status_submodule_cb (cb_data=0x7ffdc834b240, list_item=0x55e86468d490) at builtin/submodule--helper.c:665 #6 for_each_listed_submodule (cb_data=, fn=, list=) at builtin/submodule--helper.c:404 #7 module_status (argc=, argv=, prefix=) at builtin/submodule--helper.c:698 #8 0x55e862eaec95 in run_builtin (argv=, argc=, p=) at git.c:346 #9 handle_builtin (argc=, argv=) at git.c:554 #10 0x55e862eaf985 in run_argv (argv=0x7ffdc834be20, argcp=0x7ffdc834be2c) at git.c:606 #11 cmd_main (argc=, argv=) at git.c:683 #12 0x55e862eae96a in main (argc=3, argv=0x7ffdc834c078) at common-main.c:43
Re: Git for Windows for Unix?
On Fri, Sep 28 2018, Jonathan Nieder wrote: > Ævar Arnfjörð Bjarmason wrote: >> On Thu, Sep 27 2018, Jonathan Nieder wrote: > >>> That said, that seems to me like a lot of work to avoid adding some >>> patches to "next" that belong in "next" anyway. I understand why the >>> Git for Windows maintainer does not always have time to upstream >>> promptly, which is why I suggest working with him to find a way to >>> help with that. >>> >>> If there's something I'm missing and Git is actually an uncooperative >>> upstream like the cases you've mentioned, then I'd be happy to learn >>> about that so we can fix it, too. >> >> That's one and valid way to look at it, convergence would be ideal. >> >> Another way to look at it, which is closer to what I was thinking about, >> is to just view GFW as some alternate universe "next" branch (which by >> my count is ~2-3k commits ahead of master[1]). > > You could view it that way, but I don't. Many Git for Windows patches > have never even visited the Git mailing list. Not to beat this point to death, just replying because I'm not sure if we're talking past each other, or if you're just vehemently making the point that these patches *really* should be sent to the ML (which I don't disagree with at all). I meant "alternative universe 'next'" in the sense that it's a published branch that's shipped to people and is consistently ahead of master, not that the same process of sending patches to git@vger.kernel.org has been used to generate it. In the case of GFW they have their own internal contribution mechanism, GitHub PRs etc., and only eventually submit things to the ML.
Re: [PATCH] grep: provide a noop --recursive option
On Sat, Sep 29, 2018 at 4:58 PM Ævar Arnfjörð Bjarmason
wrote:
>
> This --recursive (-r) option does nothing, and is purely here to
> appease people who have "grep -r ..." burned into their muscle memory.
GNU grep -r recurses infinitely but Git grep also has --max-depth. How
do these interact? My knee-jerk reaction is -r equals --max-depth=-1
(i.e. overriding previous --mex-depth options on command line, or from
alias)
> Requested-by: Christoph Berg
> Signed-off-by: Ævar Arnfjörð Bjarmason
> ---
>
> On Sat, Sep 29, 2018 at 4:10 PM Christoph Berg wrote:
> >
> > I often use "grep -r $pattern" to recursively grep a source tree. If
> > that takes too long, I hit ^C and tag "git" in front of the command
> > line and re-run it. git then complains "error: unknown switch `r'"
> > because "git grep" is naturally recursive.
> >
> > Could we have "git grep -r" accept the argument for compatibility?
> > Other important grep switches like "-i" are compatible, adding -r
> > would improve usability.
>
> I don't have an opinion on this either way, it doesn't scratch my
> itch, but hey, why not. Here's a patch to implement it.
>
> Documentation/git-grep.txt | 6 ++
> builtin/grep.c | 3 +++
> t/t7810-grep.sh| 8
> 3 files changed, 17 insertions(+)
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index a3049af1a3..a1aea8be4e 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -290,6 +290,12 @@ providing this option will cause it to die.
> Do not output matched lines; instead, exit with status 0 when
> there is a match and with non-zero status when there isn't.
>
> +-r::
> +--recursive::
> + This option does nothing. git-grep is always recursive. This
> + noop option is provided for compatibility with the muscle
> + memory of people used to grep(1).
> +
> ...::
> Instead of searching tracked files in the working tree, search
> blobs in the given trees.
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 601f801158..02d4384225 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -785,6 +785,7 @@ int cmd_grep(int argc, const char **argv, const char
> *prefix)
> int use_index = 1;
> int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
> int allow_revs;
> + int unused_recursive; /* this is never used */
>
> struct option options[] = {
> OPT_BOOL(0, "cached", ,
> @@ -802,6 +803,8 @@ int cmd_grep(int argc, const char **argv, const char
> *prefix)
> N_("show non-matching lines")),
> OPT_BOOL('i', "ignore-case", _case,
> N_("case insensitive matching")),
> + OPT_BOOL('r', "recursive", _recursive,
> + N_("does nothing, git-grep is always recursive, for
> grep(1) muscle memory compatibility")),
> OPT_BOOL('w', "word-regexp", _regexp,
> N_("match patterns only at word boundaries")),
> OPT_SET_INT('a', "text", ,
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index be5c1bd553..c48d1fa34b 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -469,6 +469,14 @@ do
> git grep --count -h -e b $H -- ab >actual &&
> test_cmp expected actual
> '
> +
> + for flag in '' ' -r' ' --recursive'
> + do
> + test_expect_success "grep $flag . (testing that --recursive
> is a noop)" '
> + git grep$flag . >actual &&
> + test_line_count = 43 actual
> + '
> + done
> done
>
> cat >expected < --
> 2.19.0.605.g01d371f741
>
--
Duy
[PATCH] grep: provide a noop --recursive option
This --recursive (-r) option does nothing, and is purely here to
appease people who have "grep -r ..." burned into their muscle memory.
Requested-by: Christoph Berg
Signed-off-by: Ævar Arnfjörð Bjarmason
---
On Sat, Sep 29, 2018 at 4:10 PM Christoph Berg wrote:
>
> I often use "grep -r $pattern" to recursively grep a source tree. If
> that takes too long, I hit ^C and tag "git" in front of the command
> line and re-run it. git then complains "error: unknown switch `r'"
> because "git grep" is naturally recursive.
>
> Could we have "git grep -r" accept the argument for compatibility?
> Other important grep switches like "-i" are compatible, adding -r
> would improve usability.
I don't have an opinion on this either way, it doesn't scratch my
itch, but hey, why not. Here's a patch to implement it.
Documentation/git-grep.txt | 6 ++
builtin/grep.c | 3 +++
t/t7810-grep.sh| 8
3 files changed, 17 insertions(+)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index a3049af1a3..a1aea8be4e 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -290,6 +290,12 @@ providing this option will cause it to die.
Do not output matched lines; instead, exit with status 0 when
there is a match and with non-zero status when there isn't.
+-r::
+--recursive::
+ This option does nothing. git-grep is always recursive. This
+ noop option is provided for compatibility with the muscle
+ memory of people used to grep(1).
+
...::
Instead of searching tracked files in the working tree, search
blobs in the given trees.
diff --git a/builtin/grep.c b/builtin/grep.c
index 601f801158..02d4384225 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -785,6 +785,7 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
int use_index = 1;
int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
int allow_revs;
+ int unused_recursive; /* this is never used */
struct option options[] = {
OPT_BOOL(0, "cached", ,
@@ -802,6 +803,8 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
N_("show non-matching lines")),
OPT_BOOL('i', "ignore-case", _case,
N_("case insensitive matching")),
+ OPT_BOOL('r', "recursive", _recursive,
+ N_("does nothing, git-grep is always recursive, for
grep(1) muscle memory compatibility")),
OPT_BOOL('w', "word-regexp", _regexp,
N_("match patterns only at word boundaries")),
OPT_SET_INT('a', "text", ,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index be5c1bd553..c48d1fa34b 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -469,6 +469,14 @@ do
git grep --count -h -e b $H -- ab >actual &&
test_cmp expected actual
'
+
+ for flag in '' ' -r' ' --recursive'
+ do
+ test_expect_success "grep $flag . (testing that --recursive is
a noop)" '
+ git grep$flag . >actual &&
+ test_line_count = 43 actual
+ '
+ done
done
cat >expected <
wishlist: git grep -r
I often use "grep -r $pattern" to recursively grep a source tree. If that takes too long, I hit ^C and tag "git" in front of the command line and re-run it. git then complains "error: unknown switch `r'" because "git grep" is naturally recursive. Could we have "git grep -r" accept the argument for compatibility? Other important grep switches like "-i" are compatible, adding -r would improve usability. Thanks, Christoph signature.asc Description: PGP signature
Re: [PATCH] worktree: add per-worktree config files
On Mon, Sep 24, 2018 at 4:21 PM Taylor Blau wrote:
> > @@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char
> > *prefix)
> >PARSE_OPT_STOP_AT_NON_OPTION);
> >
> > if (use_global_config + use_system_config + use_local_config +
> > + use_worktree_config +
> > !!given_config_source.file + !!given_config_source.blob > 1) {
>
> I feel like we're getting into "let's extract a function" territory,
> here, since this line is growing in width. Perhaps:
>
> static int config_files_count()
> {
> return use_global_config + use_system_config + use_local_config +
> use_worktree_config +
> !!given_config_source.file +
> !!given_config_source.blob;
> }
>
> Simplifying the call to:
I think there's a bigger problem here because we have no good way to
describe that a bunch of options are mutually exclusive. We get by ok
if the number of options is two, but when it gets bigger (and this is
not the only place), it gets messier. Besides the long "if" condition,
I consider the error message "only one config file at a time"
inadequate. We should tell the user what exact options are
conflicting.
So, I'm going to bury my head in the sand this time and ignore the
problem, including adding config_files_count(). It could be a
interesting mini project if someone wants to jump in. Meanwhile it'll
go to the very bottom of my long long backlog.
--
Duy
Re: Git for Windows for Unix?
On Fri, Sep 28, 2018 at 09:57:11PM +0200, Ævar Arnfjörð Bjarmason wrote: > Another way to look at it, which is closer to what I was thinking about, > is to just view GFW as some alternate universe "next" branch (which by > my count is ~2-3k commits ahead of master[1]). > 1. $ git log --max-parents=1 --pretty=format:%s > v2.19.0...v2.19.0.windows.1|sort|uniq|wc -l >2346 This command is missing a grep -v -i -E '(^fixup!|^squash!|mingw|vcxproj|msvc|win32|vs2015|windows)' from the middle of the pipe, then it's "only" ~1500.
Re: [PATCH v3 5/6] split-index: don't compare stat data of entries already marked for split index
On Sat, Sep 29, 2018 at 11:14:29AM +0200, SZEDER Gábor wrote: > > > Note that comparing the cached data in copied and original entries in > > > > s/cached data/cached stat data/ ? I was confused for a bit. > > No, it's indeed cached data, but now that you mention it, the subject > line does need a s/stat //. Or rather s/stat/cached/
Re: [PATCH v3 5/6] split-index: don't compare stat data of entries already marked for split index
On Sat, Sep 29, 2018 at 07:36:08AM +0200, Duy Nguyen wrote:
> On Fri, Sep 28, 2018 at 06:24:58PM +0200, SZEDER Gábor wrote:
> > When unpack_trees() constructs a new index, it copies cache entries
> > from the original index [1]. prepare_to_write_split_index() has to
> > deal with this, and it has a dedicated code path for copied entries
> > that are present in the shared index, where it compares the cached
> > data in the corresponding copied and original entries. If the cached
> > data matches, then they are considered the same; if it differs, then
> > the copied entry will be marked for inclusion as a replacement entry
> > in the just about to be written split index by setting the
> > CE_UPDATE_IN_BASE flag.
> >
> > However, a cache entry already has its CE_UPDATE_IN_BASE flag set upon
> > reading the split index, if the entry already has a replacement entry
> > there, or upon refreshing the cached stat data, if the corresponding
> > file was modified. The state of this flag is then preserved when
> > unpack_trees() copies a cache entry from the shared index.
> >
> > So modify prepare_to_write_split_index() to check the copied cache
> > entries' CE_UPDATE_IN_BASE flag first, and skip the thorough
> > comparison of cached data if the flag is already set.
>
> OK so this is an optimization, not a bug fix. Right?
Well, a microoptimization at most: with all what's going on in
unpack_trees() I seriously doubt that it's effect is measurable.
> > Note that comparing the cached data in copied and original entries in
>
> s/cached data/cached stat data/ ? I was confused for a bit.
No, it's indeed cached data, but now that you mention it, the subject
line does need a s/stat //.
The comparison is done with this call:
ret = memcmp(>ce_stat_data, >ce_stat_data,
offsetof(struct cache_entry, name) -
offsetof(struct cache_entry, ce_stat_data));
i.e. it starts at the stat data and ends just before the cache entry's
name, and 'struct cache_entry' has several other fields between these
two, including e.g. the cached oid:
struct cache_entry {
struct hashmap_entry ent;
struct stat_data ce_stat_data;
unsigned int ce_mode;
unsigned int ce_flags;
unsigned int mem_pool_allocated;
unsigned int ce_namelen;
unsigned int index; /* for link extension */
struct object_id oid;
char name[FLEX_ARRAY]; /* more */
};
However, to me it's mostly about clarity of the code, and about
documenting that CE_UPDATE_IN_BASE might have already been set at that
point and why, so the next dev diving in to debug the split index
doesn't have to figure this out himself.
> > the shared index might actually be entirely unnecessary. In theory
> > all code paths refreshing the cached stat data of an entry in the
> > shared index should set the CE_UPDATE_IN_BASE flag in that entry, and
> > unpack_trees() should preserve this flag when copying cache entries.
> > This means that the cached data is only ever changed if the
> > CE_UPDATE_IN_BASE flag is set as well. Our test suite seems to
> > confirm this: instrumenting the conditions in question and running the
> > test suite repeatedly with 'GIT_TEST_SPLIT_INDEX=yes' showed that the
> > cached data in a copied entry differs from the data in the shared
> > entry only if its CE_UPDATE_IN_BASE flag is indeed set.
>
> Yes I was probably just being paranoid (or sticking to simpler
> checks). I was told that split index is computation expensive and not
> doing unnecesary/expensive checks may help. But let's leave it for
> later.
>
> > + } else {
> > + /*
> > +* Thoroughly compare the cached data to see
> > +* whether it should be marked for inclusion
> > +* in the split index.
> > +*
> > +* This comparison might be unnecessary, as
> > +* code paths modifying the cached data do
> > +* set CE_UPDATE_IN_BASE as well.
> > +*/
> > + const unsigned int ondisk_flags =
> > + CE_STAGEMASK | CE_VALID |
> > + CE_EXTENDED_FLAGS;
> > + unsigned int ce_flags, base_flags, ret;
> > + ce_flags = ce->ce_flags;
> > + base_flags = base->ce_flags;
> > + /* only on-disk flags matter */
> > + ce->ce_flags &= ondisk_flags;
> > + base->ce_flags &= ondisk_flags;
> > + ret = memcmp(>ce_stat_data,
> > >ce_stat_data,
> > +offsetof(struct cache_entry, name)
> > -
> > +offsetof(struct
Re: [PATCH] help: allow redirecting to help for aliased command
On Fri, Sep 28, 2018 at 10:18:05AM +0200, Rasmus Villemoes wrote: > > it, and then it is up to the alias to handle "-h" sensibly. > > I'd be nervous about doing this, though, especially if we introduce this > without a new opt-in config option (which seems to be the direction the > discussion is taking). There are lots of commands that don't respond > with a help message to -h, or that only recognize -h as the first word, > or... There are really too many ways this could cause headaches. > > But, now that I test it, it seems that we already let the alias handle > -h (and any other following words, with --help as the first word > special-cased). So what you're suggesting is (correct me if I'm wrong) > to _also_ intercept -h as the first word, and then print the alias info, > in addition to spawning the alias with the entire argv as usual. The > alias info would probably need to go to stderr in this case. Right, I'm proposing only to add the extra message and then continue as usual. It is a little funny, I guess, if you have a script which doesn't respond to "-h", because you'd get our "foo is aliased to git-bar" message to stderr, followed by who-knows-what. But as long as it's to stderr (and not stdout), I think it's not likely to _break_ anything. > > - "git cp --help" opens the manpage for cherry-pick. We don't bother > > with the alias definition, as it's available through other means > > (and thus we skip the obliteration/timing thing totally). > > It sounds like you suggest doing this unconditionally, and without any > opt-in via config option or a short wait? That would certainly work for > me. It is, in fact, how I expect 'git cp --help' to work, until I get > reminded that it does not... Also, as Junio noted, is consistent with > --help generally providing more information than -h - except that one > loses the 'is an alias for' part for --help. Yes, I'd suggest doing it always. No config, no wait. -Peff
Re: [PATCH] Improvement to only call Git Credential Helper once
On Fri, Sep 28, 2018 at 12:37:16PM -0400, Kyle Hubert wrote: > When calling the Git Credential Helper that is set in the git config, > the get command can return a credential. Git immediately turns around > and calls the store command, even though that credential was just > retrieved by the Helper. This creates two side effects. First of all, > if the Helper requires a passphrase, the user has to type it in > twice. Secondly, if the user has a number of helpers, this retrieves > the credential from one service and writes it to all services. > > This commit introduces a new field in the credential struct that > detects when the credential was retrieved using the Helper, and early > exits when called to store the credential. Wow, what's old is new again. Here's more or less the same patch from 2012: https://public-inbox.org/git/20120407033417.ga13...@sigill.intra.peff.net/ Unfortunately, some people seem to rely on this multi-helper behavior. I recommend reading the whole thread, as there are some interesting bits in it (that I had always meant to return to, but somehow 6 years went by). I'm not entirely opposed to breaking the current behavior in the name of better security (namely not unexpectedly propagating credentials), but it would be nice if we provided better tools for people to let their helpers interact (like the credential-wrap thing I showed in that thread). > --- > credential.c | 8 +++- > credential.h | 3 ++- > 2 files changed, 9 insertions(+), 2 deletions(-) I know your patch is right, because it's almost identical to mine. :) (Mine didn't use the "retrieved" flag in the middle, but just set "approved" directly). If we do go this route, though, we might want to steal the test from that earlier round. -Peff
Re: [PATCH] t1400: drop debug `echo` to actually execute `test`
On Fri, Sep 28, 2018 at 05:43:59PM +0200, Martin Ågren wrote: > Instead of running `test "foo" = "$(bar)"`, we prefix the whole thing > with `echo`. Comparing to nearby tests makes it clear that this is just > debug leftover. This line has actually been modified four times since it > was introduced in e52290428b (General ref log reading improvements., > 2006-05-19) and the `echo` has always survived. Let's finally drop it. Hmm, yeah. I cannot see how this echo was ever accomplishing anything. > This script could need some more cleanups. This is just an immediate fix > so that we actually test what we intend to. Yeah, this is definitely worth doing now, and not holding hostage to a bigger cleanup. > --- > t/t1400-update-ref.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) The patch looks good to me. Thanks! -Peff
Re: [PATCH v3 6/6] split-index: smudge and add racily clean cache entries to split index
On Sat, Sep 29, 2018 at 07:21:36AM +0200, Duy Nguyen wrote: > On Fri, Sep 28, 2018 at 06:24:59PM +0200, SZEDER Gábor wrote: > > diff --git a/t/t1701-racy-split-index.sh b/t/t1701-racy-split-index.sh > > index fbb77046da..5dc221ef38 100755 > > --- a/t/t1701-racy-split-index.sh > > +++ b/t/t1701-racy-split-index.sh > > @@ -148,7 +148,7 @@ done > > > > for trial in $trials > > do > > - test_expect_failure "update the split index when a racily clean cache > > entry is stored only in the shared index $trial" ' > > + test_expect_success "update the split index when a racily clean cache > > entry is stored only in the shared index #$trial" ' > > The the new '#' before '$trial' intended? Yes, they only cause troubles for 'prove' (or in the TAP output in general) in 'test_expect_failure', all the previous tests already have '#$trial', just like the related tests in 't0010-racy-git.sh'. And many more tests have '#' in their names: $ git grep 'test_expect_success.*#' master |wc -l 121 > > > rm -f .git/index .git/sharedindex.* && > > > > # The next three commands must be run within the same > > @@ -170,8 +170,6 @@ do > > # entry of racy-file is only stored in the shared index. > > # A corresponding replacement cache entry with smudged > > # stat data should be added to the new split index. > > - # > > - # Alas, such a smudged replacement entry is not added! > > git update-index --add other-file && > > > > # Subsequent git commands should notice the smudged > > @@ -182,7 +180,7 @@ done > > > > for trial in $trials > > do > > - test_expect_failure "update the split index after unpack trees() copied > > a racily clean cache entry from the shared index $trial" ' > > + test_expect_success "update the split index after unpack trees() copied > > a racily clean cache entry from the shared index #$trial" ' > > rm -f .git/index .git/sharedindex.* && > > > > # The next three commands must be run within the same > -- > Duy
Re: Request for examples on git log --format:tformat
On Fri, Sep 28, 2018 at 11:29:32AM -0700, Daniel Lo wrote: > I was reviewing the tformat parameters on: > https://git-scm.com/docs/git-log (middle of the page). > > Specifically: %<|(): make the next placeholder take at least until > Nth columns, padding spaces on the right if necessary > > I found the instructions regard space formatting to be very confusing. > An example would be helpful to illustrate what the proper space > formatting syntax is: > > Ex: > git log --format="tformat:%h %<(15)%an %s" > > 0123456 Author Name Commit message - author name is formatted to be > padded with space to occupy at least 15 characters > > All of the special symbols %<|(<>) made me confused to what was > required and what was describing the syntax. I'm not sure if you're asking for somebody to give an example here, or suggesting that the documentation should contain an example. If the former, then an example matching the documentation you quoted is: git log --format='%h %<|(15)%an %s' The difference (I think -- I've never actually use either of these in the wild myself) between %< and %<| is that the former pads out to N spaces, and the latter pads out until we've reached the Nth column (so taking into account all prior content on the line, too). To see the difference try: # pad names to 30 chars git log --format='%h %<(30)%an %s' versus # pad out to the 30th column, including the hash git log --format='%h %<|(30)%an %s' versus # pad out to the 30th column, but without the hash there should be # much more whitespace git log --format='%<|(30)%an %s' If you are suggesting that there should be some examples in the documentation, I agree (I had to stare at the descriptions and run a few tests myself to figure this out). I'm not sure if they should go near the placeholder list, or in the examples section. -Peff PS Orthogonal to your question, but one tip: "--format" defaults to "tformat:" if its argument contains %-placeholders.
Re: [PATCH] Documentation/CodingGuidelines: How to document new APIs
On Fri, Sep 28, 2018 at 09:50:14AM -0700, Junio C Hamano wrote: > -- >8 -- > Subject: CodingGuidelines: document the API in *.h files > > It makes it harder to let the API description and the reality drift > apart if the doc is kept close to the implementation or the header > of the API. We have been slowly migrating API docs out of the > Documentation/technical/api-* to *.h files, and the development > community generally considers that how inline docs in strbuf.h is > done the best current practice. > > We recommend documenting in the header over documenting near the > implementation to encourage people to write the docs that are > readable without peeking at the implemention. Yeah, I agree with all of that rationale. > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines > index 6d265327c9..e87090c849 100644 > --- a/Documentation/CodingGuidelines > +++ b/Documentation/CodingGuidelines > @@ -385,7 +385,11 @@ For C programs: > string_list for sorted string lists, a hash map (mapping struct > objects) named "struct decorate", amongst other things. > > - - When you come up with an API, document it. > + - When you come up with an API, document it the functions and the > + structures in the header file that exposes the API to its callers. > + Use what is in "strbuf.h" as a model to decide the appropriate tone > + in which the description is given, and the level of details to put > + in the description. I like the general idea here. I had trouble parsing the "in which the description is given". Maybe just: When you come up with an API, document its functions and structures in the header file that exposes the API to its callers. Use what is in "strbuf.h" as a model for the appropriate tone and level of detail. I like the idea you mentioned elsewhere of polishing up strbuf.h to serve as the model (but I don't want to hold up this much simpler patch if that seems likely to drag on). Thanks for pushing this towards a concrete conclusion. -Peff
Re: [PATCH] Documentation/CodingGuidelines: How to document new APIs
On Fri, Sep 28, 2018 at 10:14:12AM -0700, Stefan Beller wrote: > > I think other high-level concepts that are _not_ APIs (e.g., file > > formats, protocol, etc) could go into technical/. > > That is what I meant with high level concepts. Anything that talks > about or implies the existence of a function is clearly header level. Ah, OK. I thought you meant the "this is how strbuf roughly works" overview, which IMHO should remain in strbuf.h. I think we are on the same page, then. > > (Though actually, those are the thing that I would not mind at all if > > they get formatted into real manpages and shipped to end users. We do > > not expect most users to dissect our file-formats, but they could at > > least be useful to somebody poking around). > > Formats are sensible thing to present to the end user. I was also thinking > about partial-clone, which is a concept rather than a format. Yeah, definitely. Another good example is technical/api-credentials. The section on credential helpers there probably ought to be in more user-visible documentation (probably gitcredentials(7)). -Peff
Re: [PATCH] strbuf.h: format according to coding guidelines
On Fri, Sep 28, 2018 at 01:11:26PM -0700, Junio C Hamano wrote: > > I am mildly against giving names to obvious ones. > > Just to make sure nobody listening from sidelines do not > misunderstand, ones like getwholeline_fd() that takes more than one > parameter with the same time is a prime example of a function that > take non-obvious parameters that MUST be named. > > What I am mildly against is the rule that says "name ALL > parameters". I tend to think these names make headers harder to > read, and prefer to keep them to the minimum. > > I actually do not mind the rule to be more like > > * Use the same parameter names used in the function declaration when >the description in the API documentation refers the parameter. Yes, I agree very much with that rule (and your genera line of thinking). I am not personally against just naming every parameter, but I simply don't care either way. -Peff
Re: [PATCH v3 4/4] transport.c: introduce core.alternateRefsPrefixes
On Fri, Sep 28, 2018 at 03:05:57PM -0700, Taylor Blau wrote: > > > For example, to advertise only tags, a caller using > > > 'core.alternateRefsCommand' would have to do: > > > > > > $ git config core.alternateRefsCommand ' \ > > > git -C "$1" for-each-ref refs/tags --format="%(objectname)"' > > > > This has the same "$@" issue as the previous one, I think (which only > > makes your point about it being cumbersome more true!). > > Hmm. I'll be curious to how you respond to my other message about the > same topic. I feel that whatever the outcome there is will affect both > locations in the same way. I think they're separate issues, right? I was just confused on the earlier patch, but the "git config" command you show above is the actual broken case isn't it? I'm not overly concerned since this isn't recommending the technique to end users (and in fact the whole point is to give an alternative), but it may be worth showing a working command in case anybody runs across it. -Peff
Re: [PATCH v3 3/4] transport.c: introduce core.alternateRefsCommand
On Fri, Sep 28, 2018 at 03:04:10PM -0700, Taylor Blau wrote:
> > Well, you also need to pass the path so it knows which repo to look at.
> > Which I think is the primary reason we do it, but behaving differently
> > for each alternate is another option.
>
> Yeah. I think that the clearer argument is yours, so I'll amend my copy.
> I am thinking of:
>
> To find the alternate, pass its absolute path as the first argument.
>
> How does that sound?
Sounds good.
> > > +heads, configure `core.alternateRefsCommand` to the path of a script
> > > which runs
> > > +`git --git-dir="$1" for-each-ref --format='%(objectname)' refs/heads`.
> >
> > Does that script actually work? Because of the way we invoke shell
> > commands with arguments, I think we'd end up with:
> >
> > git --git-dir="$1" for-each-ref --format='%(objectname)' refs/heads "$@"
> [...]
> ...but this was what I was trying to get across with saying "...to the
> path of a script which runs...", such that we would get the implicit
> scoping that you make explicit in your example with "f() { ... }; f".
>
> Does that seem OK as-is after the additional context? I think that after
> reading your response, it seems to be confusing, so perhaps it should be
> changed...
Ah, OK. I totally missed that "path of a script" part. What you have is
correct, then, but I do wonder if we could make it less subtle.
Maybe something like:
For example, if `/path/to/script` runs `git --git-dir="$1"
for-each-ref --format='%(objectname)' refs/heads/`, then putting
`/path/to/script` in `core.alternateRefsCommand` will show only the
branch heads from the alternate.
I dunno. It's certainly clunkier. I wonder if we would be less awkward
to show the sample script in a fenced block, with the `#!/bin/sh` and
everything.
Or maybe just keep the text you have and add a note at the end like:
Note that writing that `for-each-ref` command directly in the config
option doesn't quite work, as it has to handle the path argument
specially.
I don't think we need to hand-hold a user through the f() shell-snippet
trickery. I just don't want somebody thinking they can blindly paste
that into their config.
> > The other alternative is to pass $GIT_DIR in the environment on behalf
> > of the program. Then writing:
> >
> > git for-each-ref --format='%(objectname)' refs/heads
> >
> > would Just Work. But it's a bit subtle, since it is not immediately
> > obvious that the command is meant to run in a different repository.
>
> I think that we discussed this approach a bit off-list, and I had the
> idea that it was too fragile to work in practice, and that it would be
> too surprising for callers to suddenly be in a different world.
>
> I say this not because it wouldn't make this particular scenario more
> convenient, which it uncountably would, but because it would make other
> scenarios _more_ complicated.
>
> For example, if a caller uses an alternate reference backed, perhaps,
> MySQL (or anything that _isn't_ Git), they're not going to want to have
> these GIT_ environment variable set.
If they're not using Git under the hood, then GIT_* probably isn't
hurting anything. But it is still pretty subtle. Let's forget I
mentioned it. Just chaining for-each-ref with a prefix is pretty
awkward, but that's why we have the next patch with
alternateRefsPrefixes.
Your response did make me think of one other thing, though. The
alternate file points to a directory with objects, and the
for_each_alternate_ref() code checks to see if that looks vaguely like
the objects/ directory of a git repo. But would anybody want to run
something like alternateRefsCommand on _just_ the object directory?
I.e., you don't have a real git repo there, but your script can
"somehow" come up with a list of valid tips.
That isn't inconceivable to me for the kind of multi-fork storage we do
at GitHub. E.g., imagine a shared object directory with no refs, and
then a script that goes out to the other related forks to look at their
ref tips. I don't think we have any immediate plans for it, though (and
there are a lot of subtle bits that I won't go into here that make it
non-trivial). So I'm OK to punt on it for now. I also think in a pinch
that you could easily fool the alternates code by just having a dummy
"refs/" directory.
> > > diff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh
> [...]
> > > +test_description='git receive-pack test'
> >
> > The name of this test file and the description are pretty vague. Can we
> > say something like "test handling of receive-pack with alternate-refs
> > config"?
>
> I left it intentionally vague, since I'd like for it to contain more
> tests about 'git receive-pack'-specific things in the future.
>
> I'm happy to change the name, though I wonder if we should change the
> filename accordingly, and if so, to what.
I think we'd want to have a separate script for other receive-pack tests
that aren't related to alternates. There's some startup
Re: [PATCH] config.txt: correct the note about uploadpack.packObjectsHook
On Sat, Sep 29, 2018 at 08:50:56AM +0200, Nguyễn Thái Ngọc Duy wrote: > Document for uploadpack.packObjectsHook is added in [1] and consists > of two paragraphs, the second one is quite important about where this > variable can stay. > > When the paragraph about uploadpack.allowFilter is added in [2], it's > added in between the two paragraphs. This makes the "this is non-repo > level config" note incorrectly apply to allowFilter instead of > packObjectsHook. Move allowFilter paragraph down to fix this. Nice catch, and the patch looks obviously correct. Thanks. -Peff
[PATCH] config.txt: correct the note about uploadpack.packObjectsHook
Document for uploadpack.packObjectsHook is added in [1] and consists of two paragraphs, the second one is quite important about where this variable can stay. When the paragraph about uploadpack.allowFilter is added in [2], it's added in between the two paragraphs. This makes the "this is non-repo level config" note incorrectly apply to allowFilter instead of packObjectsHook. Move allowFilter paragraph down to fix this. [1] 20b20a22f8 (upload-pack: provide a hook for running pack-objects - 2016-05-18) [2] 10ac85c785 (upload-pack: add object filtering for partial clone - 2017-12-08) Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/config.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ad0f4510c3..907b1d0cb9 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -3666,15 +3666,15 @@ uploadpack.packObjectsHook:: was run. I.e., `upload-pack` will feed input intended for `pack-objects` to the hook, and expects a completed packfile on stdout. - -uploadpack.allowFilter:: - If this option is set, `upload-pack` will support partial - clone and partial fetch object filtering. + Note that this configuration variable is ignored if it is seen in the repository-level config (this is a safety measure against fetching from untrusted repositories). +uploadpack.allowFilter:: + If this option is set, `upload-pack` will support partial + clone and partial fetch object filtering. + uploadpack.allowRefInWant:: If this option is set, `upload-pack` will support the `ref-in-want` feature of the protocol version 2 `fetch` command. This feature -- 2.19.0.341.g3acb95d729
Re: [PATCH] worktree: add per-worktree config files
On Thu, Sep 27, 2018 at 8:34 PM Ævar Arnfjörð Bjarmason wrote: > I see I'm misremembering most of the details here. I thought that if I put: > > [remote "whatever] > url = ... > > Into my ~/.gitconfig that it wouldn't work, but it does, e.g. here in my > ~/g/git: > > $ grep -A1 whatever .git/config > $ > $ grep -A1 whatever ~/.gitconfig > [remote "whatever"] > url = g...@github.com:test/git.git > > But there's still some special casing for .git/config going on, > e.g. here: > > $ git config remote.origin.url > g...@github.com:git/git.git > $ git config remote.whatever.url > g...@github.com:test/git.git > $ git remote get-url origin > g...@github.com:git/git.git > $ git remote get-url whatever > fatal: No such remote 'whatever' > > And: > > $ git remote set-url whatever g...@github.com:test2/git.git > fatal: No such remote 'whatever' > > So there is some special casing of .git/config somewhere. I looked into > this ages ago, and don't remember where that's done. To conclude this thread. Yes some code does know about where the config variable is from and it looks like only "git remote" and "git upload-pack" takes advantage of it [1] [2]. I considered documentation improvement for the git-remote part, but I don't see anywhere I can fit "some remote attributes can be shared from ~/.gitconfig, but a remote can only be added from repo-level config" in. Jeff already documented it well. I found one minor problem there in the documentation, which I will send separately. Thanks again for making me aware of this. [1] e459b073fb (remote rename: more carefully determine whether a remote is configured - 2017-01-19) [2] 20b20a22f8 (upload-pack: provide a hook for running pack-objects - 2016-05-18) -- Duy
[PATCH v2] help -a: improve and make --verbose default
When you type "git help" (or just "git") you are greeted with a list
with commonly used commands and their short description and are
suggested to use "git help -a" or "git help -g" for more details.
"git help -av" would be more friendly and inline with what is shown
with "git help" since it shows list of commands with description as
well, and commands are properly grouped.
"help -av" does not show everything "help -a" shows though. Add
external command section in "help -av" for this. While at there, add a
section for aliases as well (until now aliases have no UI, just "git
config").
Signed-off-by: Nguyễn Thái Ngọc Duy
---
v2 makes 'help -av' default and the user would need to type 'help -a
--no-verbose' to get the old printout back. 'help -av' also has
external commands and aliases.
Documentation/git-help.txt | 8 +++---
builtin/help.c | 2 +-
help.c | 50 +++---
t/t0012-help.sh| 4 +--
4 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 83d25d825a..206e3aef64 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
SYNOPSIS
[verse]
-'git help' [-a|--all [--verbose]] [-g|--guide]
+'git help' [-a|--all [--[no-]verbose]] [-g|--guide]
[-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]
DESCRIPTION
@@ -42,8 +42,10 @@ OPTIONS
--all::
Prints all the available commands on the standard output. This
option overrides any given command or guide name.
- When used with `--verbose` print description for all recognized
- commands.
+
+--verbose::
+ When used with `--all` print description for all recognized
+ commands. This is the default.
-c::
--config::
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..d83dac2839 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,7 @@ static const char *html_path;
static int show_all = 0;
static int show_guides = 0;
static int show_config;
-static int verbose;
+static int verbose = 1;
static unsigned int colopts;
static enum help_format help_format = HELP_FORMAT_NONE;
static int exclude_guides;
diff --git a/help.c b/help.c
index 96f6d221ed..4745b32299 100644
--- a/help.c
+++ b/help.c
@@ -98,7 +98,8 @@ static int cmd_name_cmp(const void *elem1, const void *elem2)
return strcmp(e1->name, e2->name);
}
-static void print_cmd_by_category(const struct category_description *catdesc)
+static void print_cmd_by_category(const struct category_description *catdesc,
+ int *longest_p)
{
struct cmdname_help *cmds;
int longest = 0;
@@ -124,6 +125,8 @@ static void print_cmd_by_category(const struct
category_description *catdesc)
print_command_list(cmds, mask, longest);
}
free(cmds);
+ if (longest_p)
+ *longest_p = longest;
}
void add_cmdname(struct cmdnames *cmds, const char *name, int len)
@@ -307,7 +310,7 @@ void list_commands(unsigned int colopts,
void list_common_cmds_help(void)
{
puts(_("These are common Git commands used in various situations:"));
- print_cmd_by_category(common_categories);
+ print_cmd_by_category(common_categories, NULL);
}
void list_all_main_cmds(struct string_list *list)
@@ -405,7 +408,7 @@ void list_common_guides_help(void)
{ CAT_guide, N_("The common Git guides are:") },
{ 0, NULL }
};
- print_cmd_by_category(catdesc);
+ print_cmd_by_category(catdesc, NULL);
putchar('\n');
}
@@ -494,9 +497,48 @@ void list_config_help(int for_human)
string_list_clear(, 0);
}
+static int get_alias(const char *var, const char *value, void *data)
+{
+ struct string_list *list = data;
+
+ if (skip_prefix(var, "alias.", ))
+ string_list_append(list, var)->util = xstrdup(value);
+
+ return 0;
+}
+
void list_all_cmds_help(void)
{
- print_cmd_by_category(main_categories);
+ struct string_list others = STRING_LIST_INIT_DUP;
+ struct string_list alias_list = STRING_LIST_INIT_DUP;
+ struct cmdname_help *aliases;
+ int i, longest;
+
+ printf_ln(_("See 'git help ' to read about a specific
subcommand"));
+ print_cmd_by_category(main_categories, );
+
+ list_all_other_cmds();
+ if (others.nr)
+ printf("\n%s\n", _("External commands"));
+ for (i = 0; i < others.nr; i++)
+ printf(" %s\n", others.items[i].string);
+ string_list_clear(, 0);
+
+ git_config(get_alias, _list);
+ string_list_sort(_list);
+ if (alias_list.nr) {
+ printf("\n%s\n", _("Command aliases"));
+ ALLOC_ARRAY(aliases, alias_list.nr + 1);
+ for (i = 0; i < alias_list.nr; i++) {
+
