[PATCH] git-rebase--interactive: fix copy-paste mistake
exec argument is a command, not a commit. Signed-off-by: Orgad Shaneh --- git-rebase--interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index cbf44f8648..85a72b933e 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -160,7 +160,7 @@ r, reword = use commit, but edit the commit message e, edit = use commit, but stop for amending s, squash = use commit, but meld into previous commit f, fixup = like \"squash\", but discard this commit's log message -x, exec = run command (the rest of the line) using shell +x, exec = run command (the rest of the line) using shell d, drop = remove commit l, label = label current HEAD with a name t, reset = reset HEAD to a label -- 2.17.0.windows.1
Re: [PATCH] packfile: Correct zlib buffer handling
On Sun, May 27, 2018 at 1:57 AM, Junio C Hamano wrote: > Duy Nguyen writes: > >> On Sat, May 26, 2018 at 12:56 AM, Jeremy Linton >> wrote: >>> @@ -1416,7 +1416,7 @@ static void *unpack_compressed_entry(struct >>> packed_git *p, >>> return NULL; >>> memset(&stream, 0, sizeof(stream)); >>> stream.next_out = buffer; >>> - stream.avail_out = size + 1; >>> + stream.avail_out = size; >> >> You may want to include in your commit message a reference to >> 39eea7bdd9 (Fix incorrect error check while reading deflated pack data >> - 2009-10-21) which adds this plus one with a fascinating story >> behind. > > A bit puzzled---are you saying that this recent patch breaks the old > fix and must be done in some other way? No. I actually wanted to answer that question when I tried to track down the commit that adds " + 1" but I did not spend enough time to understand the old problem. I guess your puzzle means you didn't think it would break anything, which is good. -- Duy
Re: [PATCH] t990X: use '.git/objects' as 'deep inside .git' path
On Sat, May 26, 2018 at 8:47 AM, Christian Couder wrote: > Tests t9902-completion.sh and t9903-bash-prompt.sh each have tests > that check what happens when we are "in the '.git' directory" and > when we are "deep inside the '.git' directory". > > To test the case when we are "deep inside the '.git' directory" the > test scripts used to perform a `cd .git/refs/heads`. > > As there are plans to implement other ref storage systems, let's > use '.git/objects' instead of '.git/refs/heads' as the "deep inside > the '.git' directory" path. Seems reasonable to me. +1. Michael
Re: 2.17.0 Regression When Adding Patches Without Whitespace In Initial Column
Todd, it looks like that may very well be the same issue. And it looks like it's planning on being fixed in the next release? Would that be 2.17.1 since it's a regression? > On 2018 May 26, at 16:07, Todd Zullinger wrote: > > Hi Jeff, > > Jeff Felchner wrote: >> Ever since 2.17.0, when saving a patch (using add --patch >> but probably other ways as well), if the whitespace is >> removed from the initial column, the patch doesn't apply. > > This sounds a bit like the issue discussed in this thread a > few weeks ago: > > https://public-inbox.org/git/e8aedc6b-5b3e-cfb2-be9d-971bfd9ad...@talktalk.net/ > > But I didn't download or watch the video, so I'm not positive. > >> Full walkthrough (including comparison with 2.16.3) in the >> video attached to this link: >> >> https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1 > > -- > Todd > ~~ > Everybody knows how to raise children, except the people who have > them. >-- P.J. O'Rourke >
Re: [RFC] git gc "--prune=now" semantics considered harmful
On Sat, May 26, 2018 at 4:31 PM Junio C Hamano wrote: > *That* is something I don't do. After all, I am fully aware that I > have started end-of-day ritual by that time, so I won't even look at > a new patch (or a pull request for that matter). Sounds like you're more organized about the end-of-day ritual than I am. For me the gc is not quite so structured. > I however have to wonder if there are opposite "oops" end-user > operation we also need to worry about, i.e. we are doing a large-ish > fetch, and get bored and run a gc fron another terminal. Perhaps > *that* is a bit too stupid to worry about? Auto-gc deliberately > does not use 'now' because it wants to leave a grace period to avoid > exactly that kind of race. For me, a "pull" never takes that long. Sure, any manual merging and the writing of the commit message might take a while, but it's "foreground" activity for me, I'd not start a gc in the middle of it. So at least to me, doing "git fsck --full" and "git gc --prune=now" are somewhat special because they take a while and tend to be background things that I "start and forget" about (the same way I sometimes start and forget a kernel build). Which is why that current "git gc --prune=now" behavior seems a bit dangerous. Linus
Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
Elijah Newren writes: >> I'd expect that a reader of the commit who cares enough to bother to >> wonder by looking at the patch and seeing that 2 became 3 would know >> why already. And a reader of the resulting file would not know that >> the 3 used to be 2, and won't be helped by "we used to count to 2, >> now we have 'out' also counted" that much, especially in the commit >> log message. What would help the latter would be to name which >> three paths we expect to see in the comment (or test against the >> exact list of paths, instead of using test_line_count). >> >>> An alternative to consider would be to add a .gitignore file in the >>> initial commit to ignore 'out', then the number of untracked files >>> don't have to be adjusted. >> >> I think that is a preferred solution that we've used in ls-files and >> status tests successfully. > > ...except that if we add a .gitignore to each initial commit (we use > test_create_repo for nearly every test to keep them separable meaning > we'd have to do this many times), then four lines above we have to > adjust the number of expected tracked files. And, for it to work, > we'd have to add an --exclude-standard flag to ls-files -o. Yeah, unless the original planned to use the .gitignore mechanism, converting it to use it now will become noisy.
Re: [PATCH] packfile: Correct zlib buffer handling
Duy Nguyen writes: > On Sat, May 26, 2018 at 12:56 AM, Jeremy Linton > wrote: >> @@ -1416,7 +1416,7 @@ static void *unpack_compressed_entry(struct packed_git >> *p, >> return NULL; >> memset(&stream, 0, sizeof(stream)); >> stream.next_out = buffer; >> - stream.avail_out = size + 1; >> + stream.avail_out = size; > > You may want to include in your commit message a reference to > 39eea7bdd9 (Fix incorrect error check while reading deflated pack data > - 2009-10-21) which adds this plus one with a fascinating story > behind. A bit puzzled---are you saying that this recent patch breaks the old fix and must be done in some other way?
Re: [RFC] git gc "--prune=now" semantics considered harmful
Linus Torvalds writes: > Soes my use pattern of "git gc --prune=now" make sense? Maybe not. But > it's what I've gotten used to, and it's at least not entirely insane. FWIW, my end-of-day ritual is to do repack -a -d -f with a large window and a small depth, followed by prune, which boils down to about the same. So I'd hope that is not entirely insane. I however do not think I bother with an explicit --expire=now when running the prune, though. In any case, that makes two of us, and the suggested patch protects only one of the two ;-) > But at least once now, I've done that "git gc" at the end of the day, and > a new pull request comes in, so I do the "git pull" without even thinking > about the fact that "git gc" is still running. *That* is something I don't do. After all, I am fully aware that I have started end-of-day ritual by that time, so I won't even look at a new patch (or a pull request for that matter). > So I actually would much prefer that foir git gc, "--prune=now" means > > (a) "now" > > (b) now at the _start_ of the "git gc" operation, not the time at > the _end_ of the operation when we've already spent a minute or > two doing repacking and are now doing the final pruning. > > anyway, with that explanation in mind, I'm appending a patch that is > pretty small and does that. It's a bit hacky, but I think it still makes > sense. > > Comments? Closing the possiblity of racing a running "gc" and new object creation like the above generally makes sense, I would think, whether the creation is due to 'pull/fetch', 'add', or even 'push'. I however have to wonder if there are opposite "oops" end-user operation we also need to worry about, i.e. we are doing a large-ish fetch, and get bored and run a gc fron another terminal. Perhaps *that* is a bit too stupid to worry about? Auto-gc deliberately does not use 'now' because it wants to leave a grace period to avoid exactly that kind of race. > diff --git a/builtin/gc.c b/builtin/gc.c > index c4777b244..98368c8b5 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -535,8 +535,12 @@ int cmd_gc(int argc, const char **argv, const char > *prefix) > if (argc > 0) > usage_with_options(builtin_gc_usage, builtin_gc_options); > > - if (prune_expire && parse_expiry_date(prune_expire, &dummy)) > - die(_("failed to parse prune expiry value %s"), prune_expire); > + if (prune_expire) { > + if (!strcmp(prune_expire, "now")) > + prune_expire = show_date(time(NULL), 0, > DATE_MODE(ISO8601)); > + if (parse_expiry_date(prune_expire, &dummy)) > + die(_("failed to parse prune expiry value %s"), > prune_expire); > + } > > if (aggressive) { > argv_array_push(&repack, "-f");
[RFC] git gc "--prune=now" semantics considered harmful
So this is a RFC patch, I'm not sure how much people really care, but I find the current behavior of "git gc --prune=now" to be unnecessarily dangerous. There's two issues with it: (a) parse_expiry_date() considers "now" to be special, and it actually doesn't mean "now" at all, it means "everything". (b) the date parsing isn't actually done "now", it's done *after* gc has already run, and we run "git prune --expire". So even if (a) wasn't true, "--prune=now" wouldn't actually mean "now" when the user expects it to happen, but "after doing repacking". I actually think that the "parse_expiry_date()" behavior makes sense within the context of "git prune --expire", so I'm not really complaining about (a) per se. I just think that what makes sense within the context of "git prune" does *not* necessarily make sense within the context of "git gc". Why do I care? I end up doing lots of random things in my local repository, and I also end up wanting to keep my repository fairly clean, so I tend to do "git gc --prune=now" to just make sure everything is packed and I've gotten rid of all the temporary stuff that so often happens when doing lots of git merges (which is what I do). You won't see those temporary objects for the usual trivial merges, but whenever you have a real recursive merge with automated conflict resolution, there will be things like those temporary merge-only objects for the 3-way base merge state. Soes my use pattern of "git gc --prune=now" make sense? Maybe not. But it's what I've gotten used to, and it's at least not entirely insane. But at least once now, I've done that "git gc" at the end of the day, and a new pull request comes in, so I do the "git pull" without even thinking about the fact that "git gc" is still running. And then the "--prune=now" behavior is actually really pretty dangerous. Because it will prune *all* unreachable objects, even if they are only *currently* unreachable because they are in the process of being unpacked by the concurrent "git fetch" (and I didn't check - I might just have been unlocky, bit I think "git prune" ignores FETCH_HEAD). So I actually would much prefer that foir git gc, "--prune=now" means (a) "now" (b) now at the _start_ of the "git gc" operation, not the time at the _end_ of the operation when we've already spent a minute or two doing repacking and are now doing the final pruning. anyway, with that explanation in mind, I'm appending a patch that is pretty small and does that. It's a bit hacky, but I think it still makes sense. Comments? Note that this really isn't likely very noticeable on most projects. When I do "git gc" on a fairly well-packed repo of git itself, it takes under 4s for me. So the window for that whole "do git pull at the same time" is simply not much of an issue. For the kernel, "git gc" takes a minute and a half on the same machine (again, this is already a packed repo, it can be worse). So there's a much bigger window there to do something stupid, Linus --- builtin/gc.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index c4777b244..98368c8b5 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -535,8 +535,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (argc > 0) usage_with_options(builtin_gc_usage, builtin_gc_options); - if (prune_expire && parse_expiry_date(prune_expire, &dummy)) - die(_("failed to parse prune expiry value %s"), prune_expire); + if (prune_expire) { + if (!strcmp(prune_expire, "now")) + prune_expire = show_date(time(NULL), 0, DATE_MODE(ISO8601)); + if (parse_expiry_date(prune_expire, &dummy)) + die(_("failed to parse prune expiry value %s"), prune_expire); + } if (aggressive) { argv_array_push(&repack, "-f");
From: Mr James Tan (Urgent & Confidential)
-- -- (From: Mr.James Tan (Urgent & Confidential) Good Day, Please Read. My name is Mr.James Tan , I am the Bill and Exchange manager here in Bank of Africa (BOA) Lome-Togo.West-Africa. I have a business proposal in the tune of $9.7m, (Nine Million Seven Hundred Thousand Dollars Only) after the successful transfer, we shall share in ratio of 40% for you and 60% for me. Should you be interested, please contact me through my private email ( jamestanta...@gmail.com) so we can commence all arrangements and i will give you more information on how we would handle this project. Please treat this business with utmost confidentiality and send me the Following: 1. Your Full Name: 2. Your Phone Number:.. 3. Your Age: 4. Your Sex:... 5. Your Occupations: 6. Your Country and City: Kind Regards, Mr.James Tan . Bill & Exchange manager (BOA) Bank of Africa. Lome-Togo. Email: jamestanta...@gmail.com
Re: 2.17.0 Regression When Adding Patches Without Whitespace In Initial Column
Hi Jeff, Jeff Felchner wrote: > Ever since 2.17.0, when saving a patch (using add --patch > but probably other ways as well), if the whitespace is > removed from the initial column, the patch doesn't apply. This sounds a bit like the issue discussed in this thread a few weeks ago: https://public-inbox.org/git/e8aedc6b-5b3e-cfb2-be9d-971bfd9ad...@talktalk.net/ But I didn't download or watch the video, so I'm not positive. > Full walkthrough (including comparison with 2.16.3) in the > video attached to this link: > > https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1 -- Todd ~~ Everybody knows how to raise children, except the people who have them. -- P.J. O'Rourke
2.17.0 Regression When Adding Patches Without Whitespace In Initial Column
Ever since 2.17.0, when saving a patch (using add --patch but probably other ways as well), if the whitespace is removed from the initial column, the patch doesn't apply. Full walkthrough (including comparison with 2.16.3) in the video attached to this link: https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1
Re: [PATCH v3 02/20] commit-graph: fix GRAPH_MIN_SIZE
On Sat, May 26, 2018 at 08:46:09PM +0200, Jakub Narebski wrote: > One issue: in the future when Git moves to NewHash, it could encounter > then both commit-graph files using SHA-1 and using NewHash. What about > GRPH_OID_LEN then: for one of those it would be incorrect. Unless it is > about minimal length of checksum, that is we assume that NewHash would > be longer than SHA-1, but ten why name it GRAPH_OID_LEN? My proposal is that whatever we're using in the .git directory is consistent. If we're using SHA-1 for objects, then everything is SHA-1. If we're using NewHash for objects, then all data is stored in NewHash (except translation tables and such). Any conversions between SHA-1 and NewHash require a lookup through the standard techniques. I agree that here it would be more helpful if it were a reference to the_hash_algo, and I've applied a patch to my object-id-part14 series to make that conversion. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH] branch: issue "-l" deprecation warning after pager starts
On Friday 25 May 2018 08:10 AM, Jeff King wrote: > Subject: [PATCH] branch: customize "-l" warning in list mode > > People mistakenly use "git branch -l", thinking that it > triggers list mode. It doesn't, but the lack of non-option > arguments in that command does (and the "-l" becomes a > silent noop). > > Since afc968e579 (branch: deprecate "-l" option, 2018-03-26) > > we've warned that "-l" is going away. But the warning text > is primarily aimed at people who _meant_ to use "-l", as in > "git branch -l foo". People who mistakenly said "git branch > -l" may be left puzzled. > So, this patch is to improve the user experience of people who use "git branch -l" for listing and not for the people who forget to give a new branch name argument for "-l". In that case, this makes sense. BTW, I hope people don't start wondering why "git branch -d" doesn't trigger list mode ;-) > + warning("the '-l' option is an alias for > '--create-reflog' and"); > + warning("has no effect in list mode. This option will > soon be"); > + warning("removed and you should omit it (or use > '--list' instead)."); I suppose s/alias/deprecated alias/ makes the point that '-l' will be removed more stronger. -- Sivaraam signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 02/20] commit-graph: fix GRAPH_MIN_SIZE
Derrick Stolee writes: > The GRAPH_MIN_SIZE macro should be the smallest size of a parsable > commit-graph file. However, the minimum number of chunks was wrong. > It is possible to write a commit-graph file with zero commits, and > that violates this macro's value. > > Rewrite the macro, and use extra macros to better explain the magic > constants. > > Signed-off-by: Derrick Stolee > --- > commit-graph.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/commit-graph.c b/commit-graph.c > index a8c337dd77..82295f0975 100644 > --- a/commit-graph.c > +++ b/commit-graph.c > @@ -33,10 +33,11 @@ > > #define GRAPH_LAST_EDGE 0x8000 > > +#define GRAPH_HEADER_SIZE 8 Nice. > #define GRAPH_FANOUT_SIZE (4 * 256) > #define GRAPH_CHUNKLOOKUP_WIDTH 12 > -#define GRAPH_MIN_SIZE (5 * GRAPH_CHUNKLOOKUP_WIDTH + GRAPH_FANOUT_SIZE + \ > - GRAPH_OID_LEN + 8) > +#define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \ > + + GRAPH_FANOUT_SIZE + GRAPH_OID_LEN) So in this case we have file header (4-byte signature, 1-byte version number, 1-byte oid/hash version, 1-byte number of chunks, 1-byte reserved: 4+1+1+1+1 = 8 bytes), chunk lookup: 3 required chunks plus terminating label = 4 entries, constant-size fanout chunks, and checksum. Two remaining required chunks (OID Lookup and Commit Data) can have length of 0. One issue: in the future when Git moves to NewHash, it could encounter then both commit-graph files using SHA-1 and using NewHash. What about GRPH_OID_LEN then: for one of those it would be incorrect. Unless it is about minimal length of checksum, that is we assume that NewHash would be longer than SHA-1, but ten why name it GRAPH_OID_LEN? This may be going too much in the future; there is no need to borrow trouble now, where we have only SHA-1 supported as OID. Still... > > char *get_commit_graph_filename(const char *obj_dir) > { Best, -- Jakub Narębski
Re: [PATCH] branch: issue "-l" deprecation warning after pager starts
On Friday 25 May 2018 01:01 AM, Jeff King wrote: > On Thu, May 24, 2018 at 03:22:14PM -0400, Jeff King wrote: > > Hmm, actually, I suppose the true value of the warning is to help people > doing "git branch -l foo", and it would still work there. The "more > extreme" from your suggested patch would only affect "branch -l". > > Still, I think I prefer the gentler version that we get by keeping it as > a warning even in the latter case. > I never wanted to suppress the warning message in the latter case. I just wanted to avoid listing the branches. Actually the patch I sent in one the previous threads[1] that avoids listing the branches has the following behaviour, $ git branch -l warning: the '-l' alias for '--create-reflog' is deprecated; warning: it will be removed in a future version of Git usage: git branch [] [-r | -a] [--merged | --no-merged] or: git branch [] [-l] [-f] [] or: git branch [] [-r] (-d | -D) ... or: git branch [] (-m | -M) [] or: git branch [] (-c | -C) [] or: git branch [] [-r | -a] [--points-at] or: git branch [] [-r | -a] [--format] So, the warning message isn't lost. It just prevents the listing of branches. Wait, maybe I'm misunderstanding what you mean by "warning". You're probably meaning something related to the way Git exits in both cases. With your patch "git branch -l" prints a warning, lists the branches and has an exit status of 0. With my patch it prints the warning, the usage specifications with exit status 128. In that case, I still don't think it's bad to turn "git branch -l" into an error now as it's been incorrect for a long time now and it's not wrong if we correct it now. Anyways, if you think it mustn't turn into an error now and only in the next stage, a suggestion follows in another thread. [1]: https://public-inbox.org/git/1527174618.10589.4.ca...@gmail.com/ -- Sivaraam QUOTE: “The most valuable person on any team is the person who makes everyone else on the team more valuable, not the person who knows the most.” - John Sonmez Sivaraam? You possibly might have noticed that my signature recently changed from 'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the new signature to be better for several reasons one of which is that the former signature has a lot of ambiguities in the place I live as it is a common name (NOTE: it's not a common spelling, just a common name). So, I switched signatures before it's too late. That said, I won't mind you calling me 'Kaartic' if you like it [of course ;-)]. You can always call me using either of the names. KIND NOTE TO THE NATIVE ENGLISH SPEAKER: As I'm not a native English speaker myself, there might be mistaeks in my usage of English. I apologise for any mistakes that I make. It would be "helpful" if you take the time to point out the mistakes. It would be "super helpful" if you could provide suggestions about how to correct those mistakes. Thanks in advance! signature.asc Description: OpenPGP digital signature
Re: RFC: New reference iteration paradigm
Jeff King writes: > On Thu, Mar 31, 2016 at 11:01:44AM -0700, Junio C Hamano wrote: >> Michael Haggerty writes: >> >>> the backend now has to implement >>> struct ref_iterator *ref_iterator_begin_fn(const char *submodule, const char *prefix, unsigned int flags); The problem with callbacks, including their lack of composability, is often solved in high-level languages by using Promises / Futures (or Channels / Supplies). But I think in this case implementing a straightforward Iterator pattern would be a better and simpler solution, especially in C. >>> The ref_iterator itself has to implement two main methods: >>> int iterator_advance_fn(struct ref_iterator *ref_iterator); void iterator_free_fn(struct ref_iterator *ref_iterator); >>> >>> A loop over references now looks something like >>> struct ref_iterator *iter = each_ref_in_iterator("refs/tags/"); while (ref_iterator_advance(iter)) { /* code using iter->refname, iter->oid, iter->flags */ } >> >> We'd want to take advantage of the tree-like organization of the >> refs (i.e. refs/tags/a and refs/tags/b sit next to each other and >> they are closer to each other than they are to refs/heads/a) so that >> a request "I want to iterate only over tags, even though I may have >> millions of other kinds of refs" can be done with cost that is >> proportional to how many tags you have. >> >> The current implementation of for_each_tag_ref() that goes down to >> do_for_each_entry() in files-backend.c has that propertly, and the >> new iteration mechanism with the above design seems to keep it, >> which is very nice. > > Actually, that is a slight fiction. :) > > We traverse only the loose ref directories we need, but we populate the > entire packed-refs tree in one go. That's fine if you have a reasonable > number of refs and care about the cold-cache case (where you care a lot > about hitting directories you don't need to, but reading the entire > packed-refs file isn't a big deal). But it's really bad if you have an > 800MB packed-refs file, as looking up one tiny subset of the entries > wastes a lot of RAM and CPU pulling that into our internal > representation[1]. > > At one point I wrote a patch to binary search the packed-refs file, find > the first "refs/tags/" entry, and then walk linearly through there. What > stopped me is that the current refs.c code (I guess file-backend.c these > days) was not happy with me partially filling in the ref_dir structs in > this "inside out" way. [...] Isn't this what reftable - an alternative way of storing refs in Git, tested by being used by JGit - would solve? See Christian Couder post "Implementing reftable in Git" https://public-inbox.org/git/cap8ufd0ppzsjbnxca7ez91vbuatchkq+juwvtd1ihcxzpbj...@mail.gmail.com/t/#u 'Efficient lookup of an entire namespace, such as refs/tags/' is allegedly one of the objectives of this format. Regards, -- Jakub Narębski
[GSoC] GSoC with git, week 4
Hi, I published my blog post about this week. You can read it here: https://blog.pa1ch.fr/posts/2018/05/26/en/gsoc2018-week-4.html All comments are welcome! Cheers, Alban
Proposal
Hello Greetings to you please i have a business proposal for you contact me for more detailes asap thanks. Best Regards, Miss.Zeliha ömer faruk Esentepe Mahallesi Büyükdere Caddesi Kristal Kule Binasi No:215 Sisli - Istanbul, Turkey
[PATCH v2 05/11] fsck: produce camelCase config key names
Signed-off-by: Nguyễn Thái Ngọc Duy --- fsck.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/fsck.c b/fsck.c index b65ff2d856..ff1547d3c7 100644 --- a/fsck.c +++ b/fsck.c @@ -74,14 +74,15 @@ enum fsck_msg_id { #undef MSG_ID #define STR(x) #x -#define MSG_ID(id, msg_type) { STR(id), NULL, FSCK_##msg_type }, +#define MSG_ID(id, msg_type) { STR(id), NULL, NULL, FSCK_##msg_type }, static struct { const char *id_string; const char *downcased; + const char *camelcased; int msg_type; } msg_id_info[FSCK_MSG_MAX + 1] = { FOREACH_MSG_ID(MSG_ID) - { NULL, NULL, -1 } + { NULL, NULL, NULL, -1 } }; #undef MSG_ID @@ -105,6 +106,20 @@ static void prepare_msg_ids(void) else *(q)++ = tolower(*(p)++); *q = '\0'; + + p = msg_id_info[i].id_string; + q = xmalloc(len); + msg_id_info[i].camelcased = q; + while (*p) { + if (*p == '_') { + p++; + if (*p) + *q++ = *p++; + } else { + *q++ = tolower(*p++); + } + } + *q = '\0'; } } @@ -127,9 +142,8 @@ void list_config_fsck_msg_ids(struct string_list *list, const char *prefix) prepare_msg_ids(); - /* TODO: we can do better by producing camelCase names */ for (i = 0; i < FSCK_MSG_MAX; i++) - list_config_item(list, prefix, msg_id_info[i].downcased); + list_config_item(list, prefix, msg_id_info[i].camelcased); } static int fsck_msg_type(enum fsck_msg_id msg_id, -- 2.17.0.705.g3525833791
[PATCH v2 11/11] log-tree: allow to customize 'grafted' color
Commit 76f5df305b (log: decorate grafted commits with "grafted" - 2011-08-18) lets us decorate grafted commits but I forgot about the color.decorate.* config. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/config.txt | 3 ++- log-tree.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ab641bf5a9..1cc18a828c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1162,7 +1162,8 @@ color.diff.:: color.decorate.:: Use customized color for 'git log --decorate' output. `` is one of `branch`, `remoteBranch`, `tag`, `stash` or `HEAD` for local - branches, remote-tracking branches, tags, stash and HEAD, respectively. + branches, remote-tracking branches, tags, stash and HEAD, respectively + and `grafted` for grafted commits. color.grep:: When set to `always`, always highlight matches. When `false` (or diff --git a/log-tree.c b/log-tree.c index 284ce0e3c5..d3a43e29cd 100644 --- a/log-tree.c +++ b/log-tree.c @@ -34,6 +34,7 @@ static const char *color_decorate_slots[] = { [DECORATION_REF_TAG]= "tag", [DECORATION_REF_STASH] = "stash", [DECORATION_REF_HEAD] = "HEAD", + [DECORATION_GRAFTED]= "grafted", }; static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix) -- 2.17.0.705.g3525833791
[PATCH v2 10/11] completion: support case-insensitive config vars
Config variables are case-insensitive but this case/esac construct is case-sensitive by default. For bash v4, it'll be easy. For platforms that are stuck with older versions, we need an external command, but that is not that critical. And where this additional overhead matters the most is Windows, but luckily Git for Windows ships with Bash v4. Helped-by: SZEDER Gábor Signed-off-by: Nguyễn Thái Ngọc Duy --- contrib/completion/git-completion.bash | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index efc930c9d1..2cbd14b346 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2151,7 +2151,15 @@ __git_compute_config_vars () _git_config () { - case "$prev" in + local varname + + if [ "${BASH_VERSINFO[0]:-0}" -ge 4 ]; then + varname="${prev,,}" + else + varname="$(echo "$prev" |tr A-Z a-z)" + fi + + case "$varname" in branch.*.remote|branch.*.pushremote) __gitcomp_nl "$(__git_remotes)" return -- 2.17.0.705.g3525833791
[PATCH v2 03/11] fsck: factor out msg_id_info[] lazy initialization code
This array will be used by some other function than parse_msg_id() in the following commit. Factor out this prep code so it could be called from that one. Signed-off-by: Nguyễn Thái Ngọc Duy --- fsck.c | 40 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/fsck.c b/fsck.c index 1722f6cab0..e2a6f33891 100644 --- a/fsck.c +++ b/fsck.c @@ -84,26 +84,34 @@ static struct { }; #undef MSG_ID -static int parse_msg_id(const char *text) +static void prepare_msg_ids(void) { int i; - if (!msg_id_info[0].downcased) { - /* convert id_string to lower case, without underscores. */ - for (i = 0; i < FSCK_MSG_MAX; i++) { - const char *p = msg_id_info[i].id_string; - int len = strlen(p); - char *q = xmalloc(len); - - msg_id_info[i].downcased = q; - while (*p) - if (*p == '_') - p++; - else - *(q)++ = tolower(*(p)++); - *q = '\0'; - } + if (msg_id_info[0].downcased) + return; + + /* convert id_string to lower case, without underscores. */ + for (i = 0; i < FSCK_MSG_MAX; i++) { + const char *p = msg_id_info[i].id_string; + int len = strlen(p); + char *q = xmalloc(len); + + msg_id_info[i].downcased = q; + while (*p) + if (*p == '_') + p++; + else + *(q)++ = tolower(*(p)++); + *q = '\0'; } +} + +static int parse_msg_id(const char *text) +{ + int i; + + prepare_msg_ids(); for (i = 0; i < FSCK_MSG_MAX; i++) if (!strcmp(text, msg_id_info[i].downcased)) -- 2.17.0.705.g3525833791
[PATCH v2 02/11] grep: keep all colors in an array
This is more inline with how we handle color slots in other code. It also allows us to get the list of configurable color slots later. Signed-off-by: Nguyễn Thái Ngọc Duy --- grep.c | 106 ++--- grep.h | 21 +++- 2 files changed, 62 insertions(+), 65 deletions(-) diff --git a/grep.c b/grep.c index 45ec7e636c..d94e2c4aeb 100644 --- a/grep.c +++ b/grep.c @@ -13,6 +13,17 @@ static int grep_source_is_binary(struct grep_source *gs); static struct grep_opt grep_defaults; +static const char *color_grep_slots[] = { + [GREP_COLOR_CONTEXT]= "context", + [GREP_COLOR_FILENAME] = "filename", + [GREP_COLOR_FUNCTION] = "function", + [GREP_COLOR_LINENO] = "lineNumber", + [GREP_COLOR_MATCH_CONTEXT] = "matchContext", + [GREP_COLOR_MATCH_SELECTED] = "matchSelected", + [GREP_COLOR_SELECTED] = "selected", + [GREP_COLOR_SEP]= "separator", +}; + static void std_output(struct grep_opt *opt, const void *buf, size_t size) { fwrite(buf, size, 1, stdout); @@ -42,14 +53,14 @@ void init_grep_defaults(void) opt->pathname = 1; opt->max_depth = -1; opt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED; - color_set(opt->color_context, ""); - color_set(opt->color_filename, ""); - color_set(opt->color_function, ""); - color_set(opt->color_lineno, ""); - color_set(opt->color_match_context, GIT_COLOR_BOLD_RED); - color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED); - color_set(opt->color_selected, ""); - color_set(opt->color_sep, GIT_COLOR_CYAN); + color_set(opt->colors[GREP_COLOR_CONTEXT], ""); + color_set(opt->colors[GREP_COLOR_FILENAME], ""); + color_set(opt->colors[GREP_COLOR_FUNCTION], ""); + color_set(opt->colors[GREP_COLOR_LINENO], ""); + color_set(opt->colors[GREP_COLOR_MATCH_CONTEXT], GIT_COLOR_BOLD_RED); + color_set(opt->colors[GREP_COLOR_MATCH_SELECTED], GIT_COLOR_BOLD_RED); + color_set(opt->colors[GREP_COLOR_SELECTED], ""); + color_set(opt->colors[GREP_COLOR_SEP], GIT_COLOR_CYAN); opt->color = -1; opt->output = std_output; } @@ -76,7 +87,7 @@ static int parse_pattern_type_arg(const char *opt, const char *arg) int grep_config(const char *var, const char *value, void *cb) { struct grep_opt *opt = &grep_defaults; - char *color = NULL; + const char *slot; if (userdiff_config(var, value) < 0) return -1; @@ -103,32 +114,18 @@ int grep_config(const char *var, const char *value, void *cb) if (!strcmp(var, "color.grep")) opt->color = git_config_colorbool(var, value); - else if (!strcmp(var, "color.grep.context")) - color = opt->color_context; - else if (!strcmp(var, "color.grep.filename")) - color = opt->color_filename; - else if (!strcmp(var, "color.grep.function")) - color = opt->color_function; - else if (!strcmp(var, "color.grep.linenumber")) - color = opt->color_lineno; - else if (!strcmp(var, "color.grep.matchcontext")) - color = opt->color_match_context; - else if (!strcmp(var, "color.grep.matchselected")) - color = opt->color_match_selected; - else if (!strcmp(var, "color.grep.selected")) - color = opt->color_selected; - else if (!strcmp(var, "color.grep.separator")) - color = opt->color_sep; - else if (!strcmp(var, "color.grep.match")) { - int rc = 0; - if (!value) - return config_error_nonbool(var); - rc |= color_parse(value, opt->color_match_context); - rc |= color_parse(value, opt->color_match_selected); - return rc; - } - - if (color) { + if (!strcmp(var, "color.grep.match")) { + if (grep_config("color.grep.matchcontext", value, cb) < 0) + return -1; + if (grep_config("color.grep.matchselected", value, cb) < 0) + return -1; + } else if (skip_prefix(var, "color.grep.", &slot)) { + int i = LOOKUP_CONFIG(color_grep_slots, slot); + char *color; + + if (i < 0) + return -1; + color = opt->colors[i]; if (!value) return config_error_nonbool(var); return color_parse(value, color); @@ -144,6 +141,7 @@ int grep_config(const char *var, const char *value, void *cb) void grep_init(struct grep_opt *opt, const char *prefix) { struct grep_opt *def = &grep_defaults; + int i; memset(opt, 0, sizeof(*opt)); opt->prefix = prefix; @@ -160,14 +158,8 @@ void grep_init(struct grep_opt *opt, const char *prefix) opt->relative = def->relative;
[PATCH v2 06/11] advice: keep config name in camelCase in advice_config[]
For parsing, we don't really need this because the main config parser will lowercase everything so we can do exact matching. But this array now is also used for printing in `git help --config`. Keep camelCase so we have a nice printout. Signed-off-by: Nguyễn Thái Ngọc Duy --- advice.c | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/advice.c b/advice.c index cf6c673ed7..2aca11f45e 100644 --- a/advice.c +++ b/advice.c @@ -54,28 +54,28 @@ static struct { const char *name; int *preference; } advice_config[] = { - { "pushupdaterejected", &advice_push_update_rejected }, - { "pushnonffcurrent", &advice_push_non_ff_current }, - { "pushnonffmatching", &advice_push_non_ff_matching }, - { "pushalreadyexists", &advice_push_already_exists }, - { "pushfetchfirst", &advice_push_fetch_first }, - { "pushneedsforce", &advice_push_needs_force }, - { "statushints", &advice_status_hints }, - { "statusuoption", &advice_status_u_option }, - { "commitbeforemerge", &advice_commit_before_merge }, - { "resolveconflict", &advice_resolve_conflict }, - { "implicitidentity", &advice_implicit_identity }, - { "detachedhead", &advice_detached_head }, - { "setupstreamfailure", &advice_set_upstream_failure }, - { "objectnamewarning", &advice_object_name_warning }, - { "rmhints", &advice_rm_hints }, - { "addembeddedrepo", &advice_add_embedded_repo }, - { "ignoredhook", &advice_ignored_hook }, - { "waitingforeditor", &advice_waiting_for_editor }, - { "graftfiledeprecated", &advice_graft_file_deprecated }, + { "pushUpdateRejected", &advice_push_update_rejected }, + { "pushNonFFCurrent", &advice_push_non_ff_current }, + { "pushNonFFMatching", &advice_push_non_ff_matching }, + { "pushAlreadyExists", &advice_push_already_exists }, + { "pushFetchFirst", &advice_push_fetch_first }, + { "pushNeedsForce", &advice_push_needs_force }, + { "statusHints", &advice_status_hints }, + { "statusUoption", &advice_status_u_option }, + { "commitBeforeMerge", &advice_commit_before_merge }, + { "resolveConflict", &advice_resolve_conflict }, + { "implicitIdentity", &advice_implicit_identity }, + { "detachedHead", &advice_detached_head }, + { "setupStreamFailure", &advice_set_upstream_failure }, + { "objectNameWarning", &advice_object_name_warning }, + { "rmHints", &advice_rm_hints }, + { "addEmbeddedRepo", &advice_add_embedded_repo }, + { "ignoredHook", &advice_ignored_hook }, + { "waitingForEditor", &advice_waiting_for_editor }, + { "graftFileDeprecated", &advice_graft_file_deprecated }, /* make this an alias for backward compatibility */ - { "pushnonfastforward", &advice_push_update_rejected } + { "pushNonFastForward", &advice_push_update_rejected } }; void advise(const char *advice, ...) @@ -123,7 +123,7 @@ int git_default_advice_config(const char *var, const char *value) return 0; for (i = 0; i < ARRAY_SIZE(advice_config); i++) { - if (strcmp(k, advice_config[i].name)) + if (strcasecmp(k, advice_config[i].name)) continue; *advice_config[i].preference = git_config_bool(var, value); return 0; -- 2.17.0.705.g3525833791
[PATCH v2 08/11] completion: drop the hard coded list of config vars
The new help option --config-for-completion is a machine friendlier version of --config where all the placeholders and wildcards are dropped, leaving only the good, completable prefixes for git-completion.bash to consume. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/help.c | 9 +- contrib/completion/git-completion.bash | 335 + help.c | 34 ++- help.h | 2 +- 4 files changed, 49 insertions(+), 331 deletions(-) diff --git a/builtin/help.c b/builtin/help.c index ccb206e1d4..8d4f6dd301 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -47,6 +47,7 @@ static struct option builtin_help_options[] = { OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")), OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")), OPT_BOOL('c', "config", &show_config, N_("print all configuration variable names")), + OPT_SET_INT_F(0, "config-for-completion", &show_config, "", 2, PARSE_OPT_HIDDEN), OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN), OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"), HELP_FORMAT_WEB), @@ -447,8 +448,14 @@ int cmd_help(int argc, const char **argv, const char *prefix) } if (show_config) { + int for_human = show_config == 1; + + if (!for_human) { + list_config_help(for_human); + return 0; + } setup_pager(); - list_config_help(); + list_config_help(for_human); printf("\n%s\n", _("'git help config' for more information")); return 0; } diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 12814e9bbf..1b0c30ed9a 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2142,6 +2142,13 @@ __git_config_get_set_variables () __git config $config_file --name-only --list } +__git_config_vars= +__git_compute_config_vars () +{ + test -n "$__git_config_vars" || + __git_config_vars="$(git help --config-for-completion | sort | uniq)" +} + _git_config () { case "$prev" in @@ -2300,332 +2307,8 @@ _git_config () return ;; esac - __gitcomp " - add.ignoreErrors - advice.amWorkDir - advice.commitBeforeMerge - advice.detachedHead - advice.implicitIdentity - advice.pushAlreadyExists - advice.pushFetchFirst - advice.pushNeedsForce - advice.pushNonFFCurrent - advice.pushNonFFMatching - advice.pushUpdateRejected - advice.resolveConflict - advice.rmHints - advice.statusHints - advice.statusUoption - advice.ignoredHook - alias. - am.keepcr - am.threeWay - apply.ignorewhitespace - apply.whitespace - branch.autosetupmerge - branch.autosetuprebase - browser. - clean.requireForce - color.branch - color.branch.current - color.branch.local - color.branch.plain - color.branch.remote - color.decorate.HEAD - color.decorate.branch - color.decorate.remoteBranch - color.decorate.stash - color.decorate.tag - color.diff - color.diff.commit - color.diff.frag - color.diff.func - color.diff.meta - color.diff.new - color.diff.old - color.diff.plain - color.diff.whitespace - color.grep - color.grep.context - color.grep.filename - color.grep.function - color.grep.linenumber - color.grep.match - color.grep.selected - color.grep.separator - color.interactive - color.interactive.error - color.interactive.header - color.interactive.help - color.interactive.prompt - color.pager - color.showbranch - color.status - color.status.added - color.status.changed - color.status.header - color.status.localBranch - color.status.nobranch - color.status.remoteBranch - color.status.unmerged - color.status.untracked - color.status.updated - color.ui - commit.cleanup -
[PATCH v2 04/11] help: add --config to list all available config
Sometimes it helps to list all available config vars so the user can search for something they want. The config man page can also be used but it's harder to search if you want to focus on the variable name, for example. This is not the best way to collect the available config since it's not precise. Ideally we should have a centralized list of config in C code (pretty much like 'struct option'), but that's a lot more work. This will do for now. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git-help.txt | 5 advice.c | 9 ++ builtin/branch.c | 3 ++ builtin/clean.c| 3 ++ builtin/commit.c | 3 ++ builtin/help.c | 9 ++ diff.c | 3 ++ fsck.c | 12 generate-cmdlist.sh| 19 + grep.c | 3 ++ help.c | 56 ++ help.h | 45 +- log-tree.c | 3 ++ 13 files changed, 172 insertions(+), 1 deletion(-) diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index a40fc38d8b..83d25d825a 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -45,6 +45,11 @@ OPTIONS When used with `--verbose` print description for all recognized commands. +-c:: +--config:: + List all available configuration variables. This is a short + summary of the list in linkgit:git-config[1]. + -g:: --guides:: Prints a list of useful guides on the standard output. This diff --git a/advice.c b/advice.c index 370a56d054..cf6c673ed7 100644 --- a/advice.c +++ b/advice.c @@ -1,6 +1,7 @@ #include "cache.h" #include "config.h" #include "color.h" +#include "help.h" int advice_push_update_rejected = 1; int advice_push_non_ff_current = 1; @@ -131,6 +132,14 @@ int git_default_advice_config(const char *var, const char *value) return 0; } +void list_config_advices(struct string_list *list, const char *prefix) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(advice_config); i++) + list_config_item(list, prefix, advice_config[i].name); +} + int error_resolve_conflict(const char *me) { if (!strcmp(me, "cherry-pick")) diff --git a/builtin/branch.c b/builtin/branch.c index 09232576b6..ddc48ca931 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -22,6 +22,7 @@ #include "wt-status.h" #include "ref-filter.h" #include "worktree.h" +#include "help.h" static const char * const builtin_branch_usage[] = { N_("git branch [] [-r | -a] [--merged | --no-merged]"), @@ -67,6 +68,8 @@ static const char *color_branch_slots[] = { static struct string_list output = STRING_LIST_INIT_DUP; static unsigned int colopts; +define_list_config_array(color_branch_slots); + static int git_branch_config(const char *var, const char *value, void *cb) { const char *slot_name; diff --git a/builtin/clean.c b/builtin/clean.c index 0ccd95e693..ab402c204c 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -16,6 +16,7 @@ #include "column.h" #include "color.h" #include "pathspec.h" +#include "help.h" static int force = -1; /* unset */ static int interactive; @@ -91,6 +92,8 @@ struct menu_stuff { void *stuff; }; +define_list_config_array(color_interactive_slots); + static int git_clean_config(const char *var, const char *value, void *cb) { const char *slot_name; diff --git a/builtin/commit.c b/builtin/commit.c index 2b43a6c48a..9bcbb0c25c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -32,6 +32,7 @@ #include "column.h" #include "sequencer.h" #include "mailmap.h" +#include "help.h" static const char * const builtin_commit_usage[] = { N_("git commit [] [--] ..."), @@ -1195,6 +1196,8 @@ static int dry_run_commit(int argc, const char **argv, const char *prefix, return commitable ? 0 : 1; } +define_list_config_array_extra(color_status_slots, {"added"}); + static int parse_status_slot(const char *slot) { if (!strcasecmp(slot, "added")) diff --git a/builtin/help.c b/builtin/help.c index 58e0a5507f..ccb206e1d4 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -37,6 +37,7 @@ static const char *html_path; static int show_all = 0; static int show_guides = 0; +static int show_config; static int verbose; static unsigned int colopts; static enum help_format help_format = HELP_FORMAT_NONE; @@ -45,6 +46,7 @@ static struct option builtin_help_options[] = { OPT_BOOL('a', "all", &show_all, N_("print all available commands")), OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")), OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")), + OPT_BOOL('c', "config", &show_config, N_("print all configuration variable names")), OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN), OPT_SET_
[PATCH v2 07/11] am: move advice.amWorkDir parsing back to advice.c
The only benefit from this move (apart from cleaner code) is that advice.amWorkDir should now show up in `git help --config`. There should be no regression since advice config is always read by the git_default_config(). While at there, use advise() like other code. We now get "hint: " prefix and the output is stderr instead of stdout (which is also the reason for the test update because stderr is checked in a following test and the extra advice can fail it). Signed-off-by: Nguyễn Thái Ngọc Duy --- advice.c | 2 ++ advice.h | 1 + builtin/am.c | 6 +- t/t4254-am-corrupt.sh | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/advice.c b/advice.c index 2aca11f45e..52aa85bdfd 100644 --- a/advice.c +++ b/advice.c @@ -17,6 +17,7 @@ int advice_implicit_identity = 1; int advice_detached_head = 1; int advice_set_upstream_failure = 1; int advice_object_name_warning = 1; +int advice_amworkdir = 1; int advice_rm_hints = 1; int advice_add_embedded_repo = 1; int advice_ignored_hook = 1; @@ -68,6 +69,7 @@ static struct { { "detachedHead", &advice_detached_head }, { "setupStreamFailure", &advice_set_upstream_failure }, { "objectNameWarning", &advice_object_name_warning }, + { "amWorkDir", &advice_amworkdir }, { "rmHints", &advice_rm_hints }, { "addEmbeddedRepo", &advice_add_embedded_repo }, { "ignoredHook", &advice_ignored_hook }, diff --git a/advice.h b/advice.h index 9f5064e82a..7e9377864f 100644 --- a/advice.h +++ b/advice.h @@ -17,6 +17,7 @@ extern int advice_implicit_identity; extern int advice_detached_head; extern int advice_set_upstream_failure; extern int advice_object_name_warning; +extern int advice_amworkdir; extern int advice_rm_hints; extern int advice_add_embedded_repo; extern int advice_ignored_hook; diff --git a/builtin/am.c b/builtin/am.c index aa989e7390..735d016525 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1827,15 +1827,11 @@ static void am_run(struct am_state *state, int resume) } if (apply_status) { - int advice_amworkdir = 1; - printf_ln(_("Patch failed at %s %.*s"), msgnum(state), linelen(state->msg), state->msg); - git_config_get_bool("advice.amworkdir", &advice_amworkdir); - if (advice_amworkdir) - printf_ln(_("Use 'git am --show-current-patch' to see the failed patch")); + advise(_("Use 'git am --show-current-patch' to see the failed patch")); die_user_resolve(state); } diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh index 168739c721..fd3bdbfe2c 100755 --- a/t/t4254-am-corrupt.sh +++ b/t/t4254-am-corrupt.sh @@ -25,7 +25,7 @@ test_expect_success setup ' # fatal: unable to write file '(null)' mode 100644: Bad address # Also, it had the unwanted side-effect of deleting f. test_expect_success 'try to apply corrupted patch' ' - test_must_fail git am bad-patch.diff 2>actual + test_must_fail git -c advice.amWorkDir=false am bad-patch.diff 2>actual ' test_expect_success 'compare diagnostic; ensure file is still here' ' -- 2.17.0.705.g3525833791
[PATCH v2 09/11] completion: keep other config var completion in camelCase
The last patch makes "git config " shows camelCase names because that's what's in the source: config.txt. There are still a couple manual var completion in this code. Let's make them follow the naming convention as well. In theory we could automate this part too because we have the information. But let's stick to one step at a time and leave this for later. Signed-off-by: Nguyễn Thái Ngọc Duy --- contrib/completion/git-completion.bash | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 1b0c30ed9a..efc930c9d1 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2249,20 +2249,20 @@ _git_config () ;; branch.*.*) local pfx="${cur%.*}." cur_="${cur##*.}" - __gitcomp "remote pushremote merge mergeoptions rebase" "$pfx" "$cur_" + __gitcomp "remote pushRemote merge mergeOptions rebase" "$pfx" "$cur_" return ;; branch.*) local pfx="${cur%.*}." cur_="${cur#*.}" __gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")" - __gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" "$cur_" + __gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" "$cur_" return ;; guitool.*.*) local pfx="${cur%.*}." cur_="${cur##*.}" __gitcomp " - argprompt cmd confirm needsfile noconsole norescan - prompt revprompt revunmerged title + argPrompt cmd confirm needsFile noConsole noRescan + prompt revPrompt revUnmerged title " "$pfx" "$cur_" return ;; @@ -2291,14 +2291,14 @@ _git_config () local pfx="${cur%.*}." cur_="${cur##*.}" __gitcomp " url proxy fetch push mirror skipDefaultUpdate - receivepack uploadpack tagopt pushurl + receivepack uploadpack tagOpt pushurl " "$pfx" "$cur_" return ;; remote.*) local pfx="${cur%.*}." cur_="${cur#*.}" __gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "." - __gitcomp_nl_append "pushdefault" "$pfx" "$cur_" + __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" return ;; url.*.*) -- 2.17.0.705.g3525833791
[PATCH v2 00/11] completion: avoid hard coding config var list
v2 changes: - based on 'next' - C99 is now mentioned in the commit message - fixed Eric's comments - the old 8/9 partial case-insensitive support patch is replaced with Szeder's version. Szeder I claimed authorship because I wrote the commit message which may not be what you like. If you want to claim it instead, I'll be glad to resend. - There is no levenshtein support yet. But the code is updated to keep this config list in memory instead of printf'ing directly to make it easier in the future to do that. Nguyễn Thái Ngọc Duy (11): Add and use generic name->id mapping code for color slot parsing grep: keep all colors in an array fsck: factor out msg_id_info[] lazy initialization code help: add --config to list all available config fsck: produce camelCase config key names advice: keep config name in camelCase in advice_config[] am: move advice.amWorkDir parsing back to advice.c completion: drop the hard coded list of config vars completion: keep other config var completion in camelCase completion: support case-insensitive config vars log-tree: allow to customize 'grafted' color Documentation/config.txt | 3 +- Documentation/git-help.txt | 5 + advice.c | 53 ++-- advice.h | 1 + builtin/am.c | 6 +- builtin/branch.c | 29 +- builtin/clean.c| 29 +- builtin/commit.c | 36 +-- builtin/help.c | 16 ++ config.c | 13 + config.h | 4 + contrib/completion/git-completion.bash | 357 ++--- diff.c | 56 ++-- fsck.c | 68 +++-- generate-cmdlist.sh| 19 ++ grep.c | 109 grep.h | 21 +- help.c | 84 ++ help.h | 45 +++- log-tree.c | 37 +-- t/t4254-am-corrupt.sh | 2 +- 21 files changed, 440 insertions(+), 553 deletions(-) -- 2.17.0.705.g3525833791
[PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing
Instead of hard coding the name-to-id mapping in C code, keep it in an array and use a common function to do the parsing. This reduces code and also allows us to list all possible color slots later. This starts using C99 designated initializers more for convenience (the first designated initializers have been introduced in builtin/clean.c for some time without complaints) Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/branch.c | 28 + builtin/clean.c | 28 + builtin/commit.c | 33 ++ config.c | 13 config.h | 4 diff.c | 53 +++- log-tree.c | 35 7 files changed, 82 insertions(+), 112 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index f85d3de531..09232576b6 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -55,26 +55,18 @@ enum color_branch { BRANCH_COLOR_UPSTREAM = 5 }; +static const char *color_branch_slots[] = { + [BRANCH_COLOR_RESET]= "reset", + [BRANCH_COLOR_PLAIN]= "plain", + [BRANCH_COLOR_REMOTE] = "remote", + [BRANCH_COLOR_LOCAL]= "local", + [BRANCH_COLOR_CURRENT] = "current", + [BRANCH_COLOR_UPSTREAM] = "upstream", +}; + static struct string_list output = STRING_LIST_INIT_DUP; static unsigned int colopts; -static int parse_branch_color_slot(const char *slot) -{ - if (!strcasecmp(slot, "plain")) - return BRANCH_COLOR_PLAIN; - if (!strcasecmp(slot, "reset")) - return BRANCH_COLOR_RESET; - if (!strcasecmp(slot, "remote")) - return BRANCH_COLOR_REMOTE; - if (!strcasecmp(slot, "local")) - return BRANCH_COLOR_LOCAL; - if (!strcasecmp(slot, "current")) - return BRANCH_COLOR_CURRENT; - if (!strcasecmp(slot, "upstream")) - return BRANCH_COLOR_UPSTREAM; - return -1; -} - static int git_branch_config(const char *var, const char *value, void *cb) { const char *slot_name; @@ -86,7 +78,7 @@ static int git_branch_config(const char *var, const char *value, void *cb) return 0; } if (skip_prefix(var, "color.branch.", &slot_name)) { - int slot = parse_branch_color_slot(slot_name); + int slot = LOOKUP_CONFIG(color_branch_slots, slot_name); if (slot < 0) return 0; if (!value) diff --git a/builtin/clean.c b/builtin/clean.c index fad533a0a7..0ccd95e693 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -42,6 +42,15 @@ enum color_clean { CLEAN_COLOR_ERROR = 5 }; +static const char *color_interactive_slots[] = { + [CLEAN_COLOR_ERROR] = "error", + [CLEAN_COLOR_HEADER] = "header", + [CLEAN_COLOR_HELP] = "help", + [CLEAN_COLOR_PLAIN] = "plain", + [CLEAN_COLOR_PROMPT] = "prompt", + [CLEAN_COLOR_RESET] = "reset", +}; + static int clean_use_color = -1; static char clean_colors[][COLOR_MAXLEN] = { [CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED, @@ -82,23 +91,6 @@ struct menu_stuff { void *stuff; }; -static int parse_clean_color_slot(const char *var) -{ - if (!strcasecmp(var, "reset")) - return CLEAN_COLOR_RESET; - if (!strcasecmp(var, "plain")) - return CLEAN_COLOR_PLAIN; - if (!strcasecmp(var, "prompt")) - return CLEAN_COLOR_PROMPT; - if (!strcasecmp(var, "header")) - return CLEAN_COLOR_HEADER; - if (!strcasecmp(var, "help")) - return CLEAN_COLOR_HELP; - if (!strcasecmp(var, "error")) - return CLEAN_COLOR_ERROR; - return -1; -} - static int git_clean_config(const char *var, const char *value, void *cb) { const char *slot_name; @@ -113,7 +105,7 @@ static int git_clean_config(const char *var, const char *value, void *cb) return 0; } if (skip_prefix(var, "color.interactive.", &slot_name)) { - int slot = parse_clean_color_slot(slot_name); + int slot = LOOKUP_CONFIG(color_interactive_slots, slot_name); if (slot < 0) return 0; if (!value) diff --git a/builtin/commit.c b/builtin/commit.c index a842fea666..2b43a6c48a 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -66,6 +66,18 @@ N_("If you wish to skip this commit, use:\n" "Then \"git cherry-pick --continue\" will resume cherry-picking\n" "the remaining commits.\n"); +static const char *color_status_slots[] = { + [WT_STATUS_HEADER]= "header", + [WT_STATUS_UPDATED] = "updated", + [WT_STATUS_CHANGED] = "changed", + [WT_STATUS_UNTRACKED] = "untracked", + [WT_STATUS_NOBRANCH] = "noBranch", + [WT_STATUS_UNMERGED] = "unmerged", + [WT_STATUS_LOCA
Re: [PATCH v3] checkout & worktree: introduce checkout.implicitRemote
On Fri, May 25, 2018 at 8:38 PM, Ævar Arnfjörð Bjarmason wrote: > > On Fri, May 25 2018, Duy Nguyen wrote: > >> On Fri, May 25, 2018 at 10:12 AM, Junio C Hamano wrote: +Currently this is used by linkgit:git-checkout[1] when 'git checkout +' will checkout the '' branch on another remote, +and by linkgit:git-worktree[1] when 'git worktree add' refers to a +remote branch. This setting might be used for other checkout-like +commands or functionality in the future. >>> >>> Hmph, that is an interesting direction. You foresee that you'll >>> have a single repository with multiple remotes to grab and share >>> objects from different people working on the same project, and use >>> multiple worktrees to work on different branches, yet you are happy >>> to declare that each worktree is to work with one particular remote? >>> >>> We'd need a per-worktree config file to make it work, I guess, or >>> a three-level checkout.$worktree_id.defaultRemote configuration >>> variable, perhaps? >> >> I still plan to revisit per-worktree config support [1] at some point >> and finish it off. Or is it decided that we don't need a generic >> mechanism and will add a new level like this for each config var that >> is per-worktree? If defaultRemote sets a precedence, then it'll be the >> way to go from now on or we'll have another mess when some config does >> "foo.$worktree.bar" while others use per-worktree config files. > > What do you think about including worktree in this at this time? I'm not > very familiar with it and just included it because I worked my way > backwards from the DWIM function, but I could exclude it if you think > it's too much trouble, i.e. if you'd like to hold out for some facility > like the per-worktree config Junio mentioned. I think Junio was talking about the future. What is currently in the patch, both code and document, is fine. But if you're going to implement core.$worktree.defaultRemote at this time, maybe wait a bit. I could try to resurrect the per-worktree config topic in a month or so and if it does not work out, then everybody will add new config vars if they want per-worktree support. -- Duy
[PATCH v2 3/4] t2203: add a test about "diff HEAD" case
Previous attempts to fix ita-related diffs breaks this case. To make sure that does not happen again, add a test to verify the behavior wrt. ita entries when we diff a worktree and a tree. Signed-off-by: Nguyễn Thái Ngọc Duy --- t/t2203-add-intent.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 1115347712..546fead6ad 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -245,4 +245,21 @@ test_expect_success 'diff-files/diff-cached shows ita as new/not-new files' ' test_cmp expected2 actual2 ' +test_expect_success '"diff HEAD" includes ita as new files' ' + git reset --hard && + echo new >new-ita && + git add -N new-ita && + git diff HEAD >actual && + cat >expected <<-\EOF && + diff --git a/new-ita b/new-ita + new file mode 100644 + index 000..3e75765 + --- /dev/null + +++ b/new-ita + @@ -0,0 +1 @@ + +new + EOF + test_cmp expected actual +' + test_done -- 2.17.0.705.g3525833791
[PATCH v2 4/4] apply: add --intent-to-add
Similar to 'git reset -N', this option makes 'git apply' automatically mark new files as intent-to-add so they are visible in the following 'git diff' command and could also be committed with 'git commit -a'. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git-apply.txt | 10 +- apply.c | 19 --- apply.h | 1 + t/t2203-add-intent.sh | 13 + 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt index 4ebc3d3271..e3b966c656 100644 --- a/Documentation/git-apply.txt +++ b/Documentation/git-apply.txt @@ -9,7 +9,7 @@ git-apply - Apply a patch to files and/or to the index SYNOPSIS [verse] -'git apply' [--stat] [--numstat] [--summary] [--check] [--index] [--3way] +'git apply' [--stat] [--numstat] [--summary] [--check] [--index | --intent-to-add] [--3way] [--apply] [--no-add] [--build-fake-ancestor=] [-R | --reverse] [--allow-binary-replacement | --binary] [--reject] [-z] [-p] [-C] [--inaccurate-eof] [--recount] [--cached] @@ -74,6 +74,14 @@ OPTIONS cached data, apply the patch, and store the result in the index without using the working tree. This implies `--index`. +--intent-to-add:: + When applying the patch only to the working tree, mark new + files to be added to the index later (see `--intent-to-add` + option in linkgit:git-add[1]). This option is ignored unless + running in a Git repository and `--index` is not specified. + Note that `--index` could be implied by other options such + as `--cached` or `--3way`. + -3:: --3way:: When the patch does not apply cleanly, fall back on 3-way merge if diff --git a/apply.c b/apply.c index 7e5792c996..899c5cc0ac 100644 --- a/apply.c +++ b/apply.c @@ -141,6 +141,8 @@ int check_apply_state(struct apply_state *state, int force_apply) return error(_("--cached outside a repository")); state->check_index = 1; } + if (state->ita_only && (state->check_index || is_not_gitdir)) + state->ita_only = 0; if (state->check_index) state->unsafe_paths = 0; @@ -4242,7 +4244,7 @@ static void patch_stats(struct apply_state *state, struct patch *patch) static int remove_file(struct apply_state *state, struct patch *patch, int rmdir_empty) { - if (state->update_index) { + if (state->update_index && !state->ita_only) { if (remove_file_from_cache(patch->old_name) < 0) return error(_("unable to remove %s from index"), patch->old_name); } @@ -4265,15 +4267,15 @@ static int add_index_file(struct apply_state *state, int namelen = strlen(path); unsigned ce_size = cache_entry_size(namelen); - if (!state->update_index) - return 0; - ce = xcalloc(1, ce_size); memcpy(ce->name, path, namelen); ce->ce_mode = create_ce_mode(mode); ce->ce_flags = create_ce_flags(0); ce->ce_namelen = namelen; - if (S_ISGITLINK(mode)) { + if (state->ita_only) { + ce->ce_flags |= CE_INTENT_TO_ADD; + set_object_name_for_intent_to_add_entry(ce); + } else if (S_ISGITLINK(mode)) { const char *s; if (!skip_prefix(buf, "Subproject commit ", &s) || @@ -4465,8 +4467,9 @@ static int create_file(struct apply_state *state, struct patch *patch) if (patch->conflicted_threeway) return add_conflicted_stages_file(state, patch); - else + else if (state->update_index) return add_index_file(state, path, mode, buf, size); + return 0; } /* phase zero is to remove, phase one is to create */ @@ -4686,7 +4689,7 @@ static int apply_patch(struct apply_state *state, if (state->whitespace_error && (state->ws_error_action == die_on_ws_error)) state->apply = 0; - state->update_index = state->check_index && state->apply; + state->update_index = (state->check_index || state->ita_only) && state->apply; if (state->update_index && !is_lock_file_locked(&state->lock_file)) { if (state->index_file) hold_lock_file_for_update(&state->lock_file, @@ -4941,6 +4944,8 @@ int apply_parse_options(int argc, const char **argv, N_("instead of applying the patch, see if the patch is applicable")), OPT_BOOL(0, "index", &state->check_index, N_("make sure the patch is applicable to the current index")), + OPT_BOOL('N', "intent-to-add", &state->ita_only, + N_("mark new files with `git add --intent-to-add`")), OPT_BOOL(0, "cached", &state->cached, N_("apply a patch without touching the working tree")),
[PATCH v2 1/4] diff: ignore --ita-[in]visible-in-index when diffing worktree-to-tree
This option is supposed to fix the diff of "diff-files" (not reporting ita entries as new files) and "diff-index --cached " ( showing ita entries as present in the index with empty content) but not "diff-index ". When --ita-invisible-in-index is set on "git diff-index ", unpack_trees() will eventually call oneway_diff() on the ita entry with the same code flow as "diff-index --cached ". We want to ignore the ita entry for "diff-index --cached " but not "diff-index " since the latter will examine and produce a diff based on worktree entry's (real) content, not ita index entry's (empty) content. Signed-off-by: Nguyễn Thái Ngọc Duy --- diff-lib.c| 8 ++-- t/t2203-add-intent.sh | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 104f954a25..a9f38eb5a3 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -389,8 +389,12 @@ static void do_oneway_diff(struct unpack_trees_options *o, struct rev_info *revs = o->unpack_data; int match_missing, cached; - /* i-t-a entries do not actually exist in the index */ - if (revs->diffopt.ita_invisible_in_index && + /* +* i-t-a entries do not actually exist in the index (if we're +* looking at its content) +*/ + if (o->index_only && + revs->diffopt.ita_invisible_in_index && idx && ce_intent_to_add(idx)) { idx = NULL; if (!tree) diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 78236dc7d8..3ab07cb404 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -70,7 +70,7 @@ test_expect_success 'i-t-a entry is simply ignored' ' git commit -m second && test $(git ls-tree HEAD -- nitfol | wc -l) = 0 && test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 && - test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | wc -l) = 0 && + test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | wc -l) = 1 && test $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) = 1 ' -- 2.17.0.705.g3525833791
[PATCH v2 0/4] Fix i-t-a entries in git-diff and git-apply
v2 first fixes a bug in --ita-invisible-in-index that accidentally affects diffing a worktree and a tree. This caused a regression when v1's 1/2 turned this option on by default. Other than that, 4/4 should address Junio's comments on v1's 2/2. Nguyễn Thái Ngọc Duy (4): diff: ignore --ita-[in]visible-in-index when diffing worktree-to-tree diff: turn --ita-invisible-in-index on by default t2203: add a test about "diff HEAD" case apply: add --intent-to-add Documentation/git-apply.txt | 10 +- apply.c | 19 +++ apply.h | 1 + builtin/diff.c | 7 diff-lib.c | 8 +++-- t/t2203-add-intent.sh | 64 + t/t4011-diff-symlink.sh | 10 +++--- 7 files changed, 99 insertions(+), 20 deletions(-) -- 2.17.0.705.g3525833791
[PATCH v2 2/4] diff: turn --ita-invisible-in-index on by default
Due to the implementation detail of intent-to-add entries, the current "git diff" (i.e. no treeish or --cached argument) would show the changes in the i-t-a file, but it does not mark the file as new, while "diff --cached" would mark the file as new while showing its content as empty. $ git diff | $ diff --cached |--- diff --git a/new b/new | diff --git a/new b/new index e69de29..5ad28e2 100644 | new file mode 100644 --- a/new | index 000..e69de29 +++ b/new | @@ -0,0 +1 @@ | +haha | One evidence of the current output being wrong is that, the output from "git diff" (with ita entries) cannot be applied because it assumes empty files exist before applying. Turning on --ita-invisible-in-index [1] [2] would fix this. The result is "new file" line moving from "git diff --cached" to "git diff". $ git diff | $ diff --cached |--- diff --git a/new b/new | new file mode 100644 | index 000..5ad28e2 | --- /dev/null | +++ b/new | @@ -0,0 +1 @@ | +haha | This option is on by default in git-status [1] but we need more fixup in rename detection code [3]. Luckily we don't need to do anything else for the rename detection code in diff.c (wt-status.c uses a customized one). [1] 425a28e0a4 (diff-lib: allow ita entries treated as "not yet exist in index" - 2016-10-24) [2] b42b451919 (diff: add --ita-[in]visible-in-index - 2016-10-24) [3] bc3dca07f4 (Merge branch 'nd/ita-wt-renames-in-status' - 2018-01-23) Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/diff.c | 7 +++ t/t2203-add-intent.sh | 34 -- t/t4011-diff-symlink.sh | 10 ++ 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/builtin/diff.c b/builtin/diff.c index 16bfb22f73..00424c296d 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -352,6 +352,13 @@ int cmd_diff(int argc, const char **argv, const char *prefix) rev.diffopt.flags.allow_external = 1; rev.diffopt.flags.allow_textconv = 1; + /* +* Default to intent-to-add entries invisible in the +* index. This makes them show up as new files in diff-files +* and not at all in diff-cached. +*/ + rev.diffopt.ita_invisible_in_index = 1; + if (nongit) die(_("Not a git repository")); argc = setup_revisions(argc, argv, &rev, NULL); diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 3ab07cb404..1115347712 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -70,8 +70,7 @@ test_expect_success 'i-t-a entry is simply ignored' ' git commit -m second && test $(git ls-tree HEAD -- nitfol | wc -l) = 0 && test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 && - test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | wc -l) = 1 && - test $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) = 1 + test $(git diff --name-only -- nitfol | wc -l) = 1 ' test_expect_success 'can commit with an unrelated i-t-a entry in index' ' @@ -99,13 +98,13 @@ test_expect_success 'cache-tree invalidates i-t-a paths' ' : >dir/bar && git add -N dir/bar && - git diff --cached --name-only >actual && + git diff --name-only >actual && echo dir/bar >expect && test_cmp expect actual && git write-tree >/dev/null && - git diff --cached --name-only >actual && + git diff --name-only >actual && echo dir/bar >expect && test_cmp expect actual ' @@ -186,7 +185,19 @@ test_expect_success 'rename detection finds the right names' ' cat >expected.3 <<-EOF && 2 .R N... 100644 100644 100644 $hash $hash R100 third first EOF - test_cmp expected.3 actual.3 + test_cmp expected.3 actual.3 && + + git diff --stat >actual.4 && + cat >expected.4 <<-EOF && +first => third | 0 +1 file changed, 0 insertions(+), 0 deletions(-) + EOF + test_cmp expected.4 actual.4 && + + git diff --cached --stat >actual.5 && + : >expected.5 && + test_cmp expected.5 actual.5 + ) ' @@ -222,5 +233,16 @@ test_expect_success 'double rename detection in status' ' ) ' -test_done +test_expect_success 'diff-files/diff-cached shows ita as new/not-new files' ' + git reset --hard && + echo new >new-ita && + git add -N new-ita && + git diff --summary >actual && +
[PATCH] upload-pack: reject shallow requests that would return nothing
Shallow clones with --shallow-since or --shalow-exclude work by running rev-list to get all reachable commits, then draw a boundary between reachable and unreachable and send "shallow" requests based on that. The code does miss one corner case: if rev-list returns nothing, we'll have no border and we'll send no shallow requests back to the client (i.e. no history cuts). This essentially means a full clone (or a full branch if the client requests just one branch). One example is the oldest commit is older than what is specified by --shallow-since. To avoid this, if rev-list returns nothing, we abort the clone/fetch. The user could adjust their request (e.g. --shallow-since further back in the past) and retry. Another possible option for this case is to fall back to a default depth (like depth 1). But I don't like too much magic that way because we may return something unexpected to the user. If they request "history since 2008" and we return a single depth at 2000, that might break stuff for them. It is better to tell them that something is wrong and let them take the best course of action. Note that we need to die() in get_shallow_commits_by_rev_list() instead of just checking for empty result from its caller deepen_by_rev_list() and handling the error there. The reason is, empty result could be a valid case: if you have commits in year 2013 and you request --shallow-since=year.2000 then you should get a full clone (i.e. empty result). Reported-by: Andreas Krey Signed-off-by: Nguyễn Thái Ngọc Duy --- shallow.c | 3 +++ t/t5500-fetch-pack.sh | 11 +++ 2 files changed, 14 insertions(+) diff --git a/shallow.c b/shallow.c index df4d44ea7a..44fdca1ace 100644 --- a/shallow.c +++ b/shallow.c @@ -175,6 +175,9 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, const char **av, die("revision walk setup failed"); traverse_commit_list(&revs, show_commit, NULL, ¬_shallow_list); + if (!not_shallow_list) + die("no commits selected for shallow requests"); + /* Mark all reachable commits as NOT_SHALLOW */ for (p = not_shallow_list; p; p = p->next) p->item->object.flags |= not_shallow_flag; diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 0680dec808..c8f2d38a58 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -711,6 +711,17 @@ test_expect_success 'fetch shallow since ...' ' test_cmp expected actual ' +test_expect_success 'clone shallow since selects no commits' ' + test_create_repo shallow-since-the-future && + ( + cd shallow-since-the-future && + GIT_COMMITTER_DATE="1 +0700" git commit --allow-empty -m one && + GIT_COMMITTER_DATE="2 +0700" git commit --allow-empty -m two && + GIT_COMMITTER_DATE="3 +0700" git commit --allow-empty -m three && + test_must_fail git clone --shallow-since "9 +0700" "file://$(pwd)/." ../shallow111 + ) +' + test_expect_success 'shallow clone exclude tag two' ' test_create_repo shallow-exclude && ( -- 2.17.0.705.g3525833791
RE: [PATCH 1/2] remote-curl: accept all encoding supported by curl
Hi Jonathan, I'd like to confirm, that your patch fixes my problem: I can sync with google repository now using git and curl version without gzip support. Do you know how this patch is going to be released? Just HEAD now and GA in the next planned release? Communication looks like follow now: root@bsb:~# GIT_TRACE_PACKET=1 GIT_CURL_VERBOSE=1 git ls-remote https://source.developers.google.co m/p/wired-balm-187912/r/dotfiles 2>&1 | sed -e 's/\(git-[^=]*\)=.*/\1=REDACTED/' -e 's/Authorizatio n: .*/Authorization: REDACTED/' > GET /p/wired-balm-187912/r/dotfiles/info/refs?service=git-upload-pack HTTP/1.1 Host: source.developers.google.com User-Agent: git/2.17.0 Accept: */* Accept-Encoding: identity Cookie: o=git-anton.golubev.gmail.com=REDACTED Pragma: no-cache < HTTP/1.1 200 OK < Cache-Control: no-cache, max-age=0, must-revalidate < Content-Length: 374 < Content-Type: application/x-git-upload-pack-advertisement < Expires: Fri, 01 Jan 1980 00:00:00 GMT < Pragma: no-cache < X-Content-Type-Options: nosniff < X-Frame-Options: SAMEORIGIN < X-Xss-Protection: 1; mode=block < Date: Sat, 26 May 2018 11:04:41 GMT < Alt-Svc: hq=":443"; ma=2592000; quic=51303433; quic=51303432; quic=51303431; quic=51303339; quic=51303335,quic=":443"; ma=2592000; v="43,42,41,39,35" < 13:04:41.274561 pkt-line.c:80 packet: git< # service=git-upload-pack 13:04:41.274634 pkt-line.c:80 packet: git< 13:04:41.274693 pkt-line.c:80 packet: git< 45e2c99dd1790529cc4b7e029b1e9dfcc817d18e HEAD\0 include-tag multi_ack_detailed multi_ack ofs-delta side-band side-band-64k thin-pack no-progress shallow no-done allow-tip-sha1-in-want allow-reachable-sha1-in-want agent=JGit/4-google filter symref=HEAD:refs/heads/master 13:04:41.274739 pkt-line.c:80 packet: git< 45e2c99dd1790529cc4b7e029b1e9dfcc817d18e refs/heads/master 13:04:41.274777 pkt-line.c:80 packet: git< 45e2c99dd1790529cc4b7e029b1e9dfcc817d18eHEAD 45e2c99dd1790529cc4b7e029b1e9dfcc817d18erefs/heads/master Kind regards, Anton Golubev -Original Message- From: Jonathan Nieder [mailto:jrnie...@gmail.com] Sent: Dienstag, 22. Mai 2018 02:01 To: Brandon Williams Cc: git@vger.kernel.org; Anton Golubev Subject: Re: [PATCH 1/2] remote-curl: accept all encoding supported by curl Hi, Brandon Williams wrote: > Subject: remote-curl: accept all encoding supported by curl nit: s/encoding/encodings > Configure curl to accept all encoding which curl supports instead of > only accepting gzip responses. Likewise. > This is necessary to fix a bug when using an installation of curl > which doesn't support gzip. Since curl doesn't do any checking to > verify that it supports the encoding set when calling > 'curl_easy_setopt()', curl can end up sending an "Accept-Encoding" > header indicating that it supports a particular encoding when in fact > it doesn't. Instead when the empty string "" is used when setting > `CURLOPT_ENCODING`, curl will send an "Accept-Encoding" header > containing only the encoding methods curl supports. > > Signed-off-by: Brandon Williams Thanks for the analysis and fix. Reported-by: Anton Golubev Also ccing the reporter so we can hopefully get a tested-by. Anton, can you test this patch and let us know how it goes? You can apply it as follows: curl \ https://public-inbox.org/git/20180521234004.142548-1-bmw...@google.com/raw \ >patch.txt git am -3 patch.txt Brandon, can the commit message also say a little more about the motivating context and symptoms? $ curl --version curl 7.52.1 (arm-openwrt-linux-gnu) libcurl/7.52.1 mbedTLS/2.6.0 Protocols: file ftp ftps http https Features: IPv6 Largefile SSL The issue is that when curl is built without the "zlib" feature, since v1.8.0-rc0~14^2 (Enable info/refs gzip decompression in HTTP client, 2012-09-19) we end up requesting "gzip" encoding anyway despite libcurl not being able to decode it. Worse, instead of getting a clear error message indicating so, we end up falling back to "dumb" http, producing a confusing and difficult to debug result. > --- > http.c | 2 +- > remote-curl.c | 2 +- > t/t5551-http-fetch-smart.sh | 4 ++-- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/http.c b/http.c > index fed13b216..709150fc7 100644 > --- a/http.c > +++ b/http.c > @@ -1788,7 +1788,7 @@ static int http_request(const char *url, > > curl_easy_setopt(slot->curl, CURLOPT_URL, url); > curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers); > - curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip"); > + curl_easy_setopt(slot->curl, CURLOPT_ENCODING, ""); > > ret = run_one_slot(slot, &results); > > diff --git a/remote-curl.c b/remote-curl.c index ceb05347b..565bba104 > 100644 > --- a/remote-curl.c > +++ b/remote-curl.c > @@ -684,7 +684,7 @@ static int post_rpc(struct rpc_state *rpc) > curl_e