[PATCH] filter-branch: skip commits present on --state-branch
The commits in state:filter.map have already been processed, so don't filter them again. This makes incremental git filter-branch much faster. Also add tests for --state-branch option. Signed-off-by: Michael Barabanov --- git-filter-branch.sh | 3 +++ t/t7003-filter-branch.sh | 15 +++ 2 files changed, 18 insertions(+) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index ccceaf19a..2df7ed107 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -372,6 +372,9 @@ while read commit parents; do git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1)) report_progress + if test -r "$workdir/../map/$commit"; then + continue + fi case "$filter_subdir" in "") diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index ec4b160dd..e23de7d0b 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh @@ -107,6 +107,21 @@ test_expect_success 'test that the directory was renamed' ' test dir/D = "$(cat diroh/D.t)" ' +V=$(git rev-parse HEAD) + +test_expect_success 'populate --state-branch' ' + git filter-branch --state-branch state -f --tree-filter "touch file || :" HEAD +' + +W=$(git rev-parse HEAD) + +test_expect_success 'using --state-branch to skip already rewritten commits' ' + test_when_finished git reset --hard $V && + git reset --hard $V && + git filter-branch --state-branch state -f --tree-filter "touch file || :" HEAD && + test_cmp_rev $W HEAD +' + git tag oldD HEAD~4 test_expect_success 'rewrite one branch, keeping a side branch' ' git branch modD oldD && -- 2.17.1
[no subject]
Could you act as the beneficiary to claim 9.8M USD that my bank wants to confiscate? I will give you 50% and Every documentation would be put in placed. Mr Ubaithullah from Hong Kong.
^D to credentials prompt results in "fatal: ... Success"
A bit of an amusing edge case.
I'm not exactly sure the correct approach to fix this but here's my
reproduction, triage, and a few potential options I see.
Note that after the username prompt, I pressed ^D
$./prefix/bin/git --version
git version 2.18.0
$ PATH=$PWD/prefix/bin:$PATH git clone
https://github.com/asottile/this-does-not-exist-i-promise
Cloning into 'this-does-not-exist-i-promise'...
Username for 'https://github.com': fatal: could not read Username for
'https://github.com': Success
Looking at `prompt.c`, it's hitting this bit of code:
if (git_env_bool("GIT_TERMINAL_PROMPT", 1)) {
r = git_terminal_prompt(prompt, flags & PROMPT_ECHO);
err = strerror(errno);
} else {
err = "terminal prompts disabled";
}
if (!r) {
/* prompts already contain ": " at the end */
die("could not read %s%s", prompt, err);
}
>From `git_terminal_prompt` (compat/terminal.c):
r = strbuf_getline_lf(, input_fh);
if (!echo) {
putc('\n', output_fh);
fflush(output_fh);
}
restore_term();
fclose(input_fh);
fclose(output_fh);
if (r == EOF)
return NULL;
return buf.buf;
in the `EOF` case, this is returning `NULL`, but `errno = 0` at this
point causing the error string to be "Success"
I see a couple of options here:
1. special case EOF in `git_terminal_prompt` / `git_prompt` and
produce an error message such as:
fatal: could not read Username for 'https://github.com': EOF
(I tried returing `EOF` directly from `git_terminal_prompt` and was
able to get this messaging to work, however `r == EOF` is a
pointer-int comparison so this approach didn't really seem like a good
idea -- changing the signature of `git_terminal_prompt` to set a
special flag for EOF is another option, but seems a lot of work for a
case that probably doesn't happen all too often)
I also couldn't find an appropriate errno to set in the EOF case either
2. treat EOF less specially
The function this is replacing, `getpass` simply returns an empty
string on `EOF`. This patch would implement that:
diff --git a/compat/terminal.c b/compat/terminal.c
index fa13ee672..8bd08108e 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -122,7 +122,7 @@ char *git_terminal_prompt(const char *prompt, int echo)
fputs(prompt, output_fh);
fflush(output_fh);
- r = strbuf_getline_lf(, input_fh);
+ strbuf_getline_lf(, input_fh);
if (!echo) {
putc('\n', output_fh);
fflush(output_fh);
@@ -132,8 +132,6 @@ char *git_terminal_prompt(const char *prompt, int echo)
fclose(input_fh);
fclose(output_fh);
- if (r == EOF)
- return NULL;
return buf.buf;
}
however then the output is a bit strange for ^D (note I pressed ^D twice):
$ PATH=$PWD/prefix/bin:$PATH git clone
https://github.com/asottile/this-does-not-exist-i-promise
Cloning into 'this-does-not-exist-i-promise'...
Username for 'https://github.com': Password for 'https://github.com':
remote: Repository not found.
fatal: Authentication failed for
'https://github.com/asottile/this-does-not-exist-i-promise/'
Anthony
Re: [PATCH v3 8/8] fetch-pack: implement ref-in-want
Hi,
Brandon Williams wrote:
> Implement ref-in-want on the client side so that when a server supports
> the "ref-in-want" feature, a client will send "want-ref" lines for each
> reference the client wants to fetch.
>
> Signed-off-by: Brandon Williams
> ---
> fetch-pack.c | 35 +++---
> remote.c | 1 +
> remote.h | 1 +
> t/t5703-upload-pack-ref-in-want.sh | 4 ++--
> 4 files changed, 36 insertions(+), 5 deletions(-)
This commit message doesn't tell me what ref-in-want is or is for. Could
it include
A. a pointer to Documentation/technical/protocol-v2.txt, or
B. an example illustrating the effect e.g. using GIT_TRACE_PACKET
or both?
[...]
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1102,9 +1102,10 @@ static void add_shallow_requests(struct strbuf
> *req_buf,
>
> static void add_wants(const struct ref *wants, struct strbuf *req_buf)
> {
> + int use_ref_in_want = server_supports_feature("fetch", "ref-in-want",
> 0);
> +
> for ( ; wants ; wants = wants->next) {
Not about this patch: it's kind of confusing that the iterator is called
'wants' even though it points into the middle of the list. I would even
be tempted to do
const struct ref *want;
for (want = wants; want; want = want->next) {
It wouldn't make sense to do in this patch, though.
[...]
> @@ -1122,8 +1123,10 @@ static void add_wants(const struct ref *wants, struct
> strbuf *req_buf)
> continue;
> }
>
> - remote_hex = oid_to_hex(remote);
> - packet_buf_write(req_buf, "want %s\n", remote_hex);
> + if (!use_ref_in_want || wants->exact_oid)
> + packet_buf_write(req_buf, "want %s\n",
> oid_to_hex(remote));
> + else
> + packet_buf_write(req_buf, "want-ref %s\n", wants->name);
Very neat.
[...]
> @@ -1334,6 +1337,29 @@ static void receive_shallow_info(struct
> fetch_pack_args *args,
> args->deepen = 1;
> }
>
> +static void receive_wanted_refs(struct packet_reader *reader, struct ref
> *refs)
> +{
> + process_section_header(reader, "wanted-refs", 0);
> + while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
> + struct object_id oid;
> + const char *end;
> + struct ref *r = NULL;
> +
> + if (parse_oid_hex(reader->line, , ) || *end++ != ' ')
> + die("expected wanted-ref, got '%s'", reader->line);
> +
> + for (r = refs; r; r = r->next) {
> + if (!strcmp(end, r->name)) {
> + oidcpy(>old_oid, );
> + break;
> + }
Stefan mentioned that the spec says
* The server MUST NOT send any refs which were not requested
using 'want-ref' lines.
Can client enforce that? If not, can the spec say SHOULD NOT for the
server and add a MUST describing appropriate client behavior?
> + }
> + }
> +
> + if (reader->status != PACKET_READ_DELIM)
The spec says
* This section is only included if the client has requested a
ref using a 'want-ref' line and if a packfile section is also
included in the response.
What should happen if the client already has all the relevant objects
(or in other words if there is no packfile to send in the packfile
section)? Is the idea that the client should already have known that
based on the ref advertisement? What if ref values change to put us
in that state between the ls-refs and fetch steps?
[...]
> --- a/remote.c
> +++ b/remote.c
> @@ -1735,6 +1735,7 @@ int get_fetch_map(const struct ref *remote_refs,
> if (refspec->exact_sha1) {
> ref_map = alloc_ref(name);
> get_oid_hex(name, _map->old_oid);
> + ref_map->exact_oid = 1;
Sensible. The alternative would be that we check whether the
refname is oid-shaped at want-ref generation time, which would be
unnecessarily complicated.
[...]
> --- a/remote.h
> +++ b/remote.h
> @@ -73,6 +73,7 @@ struct ref {
Not about this patch: why is this in remote.h instead of ref.h?
> force:1,
> forced_update:1,
> expect_old_sha1:1,
> + exact_oid:1,
> deletion:1;
Looks good, and we have room for plenty more bits there.
[...]
> +++ b/t/t5703-upload-pack-ref-in-want.sh
Nice.
Thanks for a pleasant read.
Sincerely,
Jonathan
Re: [PATCH v2 3/4] branch: deprecate "-l" option
On Fri, Jun 22, 2018 at 06:34:28PM -0400, Eric Sunshine wrote:
> I wonder if it would be better and cleaner to limit the visibility of
> this change to cmd_branch(), rather than spreading it across a global
> variable, a callback function, and cmd_branch(). Perhaps, like this:
I'd prefer that, too, but...
> @@ -615,7 +616,9 @@ int cmd_branch(int argc, const char **argv, const char
> *prefix)
> OPT_BIT('c', "copy", , N_("copy a branch and its reflog"),
> 1),
> OPT_BIT('C', NULL, , N_("copy a branch, even if target
> exists"), 2),
> OPT_BOOL(0, "list", , N_("list branch names")),
> - OPT_BOOL('l', "create-reflog", , N_("create the branch's
> reflog")),
> + OPT_BOOL(0, "create-reflog", , N_("create the branch's
> reflog")),
> + OPT_HIDDEN_BOOL('l', NULL, _reflog_option,
> + N_("deprecated synonym for --create-reflog")),
Now that "-l" processing is delayed, it interacts in a funny way with
--create-reflog. For instance:
git branch -l --no-create-reflog
currently cancels itself out, but after your patch would enable reflogs.
This is a pretty niche corner case, but I think it's important not to
change any behavior during the deprecation period. You'd have to do
something more like:
reflog = -1;
... parse options ...
if (deprecated_reflog_option && !list)
warning(...);
if (reflog < 0 && deprecated_reflog_option)
reflog = 1;
I think that probably works in all cases, but I actually think the
existing callback/global is less invasive.
-Peff
Re: [PATCH v3 0/8] Moved code detection: ignore space on uniform indentation
This seems to break the documentation build rather badly; I have a monkey-fix queued at the tip fo the topic branch for tonight's push.
Re: [PATCH v2 3/4] branch: deprecate "-l" option
On Fri, Jun 22, 2018 at 05:24:14AM -0400, Jeff King wrote:
> Let's deprecate "-l" in hopes of eventually re-purposing it
> to "--list".
>
> Signed-off-by: Jeff King
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> @@ -36,6 +36,7 @@ static const char * const builtin_branch_usage[] = {
> +static int used_deprecated_reflog_option;
> @@ -573,6 +574,14 @@ static int edit_branch_description(const char
> *branch_nam> +static int deprecated_reflog_option_cb(const struct option *opt,
> +const char *arg, int unset)
> +{
> + used_deprecated_reflog_option = 1;
> + *(int *)opt->value = !unset;
> + return 0;
> +}
> @@ -615,7 +624,13 @@ int cmd_branch(int argc, const char **argv, const char
> *p> + OPT_BOOL(0, "create-reflog", , N_("create the
> branch's reflog")),
> + {
> + OPTION_CALLBACK, 'l', NULL, , NULL,
> + N_("deprecated synonym for --create-reflog"),
> + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
> + deprecated_reflog_option_cb
> @@ -688,6 +703,11 @@ int cmd_branch(int argc, const char **argv, const char
> *p> + if (used_deprecated_reflog_option && !list) {
> + warning("the '-l' alias for '--create-reflog' is deprecated;");
> + warning("it will be removed in a future version of Git");
> + }
I wonder if it would be better and cleaner to limit the visibility of
this change to cmd_branch(), rather than spreading it across a global
variable, a callback function, and cmd_branch(). Perhaps, like this:
--- >8 ---
diff --git a/builtin/branch.c b/builtin/branch.c
index 5217ba3bde..893e5f481a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -584,6 +584,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
int icase = 0;
static struct ref_sorting *sorting = NULL, **sorting_tail =
struct ref_format format = REF_FORMAT_INIT;
+ int deprecated_reflog_option = 0;
struct option options[] = {
OPT_GROUP(N_("Generic options")),
@@ -615,7 +616,9 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT_BIT('c', "copy", , N_("copy a branch and its reflog"),
1),
OPT_BIT('C', NULL, , N_("copy a branch, even if target
exists"), 2),
OPT_BOOL(0, "list", , N_("list branch names")),
- OPT_BOOL('l', "create-reflog", , N_("create the branch's
reflog")),
+ OPT_BOOL(0, "create-reflog", , N_("create the branch's
reflog")),
+ OPT_HIDDEN_BOOL('l', NULL, _reflog_option,
+ N_("deprecated synonym for --create-reflog")),
OPT_BOOL(0, "edit-description", _description,
N_("edit the description for the branch")),
OPT__FORCE(, N_("force creation, move/rename, deletion"),
PARSE_OPT_NOCOMPLETE),
@@ -688,6 +691,12 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (list)
setup_auto_pager("branch", 1);
+ if (deprecated_reflog_option && !list) {
+ reflog = 1;
+ warning("the '-l' alias for '--create-reflog' is deprecated;");
+ warning("it will be removed in a future version of Git");
+ }
+
if (delete) {
if (!argc)
die(_("branch name required"));
--- >8 ---
Re: [PATCH v2 8/8] fetch-pack: implement ref-in-want
Hi,
Brandon Williams wrote:
> On 06/14, Stefan Beller wrote:
> > On Wed, Jun 13, 2018 at 2:39 PM Brandon Williams wrote:
>>> + for (r = refs; r; r = r->next) {
>>> + if (!strcmp(end, r->name)) {
>>> + oidcpy(>old_oid, );
>>> + break;
>>> + }
>>> + }
>>
>> The server is documented as MUST NOT send additional refs,
>> which is fine here, as we'd have no way of storing them anyway.
>> Do we want to issue a warning, though?
>>
>> if (!r) /* never break'd */
>> warning ("server send unexpected line '%s'", reader.line);
>
> Depends, does this warning help out the end user or do you think it
> would confuse users to see this and still have their fetch succeed?
I think we'd want to error out instead of warning. That keeps the
spec simple and that way, server implementors will notice early if
they are doing something that clients aren't going to understand
anyway, which would benefit users.
Thanks,
Jonathan
Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'
On Fri, Jun 22, 2018 at 11:45:09PM +0200, Johannes Schindelin wrote:
> > This might be a candidate for another "weather balloon" patch to see if
> > anybody complains, though. The last time time we dealt with this in a
> > major way was over 7 years ago in 28bd70d811 (unbreak and eliminate
> > NO_C99_FORMAT, 2011-03-16).
> >
> > I know Johannes switched out some "%lu" for PRIuMAX as recently as last
> > August[1], but I think that is more about the Windows size_t not matching
> > "unsigned long", and the decision to use PRIuMAX was to match the
> > existing codebase. AFAIK %zu is available on Windows.
>
> Nope, it's not available:
>
> git.c: In function 'cmd_main':
> git.c:733:10: error: unknown conversion type character 'z' in format
> [-Werror=format=]
> die("x: %z", (void *)(intptr_t)0x123456789a);
Well, that resolve that, then. :)
Thanks for letting us know before we went down a dead-end.
-Peff
Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'
Hi Peff,
On Thu, 21 Jun 2018, Jeff King wrote:
> On Thu, Jun 21, 2018 at 07:53:02AM -0400, Jeff King wrote:
>
> > > @@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char
> > > *bol, char *eol,
> > >*/
> > > if (opt->columnnum && cno) {
> > > char buf[32];
> > > - xsnprintf(buf, sizeof(buf), "%d", cno);
> > > + xsnprintf(buf, sizeof(buf), "%zu", cno);
> >
> > Unfortunately %z isn't portable. You have to use:
> >
> > xsnprintf(buf, sizeof(buf), "%"PRIuMAX, (uintmax_t)cno);
>
> To clarify: yes, it is in C99. But traditionally we have not required
> that.
>
> This might be a candidate for another "weather balloon" patch to see if
> anybody complains, though. The last time time we dealt with this in a
> major way was over 7 years ago in 28bd70d811 (unbreak and eliminate
> NO_C99_FORMAT, 2011-03-16).
>
> I know Johannes switched out some "%lu" for PRIuMAX as recently as last
> August[1], but I think that is more about the Windows size_t not matching
> "unsigned long", and the decision to use PRIuMAX was to match the
> existing codebase. AFAIK %zu is available on Windows.
Nope, it's not available:
git.c: In function 'cmd_main':
git.c:733:10: error: unknown conversion type character 'z' in format
[-Werror=format=]
die("x: %z", (void *)(intptr_t)0x123456789a);
Ciao,
Dscho
Re: [PATCH v3 5/8] fetch: refactor fetch_refs into two functions
Brandon Williams wrote:
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -967,10 +967,16 @@ static int fetch_refs(struct transport *transport,
> struct ref *ref_map)
> int ret = quickfetch(ref_map);
> if (ret)
> ret = transport_fetch_refs(transport, ref_map);
> - if (!ret)
> - ret |= store_updated_refs(transport->url,
> - transport->remote->name,
> - ref_map);
> + if (ret)
> + transport_unlock_pack(transport);
> + return ret;
> +}
> +
> +static int consume_refs(struct transport *transport, struct ref *ref_map)
> +{
> + int ret = store_updated_refs(transport->url,
> + transport->remote->name,
> + ref_map);
> transport_unlock_pack(transport);
> return ret;
> }
[...]
> - fetch_refs(transport, ref_map);
> + if (!fetch_refs(transport, ref_map))
> + consume_refs(transport, ref_map);
>
Ah, I missed something in my previous reply.
If transport_fetch_refs succeeds and store_updated_refs fails, then in
the old code, transport_unlock_pack would clean up by removing the no
longer needed .keep file. In the new code, that's consume_refs's
responsibility, which I find much nicer. It's probably worth
mentioning that in the commit message as well.
Thanks again,
Jonathan
Re: [PATCH v3 5/8] fetch: refactor fetch_refs into two functions
Hi, Brandon Williams wrote: > [Subject: fetch: refactor fetch_refs into two functions] > > Refactor the fetch_refs function into a function that does the fetching > of refs and another function that stores them. > > Signed-off-by: Brandon Williams > --- > builtin/fetch.c | 19 +-- > 1 file changed, 13 insertions(+), 6 deletions(-) It's hard to understand the context for this patch based on this description alone. E.g. is fetch_refs too long to follow? Or are we about to expand it? Or are we going to use the factored-out subpart for something else? [...] > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -967,10 +967,16 @@ static int fetch_refs(struct transport *transport, > struct ref *ref_map) > int ret = quickfetch(ref_map); > if (ret) > ret = transport_fetch_refs(transport, ref_map); > - if (!ret) > - ret |= store_updated_refs(transport->url, > - transport->remote->name, > - ref_map); > + if (ret) > + transport_unlock_pack(transport); > + return ret; > +} Paraphrasing the old code: try quickfetch if that fails, we have to fetch for real both of the above "lock" a pack using a .keep file to avoid races if the fetch succeeded, now update the refs finally, "unlock" the pack by rm-ing the .keep file Paraphrasing the new code: try quickfetch if that fails, we have to fetch for real both of the above "lock" a pack using a .keep file to avoid races if the fetch failed, "unlock" the pack by rm-ing the .keep file. Do I understand correctly that this is preparation for changing the 'update refs' step? As a minor nit, I think this would be easier to read if we treat the unlock_pack as a destructor. Something like this: int ret = quickfetch(ref_map); if (ret) ret = transport_fetch_refs(transport, ref_map); if (!ret) /* * Keep the new pack's ".keep" file around to allow * the caller time to update refs to reference the new * objects. */ return 0; transport_unlock_pack(transport); return ret; > + > +static int consume_refs(struct transport *transport, struct ref *ref_map) The name consume_refs doesn't make it clear to me what this function is going to do. Maybe a function comment can help. I.e. either the name or a comment would tell me that this is going to update local refs based on the ref values that the remote end told us. The rest of the patch looks good. Thanks and hope that helps, Jonathan
Re: [PATCH 0/7] Restrict the usage of config_from_gitmodules()
On 06/22, Antonio Ospite wrote: > On Fri, 22 Jun 2018 10:13:10 -0700 > Brandon Williams wrote: > > [...] > > Thanks for working on this. I think its a good approach and the end > > result makes it much harder for arbitrary config to sneak back in to the > > .gitmodules file. And after this series it looks like you should be in > > a good place to read the .gitmodules file from other places (not just in > > the worktree). > > > > :) > > > As you've mentioned here I also agree we could do without the last patch > > but I'll leave that up to you. Other than a couple typos I found I > > think this series looks good! Thanks again for revisiting this. > > > > Thanks for the review. > > I understand your compromise solution for patch 7, but I'd say let's > keep it simple and just drop patch 7 for now. Yeah that's probably the best approach for the time being. > > I am going to wait a couple of days and then send a v2. Perfect, thanks again! -- Brandon Williams
Re: [PATCH v3 1/8] test-pkt-line: add unpack-sideband subcommand
Hi,
Brandon Williams wrote:
> Add an 'unpack-sideband' subcommand to the test-pkt-line helper to
> enable unpacking packet line data sent multiplexed using a sideband.
>
> Signed-off-by: Brandon Williams
> ---
> t/helper/test-pkt-line.c | 33 +
> 1 file changed, 33 insertions(+)
Neat. It appears that this writes sideband channel 1 (packfile data)
to stdout, sideband channel 2 (progress) to stderr, and sideband
channel 3 (errors) cause the helper to fail. It would have been nice
if a comment or the commit message said that, but it's no reason to
reroll --- the code is clear enough.
> --- a/t/helper/test-pkt-line.c
> +++ b/t/helper/test-pkt-line.c
> @@ -1,3 +1,4 @@
> +#include "cache.h"
> #include "pkt-line.h"
I think this is for write_or_die. Makes sense (well, in the same way
as any of the other functions that ended up in cache.h instead of a
more thought-through place do).
The old #includes were problematic, since the caller cannot count on
git-compat-util.h to be the first include of pkt-line.h. See
Documentation/CodingGuidelines "The first #include" for more on this
subject.
[...]
> +static void unpack_sideband(void)
> +{
> + struct packet_reader reader;
> + packet_reader_init(, 0, NULL, 0,
> +PACKET_READ_GENTLE_ON_EOF |
> +PACKET_READ_CHOMP_NEWLINE);
> +
> + while (packet_reader_read() != PACKET_READ_EOF) {
> + int band;
> + int fd;
> +
> + switch (reader.status) {
> + case PACKET_READ_EOF:
> + break;
> + case PACKET_READ_NORMAL:
> + band = reader.line[0] & 0xff;
reader.line[0] is a char. This promotes it to an 'int' and then ANDs
against 0xff, which would ensure it is a positive value. In other
words, this does the same thing as
band = (int) (unsigned char) reader.line[0];
but more concisely.
More importantly, it matches what recv_sideband does. Good.
Reviewed-by: Jonathan Nieder
Thanks.
Re: Unexpected ignorecase=false behavior on Windows
On Fri, Jun 22, 2018 at 1:45 PM Marc Strapetz wrote: > > On 22.06.2018 19:36, Johannes Sixt wrote: > > Am 22.06.2018 um 14:04 schrieb Marc Strapetz: > >> On Windows, when creating following repository: > >> > >> $ git init > >> $ echo "1" > file.txt > >> $ git add . > >> $ git commit -m "initial import" > >> $ ren file.txt File.txt > >> $ git config core.ignorecase false > > > > This is a user error. core.ignorecase is *not* an instruction as in > > "hey, Git, do not ignore the case of file names". It is better regarded > > as an internal value, with which Git remembers how it should treat the > > names of files that it receives when it traverses the directories on the > > disk. > > > > Git could probe the file system capabilities each time it runs. But that > > would be wasteful. Hence, this probe happens only once when the > > repository is initialized, and the result is recorded in this > > configuration value. You should not change it. > > Sorry, it looks like my example was misleading. I'm actually questioning > current behavior in case of Windows repositories with core.ignorecase > initialized to false, like in following setup: > > $ git init > $ git config core.ignorecase false > > The repository is now set up to be case-sensitive on Windows. From this > point on, core.ignorecase won't change anymore and the repository will > be filled: I don't think Hannes's point was _when_ you changed it; it was that you changed it _at all_. Git on Windows is not designed to run with anything other than core.ignoreCase=true, and attempting to do so will cause unexpected behavior. In other words, it's not a behavior toggle so user's can request the functionality to work one way or the other; it's an implementation detail that `git init` and `git clone` set when a repository is created purely so they don't have to probe the file system each time you run a `git` command. NTFS is case-preserving-but-case-insensitive by default[1]. So long as that's the case, the only mode for running Git on Windows is core.ignoreCase=true. Hopefully this clarifies things! Bryan [1] Windows 10 1803 introduced the ability to set a folder as case-sensitive[2], but, since it's not inherited automatically by subdirectories, it still doesn't work well for Git. [2] https://blogs.msdn.microsoft.com/commandline/2018/02/28/per-directory-case-sensitivity-and-wsl/ > > $ echo "1" > file.txt > $ git add . > $ git commit -m "initial import" > $ ren file.txt File.txt > > Still, status results are: > > $ git status --porcelain > ?? File.txt > > With the same setup sequence on Unix, it's: > > $ git status --porcelain >D file.txt > ?? File.txt > > Is this difference, which is depending on the platform, intended? Why > not report missing file.txt as well? > > The drawback of the current behavior is that a subsequent "git add ." > will result in two file names in the .git/index which are only differing > in case. This will break the repository on Windows, because only one of > both files can be checked out in the working tree. Also, it makes > case-only renames harder to be performed. > > -Marc
Re: [GSoC][PATCH v3 2/3] rebase -i: rewrite setup_reflog_action() in C
Hi Junio,
Le 22/06/2018 à 18:27, Junio C Hamano a écrit :
> Alban Gruin writes:
> > This rewrites (the misnamed) setup_reflog_action() from shell to C. The
> > new version is called checkout_base_commit().
>
> ;-) on the "misnamed" part. Indeed, setting up the comment for the
> reflog entry is secondary to what this function wants to do, which
> is to check out the branch to be rebased.
>
> I do not think "base_commit" is a good name, either, though. When I
> hear 'base' in the context of 'rebase', I would imagine that the
> speaker is talking about the bottom of the range of the commits to
> be rebased (i.e. "rebase --onto ONTO BASE BRANCH", which replays
> commits BASE..BRANCH on top of ONTO and then points BRANCH at the
> result), not the top of the range or the branch these commits are
> taken from.
>
Perhaps should I name this function checkout_onto(), and rename
checkout_onto() to "detach_onto()"? And I would reorder those two commits in
the series, as this would be very confusing.
> > index 51c8ab7ac..27f8453fe 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -3129,6 +3129,36 @@ static const char *reflog_message(struct
> > replay_opts *opts,>
> > return buf.buf;
> >
> > }
> >
> > +static int run_git_checkout(struct replay_opts *opts, const char *commit,
> > + int verbose, const char *action)
> > +{
> > + struct child_process cmd = CHILD_PROCESS_INIT;
> > +
> > + cmd.git_cmd = 1;
> > +
> > + argv_array_push(, "checkout");
> > + argv_array_push(, commit);
> > + argv_array_pushf(_array, GIT_REFLOG_ACTION "=%s", action);
> > +
> > + if (verbose)
> > + return run_command();
> > + return run_command_silent_on_success();
>
> For the same reason as 1/3, I think it makes more sense to have
> "else" here.
>
Right.
> > +int checkout_base_commit(struct replay_opts *opts, const char *commit,
> > +int verbose)
> > +{
> > + const char *action;
> > +
> > + if (commit && *commit) {
>
> Hmm, isn't it a programming error to feed !commit or !*commit here?
> I offhand do not think of a reason why making such an input a silent
> no-op success, instead of making it error out, would be a good idea.
>
I think it’s correct.
> > + action = reflog_message(opts, "start", "checkout %s", commit);
> > + if (run_git_checkout(opts, commit, verbose, action))
> > + return error(_("Could not checkout %s"), commit);
> > + }
> > +
> > + return 0;
> > +}
> > +
> >
> > static const char rescheduled_advice[] =
> > N_("Could not execute the todo command\n"
> > "\n"
> >
> > diff --git a/sequencer.h b/sequencer.h
> > index 35730b13e..42c3dda81 100644
> > --- a/sequencer.h
> > +++ b/sequencer.h
> > @@ -100,6 +100,9 @@ int update_head_with_reflog(const struct commit
> > *old_head,>
> > void commit_post_rewrite(const struct commit *current_head,
> >
> > const struct object_id *new_head);
> >
> > +int checkout_base_commit(struct replay_opts *opts, const char *commit,
> > +int verbose);
> > +
> >
> > #define SUMMARY_INITIAL_COMMIT (1 << 0)
> > #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
> > void print_commit_summary(const char *prefix, const struct object_id
> > *oid,
Cheers,
Alban
Re: [GSoC][PATCH v3 1/3] sequencer: add a new function to silence a command, except if it fails.
Hi Junio,
Le 22/06/2018 à 00:03, Junio C Hamano a écrit :
> Alban Gruin writes:
> > This adds a new function, run_command_silent_on_success(), to
> > redirect the stdout and stderr of a command to a strbuf, and then to run
> > that command. This strbuf is printed only if the command fails. It is
> > functionnaly similar to output() from git-rebase.sh.
> >
> > run_git_commit() is then refactored to use of
> > run_command_silent_on_success().
>
> It might be just a difference in viewpoint, but I think it is more
> customary in this project (hence it will be easier to understand and
> accept by readers of the patch) if a change like this is explained
> NOT as "introducing a new function and then rewrite an existing code
> to use it", and instead as "extract a helper function from an
> existing code so that future callers can reuse it."
>
I like your wording. It’s not a difference of point of view, I’m just bad at
writing commit messages ;)
> > +static int run_command_silent_on_success(struct child_process *cmd)
> > +{
> > + struct strbuf buf = STRBUF_INIT;
> > + int rc;
> > +
> > + cmd->stdout_to_stderr = 1;
> > + rc = pipe_command(cmd,
> > + NULL, 0,
> > + /* stdout is already redirected */
>
> I can see that this comment was inherited from the original place
> this code was lifted from (and that is why I say this is not "adding
> a new helper and rewriting an existing piece of code to use it" but
> is "extracting this function out of an existing code for future
> reuse"), but does it still make sense in the new context to keep the
> same comment?
>
> The original in run_git_commit() made the .stdout_to_stderr=1
> assignment many lines before it called pipe_command(), and it made
> sense to remind readers why we are passing NULL to the out buffer
> and only passing the err buffer here. But in the context of this
> new helper function, the redirection that may make such a reminder
> necessary sits immediately before the pipe_command() call for
> everybody to see.
>
Yeah, you’re right.
> > @@ -861,20 +872,8 @@ static int run_git_commit(const char *defmsg, struct
> > replay_opts *opts,>
> > if (opts->allow_empty_message)
> >
> > argv_array_push(, "--allow-empty-message");
> >
> > - if (cmd.err == -1) {
> > - /* hide stderr on success */
> > - struct strbuf buf = STRBUF_INIT;
> > - int rc = pipe_command(,
> > - NULL, 0,
> > - /* stdout is already redirected */
> > - NULL, 0,
> > - , 0);
> > - if (rc)
> > - fputs(buf.buf, stderr);
> > - strbuf_release();
> > - return rc;
> > - }
> > -
> > + if (is_rebase_i(opts) && !(flags & EDIT_MSG))
> > + return run_command_silent_on_success();
> >
> > return run_command();
> >
> > }
>
> It probably is easier to understand the code if you added
> an "else" after, to highlight the fact that this is choosing one out
> of two possible ways to run "cmd", i.e.
>
> if (is_rebase_i(opts) && !(flags & EDIT_MSG))
> return run_command_silent_on_success();
> else
> return run_command();
Okay.
Cheers,
Alban
Re: [PATCH] protocol-v2 doc: put HTTP headers after request
Jonathan Nieder wrote: > Josh Steadmon wrote: >> HTTP servers return 400 if you send headers before the GET request. [...] > Tested using > > openssl s_client -connect github.com:443 > > with input > > GET /git/git/info/refs?service=git-upload-pack HTTP/1.0 > Host: github.com > Git-Protocol: version=2 > > which produces a 404 instead of the 400 that putting Git-Protocol > in front would produce. I figured out how to produce a 200: printf '%s\r\n' \ 'GET /git/git/info/refs?service=git-upload-pack HTTP/1.0' \ 'Host: github.com' \ 'User-Agent: git/jrn-at-keyboard' \ 'Git-Protocol: version=2' '' | openssl s_client -connect github.com:443 -quiet The critical part is the User-Agent starting with git/. So we should probably update Documentation/technical/http-protocol.txt to indicate that clients MUST have a user-agent string starting with Git/ to allow the kind of request routing that github does. That's all orthogonal to this patch. The patch still looks good to me. Sincerely, Jonathan
Re: Unexpected ignorecase=false behavior on Windows
On 22.06.2018 19:36, Johannes Sixt wrote: Am 22.06.2018 um 14:04 schrieb Marc Strapetz: On Windows, when creating following repository: $ git init $ echo "1" > file.txt $ git add . $ git commit -m "initial import" $ ren file.txt File.txt $ git config core.ignorecase false This is a user error. core.ignorecase is *not* an instruction as in "hey, Git, do not ignore the case of file names". It is better regarded as an internal value, with which Git remembers how it should treat the names of files that it receives when it traverses the directories on the disk. Git could probe the file system capabilities each time it runs. But that would be wasteful. Hence, this probe happens only once when the repository is initialized, and the result is recorded in this configuration value. You should not change it. Sorry, it looks like my example was misleading. I'm actually questioning current behavior in case of Windows repositories with core.ignorecase initialized to false, like in following setup: $ git init $ git config core.ignorecase false The repository is now set up to be case-sensitive on Windows. From this point on, core.ignorecase won't change anymore and the repository will be filled: $ echo "1" > file.txt $ git add . $ git commit -m "initial import" $ ren file.txt File.txt Still, status results are: $ git status --porcelain ?? File.txt With the same setup sequence on Unix, it's: $ git status --porcelain D file.txt ?? File.txt Is this difference, which is depending on the platform, intended? Why not report missing file.txt as well? The drawback of the current behavior is that a subsequent "git add ." will result in two file names in the .git/index which are only differing in case. This will break the repository on Windows, because only one of both files can be checked out in the working tree. Also, it makes case-only renames harder to be performed. -Marc
Re: [PATCH 0/7] Restrict the usage of config_from_gitmodules()
On Fri, 22 Jun 2018 10:13:10 -0700 Brandon Williams wrote: [...] > Thanks for working on this. I think its a good approach and the end > result makes it much harder for arbitrary config to sneak back in to the > .gitmodules file. And after this series it looks like you should be in > a good place to read the .gitmodules file from other places (not just in > the worktree). > :) > As you've mentioned here I also agree we could do without the last patch > but I'll leave that up to you. Other than a couple typos I found I > think this series looks good! Thanks again for revisiting this. > Thanks for the review. I understand your compromise solution for patch 7, but I'd say let's keep it simple and just drop patch 7 for now. I am going to wait a couple of days and then send a v2. Ciao, Antonio -- Antonio Ospite https://ao2.it https://twitter.com/ao2it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?
Re: [PATCH] protocol-v2 doc: put HTTP headers after request
Josh Steadmon wrote: > HTTP servers return 400 if you send headers before the GET request. > > Signed-off-by: Josh Steadmon > --- > Documentation/technical/protocol-v2.txt | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Thanks. Congrats on your first patch! ;-) Tested using openssl s_client -connect github.com:443 with input GET /git/git/info/refs?service=git-upload-pack HTTP/1.0 Host: github.com Git-Protocol: version=2 which produces a 404 instead of the 400 that putting Git-Protocol in front would produce. Reviewed-by: Jonathan Nieder
[PATCH] protocol-v2 doc: put HTTP headers after request
HTTP servers return 400 if you send headers before the GET request. Signed-off-by: Josh Steadmon --- Documentation/technical/protocol-v2.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt index 49bda76d2..f58f24b1e 100644 --- a/Documentation/technical/protocol-v2.txt +++ b/Documentation/technical/protocol-v2.txt @@ -64,9 +64,8 @@ When using the http:// or https:// transport a client makes a "smart" info/refs request as described in `http-protocol.txt` and requests that v2 be used by supplying "version=2" in the `Git-Protocol` header. - C: Git-Protocol: version=2 - C: C: GET $GIT_URL/info/refs?service=git-upload-pack HTTP/1.0 + C: Git-Protocol: version=2 A v2 server would reply: -- 2.18.0.rc2.346.g013aa6912e-goog
Re: [PATCH 23/23] midx: clear midx on repack
On 6/9/2018 2:13 PM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 4:07 PM Derrick Stolee wrote:
If a 'git repack' command replaces existing packfiles, then we must
clear the existing multi-pack-index before moving the packfiles it
references.
I think there are other places where we add or remove pack files and
need to reprepare_packed_git(). Any midx invalidation should be part
of that as well.
The other places where we call reprepare_packed_git() are for when we
may have added a packfile, such as in fetch-pack.c, or sha1_file.c. The
other candidate to consider is 'git gc', but the packfile deletion is
handled by a call to 'git repack'.
Signed-off-by: Derrick Stolee
---
builtin/repack.c | 8
midx.c | 8
midx.h | 1 +
3 files changed, 17 insertions(+)
diff --git a/builtin/repack.c b/builtin/repack.c
index 6c636e159e..66a7d8e8ea 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -8,6 +8,7 @@
#include "strbuf.h"
#include "string-list.h"
#include "argv-array.h"
+#include "midx.h"
static int delta_base_offset = 1;
static int pack_kept_objects = -1;
@@ -174,6 +175,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
int no_update_server_info = 0;
int quiet = 0;
int local = 0;
+ int midx_cleared = 0;
struct option builtin_repack_options[] = {
OPT_BIT('a', NULL, _everything,
@@ -340,6 +342,12 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
continue;
}
+ if (!midx_cleared) {
+ /* if we move a packfile, it will invalidated
the midx */
What about removing packs, which also happens in repack? If the
removed pack is part of midx, then midx becomes invalid as well.
+ clear_midx_file(get_object_directory());
+ midx_cleared = 1;
+ }
+
fname_old = mkpathdup("%s/old-%s%s", packdir,
item->string, exts[ext].name);
if (file_exists(fname_old))
diff --git a/midx.c b/midx.c
index e46f392fa4..1043c01fa7 100644
--- a/midx.c
+++ b/midx.c
@@ -913,3 +913,11 @@ int write_midx_file(const char *object_dir)
FREE_AND_NULL(pack_names);
return 0;
}
+
+void clear_midx_file(const char *object_dir)
delete_ may be more obvious than clear_
+{
+ char *midx = get_midx_filename(object_dir);
+
+ if (remove_path(midx))
+ die(_("failed to clear multi-pack-index at %s"), midx);
die_errno()
+}
diff --git a/midx.h b/midx.h
index 6996b5ff6b..46f9f44c94 100644
--- a/midx.h
+++ b/midx.h
@@ -18,5 +18,6 @@ int midx_contains_pack(struct midxed_git *m, const char
*idx_name);
int prepare_midxed_git_one(struct repository *r, const char *object_dir);
int write_midx_file(const char *object_dir);
+void clear_midx_file(const char *object_dir);
#endif
--
2.18.0.rc1
Re: [PATCH 20/23] midx: use midx in approximate_object_count
On 6/9/2018 2:03 PM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 4:06 PM Derrick Stolee wrote:
Signed-off-by: Derrick Stolee
---
packfile.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/packfile.c b/packfile.c
index 638e113972..059b2aa097 100644
--- a/packfile.c
+++ b/packfile.c
@@ -819,11 +819,14 @@ unsigned long approximate_object_count(void)
{
if (!the_repository->objects->approximate_object_count_valid) {
unsigned long count;
+ struct midxed_git *m;
struct packed_git *p;
prepare_packed_git(the_repository);
count = 0;
- for (p = the_repository->objects->packed_git; p; p = p->next) {
+ for (m = get_midxed_git(the_repository); m; m = m->next)
+ count += m->num_objects;
+ for (p = get_packed_git(the_repository); p; p = p->next) {
Please don't change this line, it's not related to this patch.
Sure. I'll revert that line.
Same
concern applies, if we have already counted objects in midx we should
ignore packs that belong to it or we double count.
Since we do not put packfiles into the packed_git list if they are
tracked by the midx, we will not double count.
if (open_pack_index(p))
continue;
count += p->num_objects;
--
2.18.0.rc1
Re: [PATCH 18/23] midx: use midx in abbreviation calculations
On 6/9/2018 2:01 PM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 4:06 PM Derrick Stolee wrote:
@@ -565,8 +632,11 @@ static void find_abbrev_len_for_pack(struct packed_git *p,
static void find_abbrev_len_packed(struct min_abbrev_data *mad)
{
+ struct midxed_git *m;
struct packed_git *p;
+ for (m = get_midxed_git(the_repository); m; m = m->next)
+ find_abbrev_len_for_midx(m, mad);
If all the packs are in midx, we don't need to run the second loop
below, do we? Otherwise I don't see why we waste cycles on finding
abbrev length on midx at all.
We put all packs _at time of writing_ into the midx. More packs may be
added later that are not in the midx. There are tests in
t5319-multi-pack-index.sh that verify everything works in this "mixed mode".
It is important that the packfiles are not loaded into the packed_git
list if they are managed by the midx.
for (p = get_packed_git(the_repository); p; p = p->next)
find_abbrev_len_for_pack(p, mad);
}
Re: [PATCH 09/23] midx: write pack names in chunk
On 6/22/2018 2:31 PM, Junio C Hamano wrote: Derrick Stolee writes: The index extension documentation doesn't appear to be clear about which extensions are optional or required, but it seems the split-index is the only "required" one and uses lowercase for its extension id. read-cache.c:: /* Index extensions. * * The first letter should be 'A'..'Z' for extensions that are not * necessary for a correct operation (i.e. optimization data). * When new extensions are added that _needs_ to be understood in * order to correctly interpret the index file, pick character that * is outside the range, to cause the reader to abort. */ Thanks! I was reading Documentation/technical/index-format.txt and optional extensions are mentioned but not described precisely.
Re: [PATCH 09/23] midx: write pack names in chunk
Derrick Stolee writes: > The index extension documentation doesn't appear to be clear about > which extensions are optional or required, but it seems the > split-index is the only "required" one and uses lowercase for its > extension id. read-cache.c:: /* Index extensions. * * The first letter should be 'A'..'Z' for extensions that are not * necessary for a correct operation (i.e. optimization data). * When new extensions are added that _needs_ to be understood in * order to correctly interpret the index file, pick character that * is outside the range, to cause the reader to abort. */
Re: [PATCH 09/23] midx: write pack names in chunk
On 6/21/2018 1:38 PM, Junio C Hamano wrote:
Derrick Stolee writes:
On 6/7/2018 2:26 PM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 4:03 PM, Derrick Stolee wrote:
@@ -74,6 +80,31 @@ struct midxed_git *load_midxed_git(const char *object_dir)
m->num_chunks = *(m->data + 6);
m->num_packs = get_be32(m->data + 8);
+ for (i = 0; i < m->num_chunks; i++) {
+ uint32_t chunk_id = get_be32(m->data + 12 +
MIDX_CHUNKLOOKUP_WIDTH * i);
+ uint64_t chunk_offset = get_be64(m->data + 16 +
MIDX_CHUNKLOOKUP_WIDTH * i);
Would be good to reduce magic numbers like 12 and 16, I think you have
some header length constants for those already.
+ switch (chunk_id) {
+ case MIDX_CHUNKID_PACKNAMES:
+ m->chunk_pack_names = m->data + chunk_offset;
+ break;
(style: aren't these case arms indented one level too deep)?
+ case 0:
+ die("terminating MIDX chunk id appears earlier than
expected");
_()
This die() and others like it are not marked for translation on
purpose, as they should never be seen by an end-user.
Should never be seen because it indicates a software bug, in which
case this should be BUG() instead of die()?
Or did we just find a file corruption on the filesystem? If so,
then the error is end-user facing and should tell the user something
that hints what is going on in the language the user understands, I
would guess.
+ default:
+ /*
+* Do nothing on unrecognized chunks, allowing
future
+* extensions to add optional chunks.
+*/
I wrote about the chunk term reminding me of PNG format then deleted
it. But it may help to do similar to PNG here. The first letter can
let us know if the chunk is optional and can be safely ignored. E.g.
uppercase first letter cannot be ignored, lowercase go wild.
That's an interesting way to think about it. That way you could add a
new "required" chunk and earlier versions could die() realizing they
don't know how to parse that required chunk.
That is how the index extension sections work and may be a good
example to follow.
The index extension documentation doesn't appear to be clear about which
extensions are optional or required, but it seems the split-index is the
only "required" one and uses lowercase for its extension id.
Since the multi-pack-index has similar structure to the commit-graph
file, and that file includes an optional chunk with no special casing of
the chunk id, I think we should stick with the existing model: chunks
that are added later are optional and if Git _must_ understand it, then
we increment the version number. Hence, for each version number there is
a fixed list of required chunks, but an extendible list of optional chunks.
Thanks,
-Stolee
Re: Unexpected ignorecase=false behavior on Windows
Am 22.06.2018 um 14:04 schrieb Marc Strapetz: On Windows, when creating following repository: $ git init $ echo "1" > file.txt $ git add . $ git commit -m "initial import" $ ren file.txt File.txt $ git config core.ignorecase false This is a user error. core.ignorecase is *not* an instruction as in "hey, Git, do not ignore the case of file names". It is better regarded as an internal value, with which Git remembers how it should treat the names of files that it receives when it traverses the directories on the disk. Git could probe the file system capabilities each time it runs. But that would be wasteful. Hence, this probe happens only once when the repository is initialized, and the result is recorded in this configuration value. You should not change it. Status results are: $ git status --porcelain ?? File.txt As on Unix, I would expect to see: $ git status --porcelain D file.txt ?? File.txt Is the Windows behavior intended? I'm asking, because e.g. JGit will report missing files, too, and I'm wondering what would be the correct way to resolve this discrepancy? (JGit does not have "ignorecase=true"-support at all, btw). Then I don't think there is a way to remove the discrepancy. -- Hannes
Re: [PATCH 5/5] commit-graph: add repo arg to graph readers
> On 6/21/2018 7:06 PM, Jonathan Tan wrote:
> >>> diff --git a/commit.c b/commit.c
> >>> index 0030e79940..38c12b002f 100644
> >>> --- a/commit.c
> >>> +++ b/commit.c
> >>> @@ -317,7 +317,7 @@ struct tree *get_commit_tree(const struct commit
> >>> *commit)
> >>> if (commit->graph_pos == COMMIT_NOT_FROM_GRAPH)
> >>> BUG("commit has NULL tree, but was not loaded from
> >>> commit-graph");
> >>>
> >>> - return get_commit_tree_in_graph(commit);
> >>> + return get_commit_tree_in_graph(the_repository, commit);
> >> Here..
> >>
> >>> }
> >>>
> >>> struct object_id *get_commit_tree_oid(const struct commit *commit)
> >>> @@ -413,7 +413,7 @@ int parse_commit_gently(struct commit *item, int
> >>> quiet_on_missing)
> >>> return -1;
> >>> if (item->object.parsed)
> >>> return 0;
> >>> - if (parse_commit_in_graph(item))
> >>> + if (parse_commit_in_graph(the_repository, item))
> >> and here
> >>
> >>> +static void test_parse_commit_in_graph(const char *gitdir, const char
> >>> *worktree,
> >>> + const struct object_id *commit_oid)
> >>> +{
> >>> + struct repository r;
> >>> + struct commit *c;
> >>> + struct commit_list *parent;
> >>> +
> >>> + /*
> >>> +* Create a commit independent of any repository.
> >>> +*/
> >>> + c = lookup_commit(commit_oid);
> >> .. and this one are unfortunate as the rest of the object store series
> >> has not progressed as far as needed.
> > I think the first 2 are in reverse - get_commit_tree depends on
> > get_commit_tree_in_graph and parse_commit_gently depends on
> > parse_commit_in_graph, so we need the commit-graph functions to be
> > changed first. But I agree about lookup_commit.
> >
> >> The lookup_commit series is out there already, and that will
> >> teach lookup_commit a repository argument. When rerolling
> >> that series I need to switch the order of repo_init and lookup_commit
> >> such that we can pass the repo to the lookup.
> > For future reference, Stefan is talking about this series:
> > https://public-inbox.org/git/20180613230522.55335-1-sbel...@google.com/
> >
> > Let me know if you want to reroll yours on top of mine, or vice versa. I
> > think it's clearer if mine goes in first, though, since (as you said in
> > that e-mail) parse_commit depends on this change in the commit graph.
>
> I was about to comment that I thought 'parse_commit_in_graph' should
> take a 'struct commit_graph' instead of 'struct repository', except for
> these lookup_commit() calls will need the repository parameter.
Not sure what you mean by the lookup_commit() calls (if you're referring
to the part quoted above, that is test code), but
parse_commit_in_graph() has to take a struct repository (or at least a
struct object_store, perhaps) because it needs to load the commit graph
for the repository if it is not already loaded.
(An alternative, of course, is to require the user to explicitly load
the graph, but since parse_commit_in_graph() is called from
parse_commit(), I think that this implicit loading is fine.)
> Please also keep in mind that ds/commit-graph-fsck has already updated
> this method to parse from a specific graph [1]. I'm just waiting for
> some things like ds/generation-numbers to get into 'master' and some
> more object-store patches to be final before I re-roll that series. I
> mentioned this in a message that I had sent, but apparently didn't make
> it on the list (so I re-sent it recently).
>
> [1]
> https://public-inbox.org/git/20180608135548.216405-4-dsto...@microsoft.com/
Thanks - I see that you introduced a new
parse_commit_in_graph_one(struct commit_graph *, struct commit *) and
made the existing parse_commit_in_graph(struct commit *item) use the
former. Combining our changes would be just adding a repository argument
to parse_commit_in_graph() and passing the graph through
parse_commit_in_graph_one() (after prepare_commit_graph() and ensuring
that the graph exists).
Re: [PATCH 0/7] Restrict the usage of config_from_gitmodules()
On 06/22, Antonio Ospite wrote: > Hi, > > when I tried to reuse and extend 'config_from_gitmodules' in > https://public-inbox.org/git/20180514105823.8378-2-...@ao2.it/ it was > pointed out to me that special care is needed to make sure that this > function does not get abused to bring in arbitrary configuration stored > in the .gitmodules file, as the latter is meant only for submodule > specific configuration. > > So I thought that the function could be made private to better > communicate that. > > This is what this series is about. > > Patch 1 moves 'config_from_gitmodules' to submodule-config.c > > Patches 2 and 3 add helpers to handle special cases and avoid calling > 'config_from_gitmodules' directly, which might set a bad example for > future code. > > Patch 4 makes the symbol private to discourage its use in code not > related to submodules. > > Patches 5 and 6 enable reusing 'config_from_gitmodules' when it's safe > to do so. > > Patches 7 is just a cleanup and I am not even sure it is worth it, so we > might as well just drop it. > > The series can be seen as a continuation of the changes from > https://public-inbox.org/git/20170802194923.88239-1-bmw...@google.com/ > > Even though the helper functions may be less elegant than what was done > back then, they should better protect from misuse of > config_from_gitmodules. > > A further change could be to print warning messages when the backward > compatibility helpers find configuration in .gitmodules that should not > belong there, but I'll leave that to someone else. > > Thanks, >Antonio > > P.S. I added Jeff King to CC as he has done some work related to > .gitmodules recently, and I removed the vcsh poeple on this one. > Thanks for working on this. I think its a good approach and the end result makes it much harder for arbitrary config to sneak back in to the .gitmodules file. And after this series it looks like you should be in a good place to read the .gitmodules file from other places (not just in the worktree). As you've mentioned here I also agree we could do without the last patch but I'll leave that up to you. Other than a couple typos I found I think this series looks good! Thanks again for revisiting this. -- Brandon Williams
Re: The state of the object store series
> I'm waiting for sb/object-store-lookup to be in 'next' before I re-roll
> that branch. If you're not in a rush to send this series, perhaps wait
> for the next version here.
Did you mean another branch? I'm looking at
https://github.com/gitster/git right now, and I see
sb/object-store{,-alloc,-grafts,-replace} but not
sb/object-store-lookup.
(I'm asking this because I'm trying to wrap my head around the
interconnectedness of all our patches.)
Re: [PATCH 7/7] submodule-config: cleanup backward compatibility helpers
On 06/22, Antonio Ospite wrote:
> Use one callback per configuration setting to handle the generic options
> which have to be supported for backward compatibility.
>
> This removes some duplication and some support code at the cost of
> parsing the .gitmodules file twice when calling the fetch command.
>
> Signed-off-by: Antonio Ospite
I'm not sure how I feel about this patch, I'm leaning towards not
needing it but I like the idea of eliminating the duplicate code. One
way you could get rid of having to read the .gitmodules file twice would
be something like the following but I don't really think its needed:
diff --git a/submodule-config.c b/submodule-config.c
index ce204fb53..7cc1864b5 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -681,19 +681,24 @@ void submodule_free(struct repository *r)
submodule_cache_clear(r->submodule_cache);
}
-struct fetch_config {
- int *max_children;
+struct fetch_clone_config {
+ int *fetchjobs;
int *recurse_submodules;
};
-static int gitmodules_fetch_config(const char *var, const char *value, void
*cb)
+static int gitmodules_fetch_clone_config(const char *var, const char *value,
+void *cb)
{
- struct fetch_config *config = cb;
+ struct fetch_clone_config *config = cb;
if (!strcmp(var, "submodule.fetchjobs")) {
- *(config->max_children) = parse_submodule_fetchjobs(var, value);
+ if (config->fetchjobs)
+ *(config->fetchjobs) =
+ parse_submodule_fetchjobs(var, value);
return 0;
} else if (!strcmp(var, "fetch.recursesubmodules")) {
- *(config ->recurse_submodules) =
parse_fetch_recurse_submodules_arg(var, value);
+ if (config->recurse_submodules)
+ *(config->recurse_submodules) =
+ parse_fetch_recurse_submodules_arg(var, value);
return 0;
}
@@ -702,23 +707,20 @@ static int gitmodules_fetch_config(const char *var, const
char *value, void *cb)
void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
{
- struct fetch_config config = {
- .max_children = max_children,
- .recurse_submodules = recurse_submodules
+ struct fetch_clone_config config = {
+ .fetchjobs = max_children,
+ .recurse_submodules = recurse_submodules,
};
- config_from_gitmodules(gitmodules_fetch_config, the_repository,
);
-}
-
-static int gitmodules_update_clone_config(const char *var, const char *value,
- void *cb)
-{
- int *max_jobs = cb;
- if (!strcmp(var, "submodule.fetchjobs"))
- *max_jobs = parse_submodule_fetchjobs(var, value);
- return 0;
+ config_from_gitmodules(gitmodules_fetch_clone_config, the_repository,
+ );
}
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository,
_jobs);
+ struct fetch_clone_config config = {
+ .fetchjobs = max_jobs,
+ .recurse_submodules = NULL,
+ };
+ config_from_gitmodules(gitmodules_fetch_clone_config, the_repository,
+ );
}
--
Brandon Williams
Re: [PATCH 6/7] submodule-config: reuse config_from_gitmodules in repo_read_gitmodules
On 06/22, Antonio Ospite wrote:
> Reuse config_from_gitmodules in repo_read_gitmodules to remove some
> duplication and also have a single point where the .gitmodules file is
> read.
>
> The change does not introduce any new behavior, the same gitmodules_cb
> config callback is still used which only deals with configuration
> specific to submodules.
>
> The config_from_gitmodules function is moved up in the file —unchanged—
> before its users to avoid a forward declaration.
>
> Signed-off-by: Antonio Ospite
> ---
> submodule-config.c | 50 +++---
> 1 file changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/submodule-config.c b/submodule-config.c
> index e50c944eb..ce204fb53 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -591,6 +591,23 @@ static void submodule_cache_check_init(struct repository
> *repo)
> submodule_cache_init(repo->submodule_cache);
> }
>
> +/*
> + * Note: This function is private for a reason, the '.gitmodules' file should
> + * not be used as as a mechanism to retrieve arbitrary configuration stored
> in
> + * the repository.
> + *
> + * Runs the provided config function on the '.gitmodules' file found in the
> + * working directory.
> + */
> +static void config_from_gitmodules(config_fn_t fn, struct repository *repo,
> void *data)
> +{
> + if (repo->worktree) {
> + char *file = repo_worktree_path(repo, GITMODULES_FILE);
> + git_config_from_file(fn, file, data);
> + free(file);
> + }
> +}
> +
> static int gitmodules_cb(const char *var, const char *value, void *data)
> {
> struct repository *repo = data;
> @@ -608,19 +625,11 @@ void repo_read_gitmodules(struct repository *repo)
> {
> submodule_cache_check_init(repo);
>
> - if (repo->worktree) {
> - char *gitmodules;
> -
> - if (repo_read_index(repo) < 0)
> - return;
> -
> - gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
> -
> - if (!is_gitmodules_unmerged(repo->index))
> - git_config_from_file(gitmodules_cb, gitmodules, repo);
> + if (repo_read_index(repo) < 0)
> + return;
>
> - free(gitmodules);
> - }
> + if (!is_gitmodules_unmerged(repo->index))
> + config_from_gitmodules(gitmodules_cb, repo, repo);
So the check for the repo's worktree has been pushed into
config_from_gitmodules(). This looks like the right thing to do in
order to get to a world where you'd rather read the gitmodules file from
the index instead of the worktree.
>
> repo->submodule_cache->gitmodules_read = 1;
> }
> @@ -672,23 +681,6 @@ void submodule_free(struct repository *r)
> submodule_cache_clear(r->submodule_cache);
> }
>
> -/*
> - * Note: This function is private for a reason, the '.gitmodules' file should
> - * not be used as as a mechanism to retrieve arbitrary configuration stored
> in
> - * the repository.
> - *
> - * Runs the provided config function on the '.gitmodules' file found in the
> - * working directory.
> - */
> -static void config_from_gitmodules(config_fn_t fn, struct repository *repo,
> void *data)
> -{
> - if (repo->worktree) {
> - char *file = repo_worktree_path(repo, GITMODULES_FILE);
> - git_config_from_file(fn, file, data);
> - free(file);
> - }
> -}
> -
> struct fetch_config {
> int *max_children;
> int *recurse_submodules;
> --
> 2.18.0
>
--
Brandon Williams
Re: [PATCH 3/7] submodule-config: add helper to get 'update-clone' config from .gitmodules
On 06/22, Antonio Ospite wrote: > Add a helper function to make it clearer that retrieving 'update-clone' > configuration from the .gitmodules file is a special case supported > solely for backward compatibility purposes. > > This change removes one direct use of 'config_from_gitmodules' for > options not strictly related to submodules: "submodule.fetchobjs" does s/fetchobjs/fetchjobs -- Brandon Williams
Re: [PATCH 2/7] submodule-config: add helper function to get 'fetch' config from .gitmodules
On 06/22, Antonio Ospite wrote:
> diff --git a/submodule-config.c b/submodule-config.c
> index b431555db..ef292eb7c 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -688,3 +688,31 @@ void config_from_gitmodules(config_fn_t fn, void *data)
> free(file);
> }
> }
> +
> +struct fetch_config {
> + int *max_children;
> + int *recurse_submodules;
> +};
> +
> +static int gitmodules_fetch_config(const char *var, const char *value, void
> *cb)
> +{
> + struct fetch_config *config = cb;
> + if (!strcmp(var, "submodule.fetchjobs")) {
> + *(config->max_children) = parse_submodule_fetchjobs(var, value);
> + return 0;
> + } else if (!strcmp(var, "fetch.recursesubmodules")) {
> + *(config ->recurse_submodules) =
> parse_fetch_recurse_submodules_arg(var, value);
There shouldn't be a space before "->" in this line.
--
Brandon Williams
[PATCH 6/7] submodule-config: reuse config_from_gitmodules in repo_read_gitmodules
Reuse config_from_gitmodules in repo_read_gitmodules to remove some
duplication and also have a single point where the .gitmodules file is
read.
The change does not introduce any new behavior, the same gitmodules_cb
config callback is still used which only deals with configuration
specific to submodules.
The config_from_gitmodules function is moved up in the file —unchanged—
before its users to avoid a forward declaration.
Signed-off-by: Antonio Ospite
---
submodule-config.c | 50 +++---
1 file changed, 21 insertions(+), 29 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index e50c944eb..ce204fb53 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -591,6 +591,23 @@ static void submodule_cache_check_init(struct repository
*repo)
submodule_cache_init(repo->submodule_cache);
}
+/*
+ * Note: This function is private for a reason, the '.gitmodules' file should
+ * not be used as as a mechanism to retrieve arbitrary configuration stored in
+ * the repository.
+ *
+ * Runs the provided config function on the '.gitmodules' file found in the
+ * working directory.
+ */
+static void config_from_gitmodules(config_fn_t fn, struct repository *repo,
void *data)
+{
+ if (repo->worktree) {
+ char *file = repo_worktree_path(repo, GITMODULES_FILE);
+ git_config_from_file(fn, file, data);
+ free(file);
+ }
+}
+
static int gitmodules_cb(const char *var, const char *value, void *data)
{
struct repository *repo = data;
@@ -608,19 +625,11 @@ void repo_read_gitmodules(struct repository *repo)
{
submodule_cache_check_init(repo);
- if (repo->worktree) {
- char *gitmodules;
-
- if (repo_read_index(repo) < 0)
- return;
-
- gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
-
- if (!is_gitmodules_unmerged(repo->index))
- git_config_from_file(gitmodules_cb, gitmodules, repo);
+ if (repo_read_index(repo) < 0)
+ return;
- free(gitmodules);
- }
+ if (!is_gitmodules_unmerged(repo->index))
+ config_from_gitmodules(gitmodules_cb, repo, repo);
repo->submodule_cache->gitmodules_read = 1;
}
@@ -672,23 +681,6 @@ void submodule_free(struct repository *r)
submodule_cache_clear(r->submodule_cache);
}
-/*
- * Note: This function is private for a reason, the '.gitmodules' file should
- * not be used as as a mechanism to retrieve arbitrary configuration stored in
- * the repository.
- *
- * Runs the provided config function on the '.gitmodules' file found in the
- * working directory.
- */
-static void config_from_gitmodules(config_fn_t fn, struct repository *repo,
void *data)
-{
- if (repo->worktree) {
- char *file = repo_worktree_path(repo, GITMODULES_FILE);
- git_config_from_file(fn, file, data);
- free(file);
- }
-}
-
struct fetch_config {
int *max_children;
int *recurse_submodules;
--
2.18.0
[PATCH 3/7] submodule-config: add helper to get 'update-clone' config from .gitmodules
Add a helper function to make it clearer that retrieving 'update-clone'
configuration from the .gitmodules file is a special case supported
solely for backward compatibility purposes.
This change removes one direct use of 'config_from_gitmodules' for
options not strictly related to submodules: "submodule.fetchobjs" does
not describe a property of a submodule, but a behavior of other commands
when dealing with submodules, so it does not really belong to the
.gitmodules file.
This is in the effort to communicate better that .gitmodules is not to
be used as a mechanism to store arbitrary configuration in the
repository that any command can retrieve.
Signed-off-by: Antonio Ospite
---
builtin/submodule--helper.c | 8
submodule-config.c | 14 ++
submodule-config.h | 1 +
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index bd250ca21..d450b81da 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1562,8 +1562,8 @@ static int update_clone_task_finished(int result,
return 0;
}
-static int gitmodules_update_clone_config(const char *var, const char *value,
- void *cb)
+static int git_update_clone_config(const char *var, const char *value,
+ void *cb)
{
int *max_jobs = cb;
if (!strcmp(var, "submodule.fetchjobs"))
@@ -1613,8 +1613,8 @@ static int update_clone(int argc, const char **argv,
const char *prefix)
};
suc.prefix = prefix;
- config_from_gitmodules(gitmodules_update_clone_config, _jobs);
- git_config(gitmodules_update_clone_config, _jobs);
+ update_clone_config_from_gitmodules(_jobs);
+ git_config(git_update_clone_config, _jobs);
argc = parse_options(argc, argv, prefix, module_update_clone_options,
git_submodule_helper_usage, 0);
diff --git a/submodule-config.c b/submodule-config.c
index ef292eb7c..46fb454ae 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -716,3 +716,17 @@ void fetch_config_from_gitmodules(int *max_children, int
*recurse_submodules)
};
config_from_gitmodules(gitmodules_fetch_config, );
}
+
+static int gitmodules_update_clone_config(const char *var, const char *value,
+ void *cb)
+{
+ int *max_jobs = cb;
+ if (!strcmp(var, "submodule.fetchjobs"))
+ *max_jobs = parse_submodule_fetchjobs(var, value);
+ return 0;
+}
+
+void update_clone_config_from_gitmodules(int *max_jobs)
+{
+ config_from_gitmodules(gitmodules_update_clone_config, _jobs);
+}
diff --git a/submodule-config.h b/submodule-config.h
index cff297a75..b6f19d0d4 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -67,5 +67,6 @@ int check_submodule_name(const char *name);
extern void config_from_gitmodules(config_fn_t fn, void *data);
extern void fetch_config_from_gitmodules(int *max_children, int
*recurse_submodules);
+extern void update_clone_config_from_gitmodules(int *max_jobs);
#endif /* SUBMODULE_CONFIG_H */
--
2.18.0
[PATCH 0/7] Restrict the usage of config_from_gitmodules()
Hi, when I tried to reuse and extend 'config_from_gitmodules' in https://public-inbox.org/git/20180514105823.8378-2-...@ao2.it/ it was pointed out to me that special care is needed to make sure that this function does not get abused to bring in arbitrary configuration stored in the .gitmodules file, as the latter is meant only for submodule specific configuration. So I thought that the function could be made private to better communicate that. This is what this series is about. Patch 1 moves 'config_from_gitmodules' to submodule-config.c Patches 2 and 3 add helpers to handle special cases and avoid calling 'config_from_gitmodules' directly, which might set a bad example for future code. Patch 4 makes the symbol private to discourage its use in code not related to submodules. Patches 5 and 6 enable reusing 'config_from_gitmodules' when it's safe to do so. Patches 7 is just a cleanup and I am not even sure it is worth it, so we might as well just drop it. The series can be seen as a continuation of the changes from https://public-inbox.org/git/20170802194923.88239-1-bmw...@google.com/ Even though the helper functions may be less elegant than what was done back then, they should better protect from misuse of config_from_gitmodules. A further change could be to print warning messages when the backward compatibility helpers find configuration in .gitmodules that should not belong there, but I'll leave that to someone else. Thanks, Antonio P.S. I added Jeff King to CC as he has done some work related to .gitmodules recently, and I removed the vcsh poeple on this one. Antonio Ospite (7): config: move config_from_gitmodules to submodule-config.c submodule-config: add helper function to get 'fetch' config from .gitmodules submodule-config: add helper to get 'update-clone' config from .gitmodules submodule-config: make 'config_from_gitmodules' private submodule-config: pass repository as argument to config_from_gitmodules submodule-config: reuse config_from_gitmodules in repo_read_gitmodules submodule-config: cleanup backward compatibility helpers builtin/fetch.c | 15 + builtin/submodule--helper.c | 8 ++--- config.c| 17 -- config.h| 10 -- submodule-config.c | 66 ++--- submodule-config.h | 12 +++ 6 files changed, 71 insertions(+), 57 deletions(-) -- Antonio Ospite https://ao2.it https://twitter.com/ao2it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?
[PATCH 1/7] config: move config_from_gitmodules to submodule-config.c
The .gitmodules file is not meant as a place to store arbitrary
configuration to distribute with the repository.
Move config_from_gitmodules() out of config.c and into
submodule-config.c to make it even clearer that it is not a mechanism to
retrieve arbitrary configuration from the .gitmodules file.
Signed-off-by: Antonio Ospite
---
config.c | 17 -
config.h | 10 --
submodule-config.c | 17 +
submodule-config.h | 11 +++
4 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/config.c b/config.c
index fbbf0f8e9..2e4dbfa19 100644
--- a/config.c
+++ b/config.c
@@ -2172,23 +2172,6 @@ int git_config_get_pathname(const char *key, const char
**dest)
return repo_config_get_pathname(the_repository, key, dest);
}
-/*
- * Note: This function exists solely to maintain backward compatibility with
- * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
- * NOT be used anywhere else.
- *
- * Runs the provided config function on the '.gitmodules' file found in the
- * working directory.
- */
-void config_from_gitmodules(config_fn_t fn, void *data)
-{
- if (the_repository->worktree) {
- char *file = repo_worktree_path(the_repository,
GITMODULES_FILE);
- git_config_from_file(fn, file, data);
- free(file);
- }
-}
-
int git_config_get_expiry(const char *key, const char **output)
{
int ret = git_config_get_string_const(key, output);
diff --git a/config.h b/config.h
index cdac2fc73..3faf4fba9 100644
--- a/config.h
+++ b/config.h
@@ -215,16 +215,6 @@ extern int repo_config_get_maybe_bool(struct repository
*repo,
extern int repo_config_get_pathname(struct repository *repo,
const char *key, const char **dest);
-/*
- * Note: This function exists solely to maintain backward compatibility with
- * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
- * NOT be used anywhere else.
- *
- * Runs the provided config function on the '.gitmodules' file found in the
- * working directory.
- */
-extern void config_from_gitmodules(config_fn_t fn, void *data);
-
extern int git_config_get_value(const char *key, const char **value);
extern const struct string_list *git_config_get_value_multi(const char *key);
extern void git_config_clear(void);
diff --git a/submodule-config.c b/submodule-config.c
index 388ef1f89..b431555db 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -671,3 +671,20 @@ void submodule_free(struct repository *r)
if (r->submodule_cache)
submodule_cache_clear(r->submodule_cache);
}
+
+/*
+ * Note: This function exists solely to maintain backward compatibility with
+ * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
+ * NOT be used anywhere else.
+ *
+ * Runs the provided config function on the '.gitmodules' file found in the
+ * working directory.
+ */
+void config_from_gitmodules(config_fn_t fn, void *data)
+{
+ if (the_repository->worktree) {
+ char *file = repo_worktree_path(the_repository,
GITMODULES_FILE);
+ git_config_from_file(fn, file, data);
+ free(file);
+ }
+}
diff --git a/submodule-config.h b/submodule-config.h
index ca1f94e2d..5148801f4 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -2,6 +2,7 @@
#define SUBMODULE_CONFIG_CACHE_H
#include "cache.h"
+#include "config.h"
#include "hashmap.h"
#include "submodule.h"
#include "strbuf.h"
@@ -55,4 +56,14 @@ void submodule_free(struct repository *r);
*/
int check_submodule_name(const char *name);
+/*
+ * Note: This function exists solely to maintain backward compatibility with
+ * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
+ * NOT be used anywhere else.
+ *
+ * Runs the provided config function on the '.gitmodules' file found in the
+ * working directory.
+ */
+extern void config_from_gitmodules(config_fn_t fn, void *data);
+
#endif /* SUBMODULE_CONFIG_H */
--
2.18.0
[PATCH 4/7] submodule-config: make 'config_from_gitmodules' private
Now that 'config_from_gitmodules' is not used in the open, it can be
marked as private.
Hopefully this will prevent its usage for retrieving arbitrary
configuration form the '.gitmodules' file.
Signed-off-by: Antonio Ospite
---
submodule-config.c | 8
submodule-config.h | 12 +---
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 46fb454ae..6a9f4b6d1 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -673,14 +673,14 @@ void submodule_free(struct repository *r)
}
/*
- * Note: This function exists solely to maintain backward compatibility with
- * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
- * NOT be used anywhere else.
+ * Note: This function is private for a reason, the '.gitmodules' file should
+ * not be used as as a mechanism to retrieve arbitrary configuration stored in
+ * the repository.
*
* Runs the provided config function on the '.gitmodules' file found in the
* working directory.
*/
-void config_from_gitmodules(config_fn_t fn, void *data)
+static void config_from_gitmodules(config_fn_t fn, void *data)
{
if (the_repository->worktree) {
char *file = repo_worktree_path(the_repository,
GITMODULES_FILE);
diff --git a/submodule-config.h b/submodule-config.h
index b6f19d0d4..dc7278eea 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -57,15 +57,13 @@ void submodule_free(struct repository *r);
int check_submodule_name(const char *name);
/*
- * Note: This function exists solely to maintain backward compatibility with
- * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
- * NOT be used anywhere else.
+ * Note: these helper functions exist solely to maintain backward
+ * compatibility with 'fetch' and 'update_clone' storing configuration in
+ * '.gitmodules'.
*
- * Runs the provided config function on the '.gitmodules' file found in the
- * working directory.
+ * New helpers to retrieve arbitrary configuration from the '.gitmodules' file
+ * should NOT be added.
*/
-extern void config_from_gitmodules(config_fn_t fn, void *data);
-
extern void fetch_config_from_gitmodules(int *max_children, int
*recurse_submodules);
extern void update_clone_config_from_gitmodules(int *max_jobs);
--
2.18.0
Re: [GSoC][PATCH v3 2/3] rebase -i: rewrite setup_reflog_action() in C
Alban Gruin writes:
> This rewrites (the misnamed) setup_reflog_action() from shell to C. The
> new version is called checkout_base_commit().
;-) on the "misnamed" part. Indeed, setting up the comment for the
reflog entry is secondary to what this function wants to do, which
is to check out the branch to be rebased.
I do not think "base_commit" is a good name, either, though. When I
hear 'base' in the context of 'rebase', I would imagine that the
speaker is talking about the bottom of the range of the commits to
be rebased (i.e. "rebase --onto ONTO BASE BRANCH", which replays
commits BASE..BRANCH on top of ONTO and then points BRANCH at the
result), not the top of the range or the branch these commits are
taken from.
> index 51c8ab7ac..27f8453fe 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3129,6 +3129,36 @@ static const char *reflog_message(struct replay_opts
> *opts,
> return buf.buf;
> }
>
> +static int run_git_checkout(struct replay_opts *opts, const char *commit,
> + int verbose, const char *action)
> +{
> + struct child_process cmd = CHILD_PROCESS_INIT;
> +
> + cmd.git_cmd = 1;
> +
> + argv_array_push(, "checkout");
> + argv_array_push(, commit);
> + argv_array_pushf(_array, GIT_REFLOG_ACTION "=%s", action);
> +
> + if (verbose)
> + return run_command();
> + return run_command_silent_on_success();
For the same reason as 1/3, I think it makes more sense to have
"else" here.
> +int checkout_base_commit(struct replay_opts *opts, const char *commit,
> + int verbose)
> +{
> + const char *action;
> +
> + if (commit && *commit) {
Hmm, isn't it a programming error to feed !commit or !*commit here?
I offhand do not think of a reason why making such an input a silent
no-op success, instead of making it error out, would be a good idea.
> + action = reflog_message(opts, "start", "checkout %s", commit);
> + if (run_git_checkout(opts, commit, verbose, action))
> + return error(_("Could not checkout %s"), commit);
> + }
> +
> + return 0;
> +}
> +
> static const char rescheduled_advice[] =
> N_("Could not execute the todo command\n"
> "\n"
> diff --git a/sequencer.h b/sequencer.h
> index 35730b13e..42c3dda81 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -100,6 +100,9 @@ int update_head_with_reflog(const struct commit *old_head,
> void commit_post_rewrite(const struct commit *current_head,
>const struct object_id *new_head);
>
> +int checkout_base_commit(struct replay_opts *opts, const char *commit,
> + int verbose);
> +
> #define SUMMARY_INITIAL_COMMIT (1 << 0)
> #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
> void print_commit_summary(const char *prefix, const struct object_id *oid,
[PATCH 2/7] submodule-config: add helper function to get 'fetch' config from .gitmodules
Add a helper function to make it clearer that retrieving 'fetch'
configuration from the .gitmodules file is a special case supported
solely for backward compatibility purposes.
This change removes one direct use of 'config_from_gitmodules' in code
not strictly related to submodules, in the effort to communicate better
that .gitmodules is not to be used as a mechanism to store arbitrary
configuration in the repository that any command can retrieve.
Signed-off-by: Antonio Ospite
---
builtin/fetch.c| 15 +--
submodule-config.c | 28
submodule-config.h | 2 ++
3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ea5b9669a..92a5d235d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -93,19 +93,6 @@ static int git_fetch_config(const char *k, const char *v,
void *cb)
return git_default_config(k, v, cb);
}
-static int gitmodules_fetch_config(const char *var, const char *value, void
*cb)
-{
- if (!strcmp(var, "submodule.fetchjobs")) {
- max_children = parse_submodule_fetchjobs(var, value);
- return 0;
- } else if (!strcmp(var, "fetch.recursesubmodules")) {
- recurse_submodules = parse_fetch_recurse_submodules_arg(var,
value);
- return 0;
- }
-
- return 0;
-}
-
static int parse_refmap_arg(const struct option *opt, const char *arg, int
unset)
{
/*
@@ -1433,7 +1420,7 @@ int cmd_fetch(int argc, const char **argv, const char
*prefix)
for (i = 1; i < argc; i++)
strbuf_addf(_rla, " %s", argv[i]);
- config_from_gitmodules(gitmodules_fetch_config, NULL);
+ fetch_config_from_gitmodules(_children, _submodules);
git_config(git_fetch_config, NULL);
argc = parse_options(argc, argv, prefix,
diff --git a/submodule-config.c b/submodule-config.c
index b431555db..ef292eb7c 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -688,3 +688,31 @@ void config_from_gitmodules(config_fn_t fn, void *data)
free(file);
}
}
+
+struct fetch_config {
+ int *max_children;
+ int *recurse_submodules;
+};
+
+static int gitmodules_fetch_config(const char *var, const char *value, void
*cb)
+{
+ struct fetch_config *config = cb;
+ if (!strcmp(var, "submodule.fetchjobs")) {
+ *(config->max_children) = parse_submodule_fetchjobs(var, value);
+ return 0;
+ } else if (!strcmp(var, "fetch.recursesubmodules")) {
+ *(config ->recurse_submodules) =
parse_fetch_recurse_submodules_arg(var, value);
+ return 0;
+ }
+
+ return 0;
+}
+
+void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
+{
+ struct fetch_config config = {
+ .max_children = max_children,
+ .recurse_submodules = recurse_submodules
+ };
+ config_from_gitmodules(gitmodules_fetch_config, );
+}
diff --git a/submodule-config.h b/submodule-config.h
index 5148801f4..cff297a75 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -66,4 +66,6 @@ int check_submodule_name(const char *name);
*/
extern void config_from_gitmodules(config_fn_t fn, void *data);
+extern void fetch_config_from_gitmodules(int *max_children, int
*recurse_submodules);
+
#endif /* SUBMODULE_CONFIG_H */
--
2.18.0
[PATCH 5/7] submodule-config: pass repository as argument to config_from_gitmodules
Generlize config_from_gitmodules to accept a repository as an argument.
This is in preparation to reuse the function in repo_read_gitmodules in
order to have a single point where the '.gitmodules' file is accessed.
Signed-off-by: Antonio Ospite
---
submodule-config.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 6a9f4b6d1..e50c944eb 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -680,10 +680,10 @@ void submodule_free(struct repository *r)
* Runs the provided config function on the '.gitmodules' file found in the
* working directory.
*/
-static void config_from_gitmodules(config_fn_t fn, void *data)
+static void config_from_gitmodules(config_fn_t fn, struct repository *repo,
void *data)
{
- if (the_repository->worktree) {
- char *file = repo_worktree_path(the_repository,
GITMODULES_FILE);
+ if (repo->worktree) {
+ char *file = repo_worktree_path(repo, GITMODULES_FILE);
git_config_from_file(fn, file, data);
free(file);
}
@@ -714,7 +714,7 @@ void fetch_config_from_gitmodules(int *max_children, int
*recurse_submodules)
.max_children = max_children,
.recurse_submodules = recurse_submodules
};
- config_from_gitmodules(gitmodules_fetch_config, );
+ config_from_gitmodules(gitmodules_fetch_config, the_repository,
);
}
static int gitmodules_update_clone_config(const char *var, const char *value,
@@ -728,5 +728,5 @@ static int gitmodules_update_clone_config(const char *var,
const char *value,
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, _jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, the_repository,
_jobs);
}
--
2.18.0
[PATCH 7/7] submodule-config: cleanup backward compatibility helpers
Use one callback per configuration setting to handle the generic options
which have to be supported for backward compatibility.
This removes some duplication and some support code at the cost of
parsing the .gitmodules file twice when calling the fetch command.
Signed-off-by: Antonio Ospite
---
submodule-config.c | 39 +++
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index ce204fb53..0a5274891 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -681,36 +681,20 @@ void submodule_free(struct repository *r)
submodule_cache_clear(r->submodule_cache);
}
-struct fetch_config {
- int *max_children;
- int *recurse_submodules;
-};
-
-static int gitmodules_fetch_config(const char *var, const char *value, void
*cb)
+static int gitmodules_recurse_submodules_config(const char *var,
+ const char *value, void *cb)
{
- struct fetch_config *config = cb;
- if (!strcmp(var, "submodule.fetchjobs")) {
- *(config->max_children) = parse_submodule_fetchjobs(var, value);
- return 0;
- } else if (!strcmp(var, "fetch.recursesubmodules")) {
- *(config ->recurse_submodules) =
parse_fetch_recurse_submodules_arg(var, value);
+ int *recurse_submodules = cb;
+ if (!strcmp(var, "fetch.recursesubmodules")) {
+ *recurse_submodules = parse_fetch_recurse_submodules_arg(var,
value);
return 0;
}
return 0;
}
-void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
-{
- struct fetch_config config = {
- .max_children = max_children,
- .recurse_submodules = recurse_submodules
- };
- config_from_gitmodules(gitmodules_fetch_config, the_repository,
);
-}
-
-static int gitmodules_update_clone_config(const char *var, const char *value,
- void *cb)
+static int gitmodules_fetchobjs_config(const char *var, const char *value,
+ void *cb)
{
int *max_jobs = cb;
if (!strcmp(var, "submodule.fetchjobs"))
@@ -718,7 +702,14 @@ static int gitmodules_update_clone_config(const char *var,
const char *value,
return 0;
}
+
+void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
+{
+ config_from_gitmodules(gitmodules_fetchobjs_config, the_repository,
_children);
+ config_from_gitmodules(gitmodules_recurse_submodules_config,
the_repository, _submodules);
+}
+
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository,
_jobs);
+ config_from_gitmodules(gitmodules_fetchobjs_config, the_repository,
_jobs);
}
--
2.18.0
[PATCH v3 6/7] grep.c: add configuration variables to show matched option
To support git-grep(1)'s new option, '--column', document and teach
grep.c how to interpret relevant configuration options, similar to those
associated with '--line-number'.
Signed-off-by: Taylor Blau
---
Documentation/config.txt | 5 +
Documentation/git-grep.txt | 3 +++
grep.c | 6 ++
3 files changed, 14 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 58fde4daea..e4cbed3078 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1183,6 +1183,8 @@ color.grep.::
function name lines (when using `-p`)
`lineNumber`;;
line number prefix (when using `-n`)
+`column`;;
+ column number prefix (when using `--column`)
`match`;;
matching text (same as setting `matchContext` and `matchSelected`)
`matchContext`;;
@@ -1797,6 +1799,9 @@ gitweb.snapshot::
grep.lineNumber::
If set to true, enable `-n` option by default.
+grep.column::
+ If set to true, enable the `--column` option by default.
+
grep.patternType::
Set the default matching behavior. Using a value of 'basic', 'extended',
'fixed', or 'perl' will enable the `--basic-regexp`,
`--extended-regexp`,
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 31dc0392a6..0de3493b80 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -44,6 +44,9 @@ CONFIGURATION
grep.lineNumber::
If set to true, enable `-n` option by default.
+grep.column::
+ If set to true, enable the `--column` option by default.
+
grep.patternType::
Set the default matching behavior. Using a value of 'basic', 'extended',
'fixed', or 'perl' will enable the `--basic-regexp`,
`--extended-regexp`,
diff --git a/grep.c b/grep.c
index 83fe32a6a0..992673fe7e 100644
--- a/grep.c
+++ b/grep.c
@@ -96,6 +96,10 @@ int grep_config(const char *var, const char *value, void *cb)
opt->linenum = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "grep.column")) {
+ opt->columnnum = git_config_bool(var, value);
+ return 0;
+ }
if (!strcmp(var, "grep.fullname")) {
opt->relative = !git_config_bool(var, value);
@@ -112,6 +116,8 @@ int grep_config(const char *var, const char *value, void
*cb)
color = opt->color_function;
else if (!strcmp(var, "color.grep.linenumber"))
color = opt->color_lineno;
+ else if (!strcmp(var, "color.grep.column"))
+ color = opt->color_columnno;
else if (!strcmp(var, "color.grep.matchcontext"))
color = opt->color_match_context;
else if (!strcmp(var, "color.grep.matchselected"))
--
2.18.0
[PATCH v3 7/7] contrib/git-jump/git-jump: jump to exact location
Take advantage of 'git-grep(1)''s new option, '--column' in order to
teach Peff's 'git-jump' script how to jump to the correct column for any
given match.
'git-grep(1)''s output is in the correct format for Vim's jump list, so
no additional cleanup is necessary.
Signed-off-by: Taylor Blau
---
contrib/git-jump/README | 12 ++--
contrib/git-jump/git-jump | 2 +-
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 4484bda410..2f618a7f97 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -25,6 +25,13 @@ git-jump will feed this to the editor:
foo.c:2: printf("hello word!\n");
---
+Or, when running 'git jump grep', column numbers will also be emitted,
+e.g. `git jump grep "hello"` would return:
+
+---
+foo.c:2:9: printf("hello word!\n");
+---
+
Obviously this trivial case isn't that interesting; you could just open
`foo.c` yourself. But when you have many changes scattered across a
project, you can use the editor's support to "jump" from point to point.
@@ -35,7 +42,8 @@ Git-jump can generate four types of interesting lists:
2. The beginning of any merge conflict markers.
- 3. Any grep matches.
+ 3. Any grep matches, including the column of the first match on a
+ line.
4. Any whitespace errors detected by `git diff --check`.
@@ -82,7 +90,7 @@ which does something similar to `git jump grep`. However, it
is limited
to positioning the cursor to the correct line in only the first file,
leaving you to locate subsequent hits in that file or other files using
the editor or pager. By contrast, git-jump provides the editor with a
-complete list of files and line numbers for each match.
+complete list of files, lines, and a column number for each match.
Limitations
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 80ab0590bc..931b0fe3a9 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -52,7 +52,7 @@ mode_merge() {
# editor shows them to us in the status bar.
mode_grep() {
cmd=$(git config jump.grepCmd)
- test -n "$cmd" || cmd="git grep -n"
+ test -n "$cmd" || cmd="git grep -n --column"
$cmd "$@" |
perl -pe '
s/[ \t]+/ /g;
--
2.18.0
[PATCH v3 4/7] grep.c: display column number of first match
To prepare for 'git grep' learning '--column', teach grep.c's
show_line() how to show the column of the first match on non-context
lines.
Signed-off-by: Taylor Blau
---
grep.c | 33 -
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/grep.c b/grep.c
index c885101017..83fe32a6a0 100644
--- a/grep.c
+++ b/grep.c
@@ -1405,7 +1405,7 @@ static int next_match(struct grep_opt *opt, char *bol,
char *eol,
}
static void show_line(struct grep_opt *opt, char *bol, char *eol,
- const char *name, unsigned lno, char sign)
+ const char *name, unsigned lno, ssize_t cno, char sign)
{
int rest = eol - bol;
const char *match_color, *line_color = NULL;
@@ -1440,6 +1440,17 @@ static void show_line(struct grep_opt *opt, char *bol,
char *eol,
output_color(opt, buf, strlen(buf), opt->color_lineno);
output_sep(opt, sign);
}
+ /*
+* Treat 'cno' as the 1-indexed offset from the start of a non-context
+* line to its first match. Otherwise, 'cno' is 0 indicating that we are
+* being called with a context line.
+*/
+ if (opt->columnnum && cno) {
+ char buf[32];
+ xsnprintf(buf, sizeof(buf), "%"PRIuMAX, (uintmax_t)cno);
+ output_color(opt, buf, strlen(buf), opt->color_columnno);
+ output_sep(opt, sign);
+ }
if (opt->color) {
regmatch_t match;
enum grep_context ctx = GREP_CONTEXT_BODY;
@@ -1545,7 +1556,7 @@ static void show_funcname_line(struct grep_opt *opt,
struct grep_source *gs,
break;
if (match_funcname(opt, gs, bol, eol)) {
- show_line(opt, bol, eol, gs->name, lno, '=');
+ show_line(opt, bol, eol, gs->name, lno, 0, '=');
break;
}
}
@@ -1610,7 +1621,7 @@ static void show_pre_context(struct grep_opt *opt, struct
grep_source *gs,
while (*eol != '\n')
eol++;
- show_line(opt, bol, eol, gs->name, cur, sign);
+ show_line(opt, bol, eol, gs->name, cur, 0, sign);
bol = eol + 1;
cur++;
}
@@ -1809,6 +1820,7 @@ static int grep_source_1(struct grep_opt *opt, struct
grep_source *gs, int colle
while (left) {
char *eol, ch;
int hit;
+ ssize_t cno;
ssize_t col = -1, icol = -1;
/*
@@ -1874,7 +1886,18 @@ static int grep_source_1(struct grep_opt *opt, struct
grep_source *gs, int colle
show_pre_context(opt, gs, bol, eol, lno);
else if (opt->funcname)
show_funcname_line(opt, gs, bol, lno);
- show_line(opt, bol, eol, gs->name, lno, ':');
+ cno = opt->invert ? icol : col;
+ if (cno < 0) {
+ /*
+* A negative cno indicates that there was no
+* match on the line. We are thus inverted and
+* being asked to show all lines that _don't_
+* match a given expression. Therefore, set cno
+* to 0 to suggest the whole line matches.
+*/
+ cno = 0;
+ }
+ show_line(opt, bol, eol, gs->name, lno, cno + 1, ':');
last_hit = lno;
if (opt->funcbody)
show_function = 1;
@@ -1903,7 +1926,7 @@ static int grep_source_1(struct grep_opt *opt, struct
grep_source *gs, int colle
/* If the last hit is within the post context,
* we need to show this line.
*/
- show_line(opt, bol, eol, gs->name, lno, '-');
+ show_line(opt, bol, eol, gs->name, lno, col + 1, '-');
}
next_line:
--
2.18.0
[PATCH v3 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'
Teach 'git-grep(1)' a new option, '--column', to show the column
number of the first match on a non-context line. This makes it possible
to teach 'contrib/git-jump/git-jump' how to seek to the first matching
position of a grep match in your editor, and allows similar additional
scripting capabilities.
For example:
$ git grep -n --column foo | head -n3
.clang-format:51:14:# myFunction(foo, bar, baz);
.clang-format:64:7:# int foo();
.clang-format:75:8:# void foo()
Signed-off-by: Taylor Blau
---
Documentation/git-grep.txt | 6 ++-
builtin/grep.c | 1 +
t/t7810-grep.sh| 95 ++
3 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 312409a607..31dc0392a6 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[-v | --invert-match] [-h|-H] [--full-name]
[-E | --extended-regexp] [-G | --basic-regexp]
[-P | --perl-regexp]
- [-F | --fixed-strings] [-n | --line-number]
+ [-F | --fixed-strings] [-n | --line-number] [--column]
[-l | --files-with-matches] [-L | --files-without-match]
[(-O | --open-files-in-pager) []]
[-z | --null]
@@ -169,6 +169,10 @@ providing this option will cause it to die.
--line-number::
Prefix the line number to matching lines.
+--column::
+ Prefix the 1-indexed byte-offset of the first match from the start of
the
+ matching line.
+
-l::
--files-with-matches::
--name-only::
diff --git a/builtin/grep.c b/builtin/grep.c
index ee753a403e..61bcaf6e58 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -828,6 +828,7 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
GREP_PATTERN_TYPE_PCRE),
OPT_GROUP(""),
OPT_BOOL('n', "line-number", , N_("show line
numbers")),
+ OPT_BOOL(0, "column", , N_("show column number of
first match")),
OPT_NEGBIT('h', NULL, , N_("don't show
filenames"), 1),
OPT_BIT('H', NULL, , N_("show filenames"), 1),
OPT_NEGBIT(0, "full-name", ,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 1797f632a3..9312c8daf5 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -99,6 +99,101 @@ do
test_cmp expected actual
'
+ test_expect_success "grep -w $L (with --column)" '
+ {
+ echo ${HC}file:5:foo mmap bar
+ echo ${HC}file:14:foo_mmap bar mmap
+ echo ${HC}file:5:foo mmap bar_mmap
+ echo ${HC}file:14:foo_mmap bar mmap baz
+ } >expected &&
+ git grep --column -w -e mmap $H >actual &&
+ test_cmp expected actual
+ '
+
+ test_expect_success "grep -w $L (with --column, extended OR)" '
+ {
+ echo ${HC}file:14:foo_mmap bar mmap
+ echo ${HC}file:19:foo_mmap bar mmap baz
+ } >expected &&
+ git grep --column -w -e mmap$ --or -e baz $H >actual &&
+ test_cmp expected actual
+ '
+
+ test_expect_success "grep -w $L (with --column, --invert)" '
+ {
+ echo ${HC}file:1:foo mmap bar
+ echo ${HC}file:1:foo_mmap bar
+ echo ${HC}file:1:foo_mmap bar mmap
+ echo ${HC}file:1:foo mmap bar_mmap
+ } >expected &&
+ git grep --column --invert -w -e baz $H -- file >actual &&
+ test_cmp expected actual
+ '
+
+ test_expect_success "grep $L (with --column, --invert, extended OR)" '
+ {
+ echo ${HC}hello_world:6:HeLLo_world
+ } >expected &&
+ git grep --column --invert -e ll --or --not -e _ $H --
hello_world \
+ >actual &&
+ test_cmp expected actual
+ '
+
+ test_expect_success "grep $L (with --column, --invert, extended AND)" '
+ {
+ echo ${HC}hello_world:3:Hello world
+ echo ${HC}hello_world:3:Hello_world
+ echo ${HC}hello_world:6:HeLLo_world
+ } >expected &&
+ git grep --column --invert --not -e _ --and --not -e ll $H --
hello_world \
+ >actual &&
+ test_cmp expected actual
+ '
+
+ test_expect_success "grep $L (with --column, double-negation)" '
+ {
+ echo ${HC}file:1:foo_mmap bar mmap baz
+ } >expected &&
+ git grep --column --not \( --not -e foo --or --not -e baz \) $H
-- file \
+ >actual &&
+ test_cmp expected actual
+ '
+
+ test_expect_success "grep -w $L (with
[PATCH v3 3/7] grep.[ch]: extend grep_opt to allow showing matched column
To support showing the matched column when calling 'git-grep(1)', teach
'grep_opt' the normal set of options to configure the default behavior
and colorization of this feature.
Now that we have opt->columnnum, use it to disable short-circuiting over
ORs and ANDs so that col and icol are always filled with the earliest
matches on each line. In addition, don't return the first match from
match_line(), for the same reason.
Signed-off-by: Taylor Blau
---
grep.c | 47 +--
grep.h | 2 ++
2 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/grep.c b/grep.c
index dedfe17f93..c885101017 100644
--- a/grep.c
+++ b/grep.c
@@ -46,6 +46,7 @@ void init_grep_defaults(void)
color_set(opt->color_filename, "");
color_set(opt->color_function, "");
color_set(opt->color_lineno, "");
+ color_set(opt->color_columnno, "");
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, "");
@@ -155,6 +156,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
opt->extended_regexp_option = def->extended_regexp_option;
opt->pattern_type_option = def->pattern_type_option;
opt->linenum = def->linenum;
+ opt->columnnum = def->columnnum;
opt->max_depth = def->max_depth;
opt->pathname = def->pathname;
opt->relative = def->relative;
@@ -164,6 +166,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
color_set(opt->color_filename, def->color_filename);
color_set(opt->color_function, def->color_function);
color_set(opt->color_lineno, def->color_lineno);
+ color_set(opt->color_columnno, def->color_columnno);
color_set(opt->color_match_context, def->color_match_context);
color_set(opt->color_match_selected, def->color_match_selected);
color_set(opt->color_selected, def->color_selected);
@@ -1277,23 +1280,36 @@ static int match_expr_eval(struct grep_opt *opt, struct
grep_expr *x, char *bol,
0);
break;
case GREP_NODE_AND:
- if (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
-icol, 0))
- return 0;
- h = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
+ h = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
icol, 0);
+ if (h || opt->columnnum) {
+ /*
+* Don't short-circuit AND when given --column, since a
+* NOT earlier in the tree may turn this into an OR. In
+* this case, see the below comment.
+*/
+ h &= match_expr_eval(opt, x->u.binary.right, bol, eol,
+ctx, col, icol, 0);
+ }
break;
case GREP_NODE_OR:
- if (!collect_hits)
+ if (!(collect_hits || opt->columnnum)) {
+ /*
+* Don't short-circuit OR when given --column (or
+* collecting hits) to ensure we don't skip a later
+* child that would produce an earlier match.
+*/
return (match_expr_eval(opt, x->u.binary.left, bol, eol,
ctx, col, icol, 0) ||
match_expr_eval(opt, x->u.binary.right, bol,
eol, ctx, col, icol, 0));
+ }
h = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
icol, 0);
- x->u.binary.left->hit |= h;
+ if (collect_hits)
+ x->u.binary.left->hit |= h;
h |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
-icol, 1);
+icol, collect_hits);
break;
default:
die("Unexpected node type (internal error) %d", x->node);
@@ -1316,6 +1332,7 @@ static int match_line(struct grep_opt *opt, char *bol,
char *eol,
enum grep_context ctx, int collect_hits)
{
struct grep_pat *p;
+ int hit = 0;
if (opt->extended)
return match_expr(opt, bol, eol, ctx, col, icol,
@@ -1325,11 +1342,21 @@ static int match_line(struct grep_opt *opt, char *bol,
char *eol,
for (p = opt->pattern_list; p; p = p->next) {
regmatch_t tmp;
if (match_one_pattern(p, bol, eol, ctx, , 0)) {
- *col = tmp.rm_so;
- return 1;
+ hit |= 1;
+
[PATCH v3 0/7] grep.c: teach --column to 'git-grep(1)'
Hi,
Attached is my third--anticipate the final--re-roll of my series to
teach 'git grep --column'.
Since the last time, only a couple of things have changed at Peff's
suggestions in [1]. The changes are summarized here, and an inter-diff
is available below:
- Change "%zu" to PRIuMAX (and an appropriate cast into uintmax_t). I
plan to send a follow-up patch to convert this back to "%zu" to see
how people feel about it, but I wanted to keep that out of the
present series in order to not hold things up.
- Don't short-circuit AND when given --column, since an earlier NOT
higher in the tree may cause an AND to be converted into an OR via
de Morgan's Law, in which case the problem is reduced to the OR case
(and should not have been short-circuited in the first place).
- Add a test in t7810 to cover this behavior (i.e., '--not \( -e x
--and -e y \)').
Thanks,
Taylor
[1]: https://public-inbox.org/git/20180621115302.gb15...@sigill.intra.peff.net/
Taylor Blau (7):
Documentation/config.txt: camel-case lineNumber for consistency
grep.c: expose {,inverted} match column in match_line()
grep.[ch]: extend grep_opt to allow showing matched column
grep.c: display column number of first match
builtin/grep.c: add '--column' option to 'git-grep(1)'
grep.c: add configuration variables to show matched option
contrib/git-jump/git-jump: jump to exact location
Documentation/config.txt | 7 +-
Documentation/git-grep.txt | 9 ++-
builtin/grep.c | 1 +
contrib/git-jump/README| 12 +++-
contrib/git-jump/git-jump | 2 +-
grep.c | 134 +
grep.h | 2 +
t/t7810-grep.sh| 95 ++
8 files changed, 228 insertions(+), 34 deletions(-)
Inter-diff (since: v2)
diff --git a/grep.c b/grep.c
index 08d3df2855..992673fe7e 100644
--- a/grep.c
+++ b/grep.c
@@ -1286,11 +1286,17 @@ static int match_expr_eval(struct grep_opt *opt, struct
grep_expr *x, char *bol,
0);
break;
case GREP_NODE_AND:
- if (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
-icol, 0))
- return 0;
- h = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
+ h = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
icol, 0);
+ if (h || opt->columnnum) {
+ /*
+* Don't short-circuit AND when given --column, since a
+* NOT earlier in the tree may turn this into an OR. In
+* this case, see the below comment.
+*/
+ h &= match_expr_eval(opt, x->u.binary.right, bol, eol,
+ctx, col, icol, 0);
+ }
break;
case GREP_NODE_OR:
if (!(collect_hits || opt->columnnum)) {
@@ -1447,7 +1453,7 @@ static void show_line(struct grep_opt *opt, char *bol,
char *eol,
*/
if (opt->columnnum && cno) {
char buf[32];
- xsnprintf(buf, sizeof(buf), "%zu", cno);
+ xsnprintf(buf, sizeof(buf), "%"PRIuMAX, (uintmax_t)cno);
output_color(opt, buf, strlen(buf), opt->color_columnno);
output_sep(opt, sign);
}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index bf0b572dab..9312c8daf5 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -110,7 +110,7 @@ do
test_cmp expected actual
'
- test_expect_success "grep -w $L (with --column, extended)" '
+ test_expect_success "grep -w $L (with --column, extended OR)" '
{
echo ${HC}file:14:foo_mmap bar mmap
echo ${HC}file:19:foo_mmap bar mmap baz
@@ -130,7 +130,7 @@ do
test_cmp expected actual
'
- test_expect_success "grep $L (with --column, --invert, extended)" '
+ test_expect_success "grep $L (with --column, --invert, extended OR)" '
{
echo ${HC}hello_world:6:HeLLo_world
} >expected &&
@@ -139,6 +139,17 @@ do
test_cmp expected actual
'
+ test_expect_success "grep $L (with --column, --invert, extended AND)" '
+ {
+ echo ${HC}hello_world:3:Hello world
+ echo ${HC}hello_world:3:Hello_world
+ echo ${HC}hello_world:6:HeLLo_world
+ } >expected &&
+ git grep --column --invert --not -e _ --and --not -e ll $H --
hello_world \
+ >actual &&
+ test_cmp expected actual
+ '
+
test_expect_success "grep $L (with --column, double-negation)" '
{
[PATCH v3 1/7] Documentation/config.txt: camel-case lineNumber for consistency
lineNumber has casing that is inconsistent with surrounding options, like color.grep.matchContext, and color.grep.matchSelected. Re-case this documentation in order to be consistent with the text around it, and to ensure that new entries are consistent, too. Signed-off-by: Taylor Blau --- Documentation/config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ab641bf5a9..58fde4daea 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1181,7 +1181,7 @@ color.grep.:: filename prefix (when not using `-h`) `function`;; function name lines (when using `-p`) -`linenumber`;; +`lineNumber`;; line number prefix (when using `-n`) `match`;; matching text (same as setting `matchContext` and `matchSelected`) -- 2.18.0
[PATCH v3 2/7] grep.c: expose {,inverted} match column in match_line()
When calling match_line(), callers presently cannot determine the
relative offset of the match because match_line() discards the
'regmatch_t' that contains this information.
Instead, teach match_line() to take in two 'ssize_t's. Fill the first
with the offset of the match produced by the given expression. If
extended, fill the later with the offset of the match produced as if
--invert were given.
For instance, matching "--not -e x" on this line produces a columnar
offset of 0, (i.e., the whole line does not contain an x), but "--invert
--not -e -x" will fill the later ssize_t of the column containing an
"x", because this expression is semantically equivalent to "-e x".
To determine the column for the inverted and non-inverted case, do the
following:
- If matching an atom, the non-inverted column is as given from
match_one_pattern(), and the inverted column is unset.
- If matching a --not, the inverted column and non-inverted column
swap.
- If matching an --and, or --or, the non-inverted column is the
minimum of the two children.
Presently, the existing short-circuiting logic for AND and OR applies as
before. This will change in the following commit when we add options to
configure the --column flag. Taken together, this and the forthcoming
change will always yield the earlier column on a given line.
This patch will become useful when we later pick between the two new
results in order to display the column number of the first match on a
line with --column.
Co-authored-by: Jeff King
Signed-off-by: Taylor Blau
---
grep.c | 58 +++---
1 file changed, 39 insertions(+), 19 deletions(-)
diff --git a/grep.c b/grep.c
index 45ec7e636c..dedfe17f93 100644
--- a/grep.c
+++ b/grep.c
@@ -1248,11 +1248,11 @@ static int match_one_pattern(struct grep_pat *p, char
*bol, char *eol,
return hit;
}
-static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
- enum grep_context ctx, int collect_hits)
+static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char
*bol,
+ char *eol, enum grep_context ctx, ssize_t *col,
+ ssize_t *icol, int collect_hits)
{
int h = 0;
- regmatch_t match;
if (!x)
die("Not a valid grep expression");
@@ -1261,25 +1261,39 @@ static int match_expr_eval(struct grep_expr *x, char
*bol, char *eol,
h = 1;
break;
case GREP_NODE_ATOM:
- h = match_one_pattern(x->u.atom, bol, eol, ctx, , 0);
+ {
+ regmatch_t tmp;
+ h = match_one_pattern(x->u.atom, bol, eol, ctx,
+ , 0);
+ if (h && (*col < 0 || tmp.rm_so < *col))
+ *col = tmp.rm_so;
+ }
break;
case GREP_NODE_NOT:
- h = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);
+ /*
+* Upon visiting a GREP_NODE_NOT, col and icol become swapped.
+*/
+ h = !match_expr_eval(opt, x->u.unary, bol, eol, ctx, icol, col,
+0);
break;
case GREP_NODE_AND:
- if (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))
+ if (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
+icol, 0))
return 0;
- h = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);
+ h = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
+ icol, 0);
break;
case GREP_NODE_OR:
if (!collect_hits)
- return (match_expr_eval(x->u.binary.left,
- bol, eol, ctx, 0) ||
- match_expr_eval(x->u.binary.right,
- bol, eol, ctx, 0));
- h = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);
+ return (match_expr_eval(opt, x->u.binary.left, bol, eol,
+ ctx, col, icol, 0) ||
+ match_expr_eval(opt, x->u.binary.right, bol,
+ eol, ctx, col, icol, 0));
+ h = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
+ icol, 0);
x->u.binary.left->hit |= h;
- h |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);
+ h |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
+icol, 1);
break;
default:
die("Unexpected node type (internal error) %d", x->node);
@@
عرض عمل
Dear Sir/Madam, Greetings from United Arab Emirate(U.A.E)Dubai,i am banker with a confidential project which i need to solicite your cooperation. Please reply to enable me send you the complete details about this project. Best Regards, Mr.Thomas Pereira
[RFC PATCH v5] Implement --first-parent for git rev-list --bisect
This will enable users to implement bisecting on first parents
which can be useful for when the commits from a feature branch
that we want to merge are not always tested.
Signed-off-by: Tiago Botelho
---
This patch resolves every issue with the unit tests.
The bug detected by Junio regarding do_find_bisection() propagating
a weight of -1 over to the child nodes ended up not being an
issue since it cannot happen in practice, there will always be at
least one UNINTERESTING parent which will propagate either a weight
of 0 or 1 throughout the child nodes.
bisect.c | 43 ++
bisect.h | 1 +
builtin/rev-list.c | 3 +++
revision.c | 3 ---
t/t6002-rev-list-bisect.sh | 58 ++
5 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/bisect.c b/bisect.c
index 4eafc8262..cb80f29c5 100644
--- a/bisect.c
+++ b/bisect.c
@@ -82,15 +82,16 @@ static inline void weight_set(struct commit_list *elem, int
weight)
*((int*)(elem->item->util)) = weight;
}
-static int count_interesting_parents(struct commit *commit)
+static int count_interesting_parents(struct commit *commit, unsigned
bisect_flags)
{
struct commit_list *p;
int count;
for (count = 0, p = commit->parents; p; p = p->next) {
- if (p->item->object.flags & UNINTERESTING)
- continue;
- count++;
+ if (!(p->item->object.flags & UNINTERESTING))
+ count++;
+ if (bisect_flags & BISECT_FIRST_PARENT)
+ break;
}
return count;
}
@@ -117,10 +118,10 @@ static inline int halfway(struct commit_list *p, int nr)
}
#if !DEBUG_BISECT
-#define show_list(a,b,c,d) do { ; } while (0)
+#define show_list(a,b,c,d,e) do { ; } while (0)
#else
static void show_list(const char *debug, int counted, int nr,
- struct commit_list *list)
+ struct commit_list *list, unsigned bisect_flags)
{
struct commit_list *p;
@@ -146,10 +147,14 @@ static void show_list(const char *debug, int counted, int
nr,
else
fprintf(stderr, "---");
fprintf(stderr, " %.*s", 8, oid_to_hex(>object.oid));
- for (pp = commit->parents; pp; pp = pp->next)
+ for (pp = commit->parents; pp; pp = pp->next) {
fprintf(stderr, " %.*s", 8,
oid_to_hex(>item->object.oid));
+ if (bisect_flags & BISECT_FIRST_PARENT)
+ break;
+ }
+
subject_len = find_commit_subject(buf, _start);
if (subject_len)
fprintf(stderr, " %.*s", subject_len, subject_start);
@@ -267,13 +272,13 @@ static struct commit_list *do_find_bisection(struct
commit_list *list,
unsigned flags = commit->object.flags;
p->item->util = [n++];
- switch (count_interesting_parents(commit)) {
+ switch (count_interesting_parents(commit, bisect_flags)) {
case 0:
if (!(flags & TREESAME)) {
weight_set(p, 1);
counted++;
show_list("bisection 2 count one",
- counted, nr, list);
+ counted, nr, list, bisect_flags);
}
/*
* otherwise, it is known not to reach any
@@ -289,7 +294,7 @@ static struct commit_list *do_find_bisection(struct
commit_list *list,
}
}
- show_list("bisection 2 initialize", counted, nr, list);
+ show_list("bisection 2 initialize", counted, nr, list, bisect_flags);
/*
* If you have only one parent in the resulting set
@@ -319,7 +324,7 @@ static struct commit_list *do_find_bisection(struct
commit_list *list,
counted++;
}
- show_list("bisection 2 count_distance", counted, nr, list);
+ show_list("bisection 2 count_distance", counted, nr, list,
bisect_flags);
while (counted < nr) {
for (p = list; p; p = p->next) {
@@ -329,6 +334,11 @@ static struct commit_list *do_find_bisection(struct
commit_list *list,
if (0 <= weight(p))
continue;
for (q = p->item->parents; q; q = q->next) {
+ if ((bisect_flags & BISECT_FIRST_PARENT)) {
+ if (weight(q) < 0)
+ q = NULL;
+ break;
+ }
if
Unexpected ignorecase=false behavior on Windows
On Windows, when creating following repository: $ git init $ echo "1" > file.txt $ git add . $ git commit -m "initial import" $ ren file.txt File.txt $ git config core.ignorecase false Status results are: $ git status --porcelain ?? File.txt As on Unix, I would expect to see: $ git status --porcelain D file.txt ?? File.txt Is the Windows behavior intended? I'm asking, because e.g. JGit will report missing files, too, and I'm wondering what would be the correct way to resolve this discrepancy? (JGit does not have "ignorecase=true"-support at all, btw). Tested with git version 2.17.1.windows.2 -Marc
[ANNOUNCE] Git for Windows 2.18.0
Dear Git users, It is my pleasure to announce that Git for Windows 2.18.0 is available from: https://gitforwindows.org/ Changes since Git for Windows v2.17.1(2) (May 29th 2018) New Features * Comes with Git v2.18.0. * Comes with Git Credential Manager v1.16.2. Bug Fixes * The diff filter for .pdf files was fixed. * The start-ssh-agent.cmd script no longer overrides the HOME variable. * Fixes an issue where passing an argument with a trailing slash from Git Bash to git.exe was dropping that trailing slash. * The http.schannel.checkRevoke setting now really works. Filename | SHA-256 | --- Git-2.18.0-64-bit.exe | aa81c9f2a81fd07ba0582095474365821880fd787b1cbe03abaf71d9aa69d359 Git-2.18.0-32-bit.exe | f4e910eb5182aefada20226a5a0a64b5b528ad1439c28a47c25252ee27f71af0 PortableGit-2.18.0-64-bit.7z.exe | cd84a13b6c7aac0e924cb4db2476e2f4379aab4b8e60246992a6c5eebeac360c PortableGit-2.18.0-32-bit.7z.exe | 28e68a781a78009913fef3d6c1074a6c91b05e4010bfd9efaff7b8398c87e017 MinGit-2.18.0-64-bit.zip | 1dfd05de1320d57f448ed08a07c0b9de2de8976c83840f553440689b5db6a1cf MinGit-2.18.0-32-bit.zip | c2f59c121d0f5aac31c959e5ba2878542b6cbca6604778566061d45585e70895 MinGit-2.18.0-busybox-64-bit.zip | 1a34608fbb82f607924d21ac2737c189a250fd9c6f4255a939e55c7fc3b10a0d MinGit-2.18.0-busybox-32-bit.zip | 6a667b03bacbcd2627e4e9a29a222c1ccb01c340bc721352afd6427f3621945c Git-2.18.0-64-bit.tar.bz2 | 120b7501b5563212f1ecbcd8fadac257e510067e0297ff844bc3b18d90eefb96 Git-2.18.0-32-bit.tar.bz2 | 3eab2705622b7fc5b49e4195e30fa92162ad4924072d01ca52abf0d7134d356c pdbs-for-git-64-bit-2.18.0.1.cd1a74fc9d-1.zip | 20fa4809b19a346e70d76066e98b76ca42f8364fb2997f3922abdb14d6321c56 pdbs-for-git-32-bit-2.18.0.1.cd1a74fc9d-1.zip | b410397e237277e2ae1c22207190d11ee2f137e986baa15dfdb8dd92f0285120 Ciao, Johannes
[PATCH v2 4/4] branch: make "-l" a synonym for "--list"
The other "mode" options of git-branch have short-option
aliases that are easy to type (e.g., "-d" and "-m"). Let's
give "--list" the same treatment.
This also makes it consistent with the similar "git tag -l"
option.
We didn't do this originally because "--create-reflog" was
squatting on the "-l" option. Now that we've deprecated that
use for long enough, we can make the switch.
Signed-off-by: Jeff King
---
This one is mostly a squash of the remove/reincarnate steps from the
previous iteration. I also noticed that my original forgot to update the
documentation, which this patch does.
Documentation/git-branch.txt | 3 +--
builtin/branch.c | 22 +-
2 files changed, 2 insertions(+), 23 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 1072ca0eb6..fc88e984e1 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -100,8 +100,6 @@ OPTIONS
The negated form `--no-create-reflog` only overrides an earlier
`--create-reflog`, but currently does not negate the setting of
`core.logAllRefUpdates`.
-+
-The `-l` option is a deprecated synonym for `--create-reflog`.
-f::
--force::
@@ -156,6 +154,7 @@ This option is only applicable in non-verbose mode.
--all::
List both remote-tracking branches and local branches.
+-l::
--list::
List branches. With optional `...`, e.g. `git
branch --list 'maint-*'`, list only the branches that match
diff --git a/builtin/branch.c b/builtin/branch.c
index ed4c093747..c1662e3db3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -36,7 +36,6 @@ static const char * const builtin_branch_usage[] = {
static const char *head;
static struct object_id head_oid;
-static int used_deprecated_reflog_option;
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
@@ -574,14 +573,6 @@ static int edit_branch_description(const char *branch_name)
return 0;
}
-static int deprecated_reflog_option_cb(const struct option *opt,
- const char *arg, int unset)
-{
- used_deprecated_reflog_option = 1;
- *(int *)opt->value = !unset;
- return 0;
-}
-
int cmd_branch(int argc, const char **argv, const char *prefix)
{
int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
@@ -623,14 +614,8 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT_BIT('M', NULL, , N_("move/rename a branch, even if
target exists"), 2),
OPT_BIT('c', "copy", , N_("copy a branch and its reflog"),
1),
OPT_BIT('C', NULL, , N_("copy a branch, even if target
exists"), 2),
- OPT_BOOL(0, "list", , N_("list branch names")),
+ OPT_BOOL('l', "list", , N_("list branch names")),
OPT_BOOL(0, "create-reflog", , N_("create the branch's
reflog")),
- {
- OPTION_CALLBACK, 'l', NULL, , NULL,
- N_("deprecated synonym for --create-reflog"),
- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
- deprecated_reflog_option_cb
- },
OPT_BOOL(0, "edit-description", _description,
N_("edit the description for the branch")),
OPT__FORCE(, N_("force creation, move/rename, deletion"),
PARSE_OPT_NOCOMPLETE),
@@ -703,11 +688,6 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (list)
setup_auto_pager("branch", 1);
- if (used_deprecated_reflog_option && !list) {
- warning("the '-l' alias for '--create-reflog' is deprecated;");
- warning("it will be removed in a future version of Git");
- }
-
if (delete) {
if (!argc)
die(_("branch name required"));
--
2.18.0.539.gcf23d6d4f1
[PATCH v2 3/4] branch: deprecate "-l" option
The "-l" option is short for "--create-reflog". This has
caused much confusion over the years. Most people expect it
to work as "--list", because that would match the other
"mode" options like -d/--delete and -m/--move, as well as
the similar -l/--list option of git-tag.
Adding to the confusion, using "-l" _appears_ to work as
"--list" in some cases:
$ git branch -l
* master
because the branch command defaults to listing (so even
trying to specify --list in the command above is redundant).
But that may bite the user later when they add a pattern,
like:
$ git branch -l foo
which does not return an empty list, but in fact creates a
new branch (with a reflog, naturally) called "foo".
It's also probably quite uncommon for people to actually use
"-l" to create a reflog. Since 0bee591869 (Enable reflogs by
default in any repository with a working directory.,
2006-12-14), this is the default in non-bare repositories.
So it's rather unfortunate that the feature squats on the
short-and-sweet "-l" (which was only added in 3a4b3f269c
(Create/delete branch ref logs., 2006-05-19), meaning there
were only 7 months where it was actually useful).
Let's deprecate "-l" in hopes of eventually re-purposing it
to "--list".
Note that we issue the warning only when we're not in list
mode. This means that people for whom it works as a happy
accident, namely:
$ git branch -l
master
won't see the warning at all. And when we eventually switch
to it meaning "--list", that will just continue to work.
We do the issue the warning for these important cases:
- when we are actually creating a branch, in case the user
really did mean it as "--create-reflog"
- when we are in some _other_ mode, like deletion. There
the "-l" is a noop for now, but it will eventually
conflict with any other mode request, and the user
should be told that this is changing.
Signed-off-by: Jeff King
---
This one has the interesting changes in this re-roll.
Documentation/git-branch.txt | 3 ++-
builtin/branch.c | 22 +-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 02eccbb931..1072ca0eb6 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -91,7 +91,6 @@ OPTIONS
-D::
Shortcut for `--delete --force`.
--l::
--create-reflog::
Create the branch's reflog. This activates recording of
all changes made to the branch ref, enabling use of date
@@ -101,6 +100,8 @@ OPTIONS
The negated form `--no-create-reflog` only overrides an earlier
`--create-reflog`, but currently does not negate the setting of
`core.logAllRefUpdates`.
++
+The `-l` option is a deprecated synonym for `--create-reflog`.
-f::
--force::
diff --git a/builtin/branch.c b/builtin/branch.c
index 5217ba3bde..ed4c093747 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -36,6 +36,7 @@ static const char * const builtin_branch_usage[] = {
static const char *head;
static struct object_id head_oid;
+static int used_deprecated_reflog_option;
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
@@ -573,6 +574,14 @@ static int edit_branch_description(const char *branch_name)
return 0;
}
+static int deprecated_reflog_option_cb(const struct option *opt,
+ const char *arg, int unset)
+{
+ used_deprecated_reflog_option = 1;
+ *(int *)opt->value = !unset;
+ return 0;
+}
+
int cmd_branch(int argc, const char **argv, const char *prefix)
{
int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
@@ -615,7 +624,13 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT_BIT('c', "copy", , N_("copy a branch and its reflog"),
1),
OPT_BIT('C', NULL, , N_("copy a branch, even if target
exists"), 2),
OPT_BOOL(0, "list", , N_("list branch names")),
- OPT_BOOL('l', "create-reflog", , N_("create the branch's
reflog")),
+ OPT_BOOL(0, "create-reflog", , N_("create the branch's
reflog")),
+ {
+ OPTION_CALLBACK, 'l', NULL, , NULL,
+ N_("deprecated synonym for --create-reflog"),
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
+ deprecated_reflog_option_cb
+ },
OPT_BOOL(0, "edit-description", _description,
N_("edit the description for the branch")),
OPT__FORCE(, N_("force creation, move/rename, deletion"),
PARSE_OPT_NOCOMPLETE),
@@ -688,6 +703,11 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (list)
setup_auto_pager("branch", 1);
+ if (used_deprecated_reflog_option && !list) {
+ warning("the '-l' alias for '--create-reflog' is deprecated;");
+
[PATCH v2 1/4] t3200: unset core.logallrefupdates when testing reflog creation
This test checks that the "-l" option creates a reflog. But in fact we'd create one even without it, since the default in a non-bare repository is to do so. Let's unset the config so we can be sure our "-l" option is kicking in. Note that we can't do this with test_config, since that would leave the variable unset after our test finishes, confusing downstream tests (the helper is not not smart enough to restore the previous value, and just always runs test_unconfig). Signed-off-by: Jeff King --- Same as before. t/t3200-branch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 08467982f6..ec56176093 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -51,7 +51,7 @@ $ZERO_OID $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 + bran EOF test_expect_success 'git branch -l d/e/f should create a branch and a log' ' GIT_COMMITTER_DATE="2005-05-26 23:30" \ - git branch -l d/e/f && + git -c core.logallrefupdates=false branch -l d/e/f && test_path_is_file .git/refs/heads/d/e/f && test_path_is_file .git/logs/refs/heads/d/e/f && test_cmp expect .git/logs/refs/heads/d/e/f -- 2.18.0.539.gcf23d6d4f1
[PATCH v2 2/4] t: switch "branch -l" to "branch --create-reflog"
In preparation for deprecating "-l", let's make sure we're using the recommended option ourselves. This patch just mechanically converts "branch -l" to "branch --create-reflog". Note that with the exception of the actual "--create-reflog" test, we could actually remove "-l" entirely from most of these callers. That's because these days core.logallrefupdates defaults to true in a non-bare repository. I've left them in place, though, since they serve to document the expectation of the test, even if they are technically noops. Signed-off-by: Jeff King --- Same as before. t/t1410-reflog.sh | 4 ++-- t/t3200-branch.sh | 34 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh index 553e26d9ce..8293131001 100755 --- a/t/t1410-reflog.sh +++ b/t/t1410-reflog.sh @@ -339,8 +339,8 @@ test_expect_failure 'reflog with non-commit entries displays all entries' ' ' test_expect_success 'reflog expire operates on symref not referrent' ' - git branch -l the_symref && - git branch -l referrent && + git branch --create-reflog the_symref && + git branch --create-reflog referrent && git update-ref referrent HEAD && git symbolic-ref refs/heads/the_symref refs/heads/referrent && test_when_finished "rm -f .git/refs/heads/referrent.lock" && diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index ec56176093..dbca665da4 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -49,9 +49,9 @@ test_expect_success 'git branch HEAD should fail' ' cat >expect < 1117150200 + branch: Created from master EOF -test_expect_success 'git branch -l d/e/f should create a branch and a log' ' +test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' ' GIT_COMMITTER_DATE="2005-05-26 23:30" \ - git -c core.logallrefupdates=false branch -l d/e/f && + git -c core.logallrefupdates=false branch --create-reflog d/e/f && test_path_is_file .git/refs/heads/d/e/f && test_path_is_file .git/logs/refs/heads/d/e/f && test_cmp expect .git/logs/refs/heads/d/e/f @@ -82,7 +82,7 @@ test_expect_success 'git branch -m dumps usage' ' test_expect_success 'git branch -m m broken_symref should work' ' test_when_finished "git branch -D broken_symref" && - git branch -l m && + git branch --create-reflog m && git symbolic-ref refs/heads/broken_symref refs/heads/i_am_broken && git branch -m m broken_symref && git reflog exists refs/heads/broken_symref && @@ -90,13 +90,13 @@ test_expect_success 'git branch -m m broken_symref should work' ' ' test_expect_success 'git branch -m m m/m should work' ' - git branch -l m && + git branch --create-reflog m && git branch -m m m/m && git reflog exists refs/heads/m/m ' test_expect_success 'git branch -m n/n n should work' ' - git branch -l n/n && + git branch --create-reflog n/n && git branch -m n/n n && git reflog exists refs/heads/n ' @@ -378,9 +378,9 @@ mv .git/config-saved .git/config git config branch.s/s.dummy Hello test_expect_success 'git branch -m s/s s should work when s/t is deleted' ' - git branch -l s/s && + git branch --create-reflog s/s && git reflog exists refs/heads/s/s && - git branch -l s/t && + git branch --create-reflog s/t && git reflog exists refs/heads/s/t && git branch -d s/t && git branch -m s/s s && @@ -444,7 +444,7 @@ test_expect_success 'git branch --copy dumps usage' ' ' test_expect_success 'git branch -c d e should work' ' - git branch -l d && + git branch --create-reflog d && git reflog exists refs/heads/d && git config branch.d.dummy Hello && git branch -c d e && @@ -459,7 +459,7 @@ test_expect_success 'git branch -c d e should work' ' ' test_expect_success 'git branch --copy is a synonym for -c' ' - git branch -l copy && + git branch --create-reflog copy && git reflog exists refs/heads/copy && git config branch.copy.dummy Hello && git branch --copy copy copy-to && @@ -486,7 +486,7 @@ test_expect_success 'git branch -c ee ef should copy ee to create branch ef' ' ' test_expect_success 'git branch -c f/f g/g should work' ' - git branch -l f/f && + git branch --create-reflog f/f && git reflog exists refs/heads/f/f && git config branch.f/f.dummy Hello && git branch -c f/f g/g && @@ -497,7 +497,7 @@ test_expect_success 'git branch -c f/f g/g should work' ' ' test_expect_success 'git branch -c m2 m2 should work' ' - git branch -l m2 && + git branch --create-reflog m2 && git reflog exists refs/heads/m2 && git config branch.m2.dummy Hello && git branch -c m2 m2 && @@ -506,18 +506,18 @@ test_expect_success 'git branch -c m2 m2 should work' ' '
[PATCH v2 0/4] branch -l deprecation revisited
This series replaces the contents of jk/branch-l-0-deprecation, jk/branch-l-1-removal, and jk/branch-l-2-reincarnation. It implements the idea discussed in the subthread in: https://public-inbox.org/git/xmqqzi0hety4@gitster-ct.c.googlers.com/ Namely that "branch -l" would warn about deprecation only when we're not in list mode, and then we'll eventually jump straight to repurposing "-l" as "--list". This gets us to the end result faster, without the annoying "-l doesn't mean anything" step in between, and without useless warnings on "git branch -l". It also sidesteps any pager issues since list mode will not print the warning. The first 3 patches would become jk/branch-l-0-deprecate, and then the final one would become jk/branch-l-1-repurpose or similar. No third step required. [1/4]: t3200: unset core.logallrefupdates when testing reflog creation [2/4]: t: switch "branch -l" to "branch --create-reflog" [3/4]: branch: deprecate "-l" option [4/4]: branch: make "-l" a synonym for "--list" Documentation/git-branch.txt | 2 +- builtin/branch.c | 4 ++-- t/t1410-reflog.sh| 4 ++-- t/t3200-branch.sh| 34 +- 4 files changed, 22 insertions(+), 22 deletions(-) -Peff
Re: [PATCH] cherry-pick: do not error on non-merge commits when '-m 1' is specified
Junio C Hamano writes: > Sergey Organov writes: > >> When cherry-picking multiple commits, it's impossible to have both >> merge- and non-merge commits on the same command-line. Not specifying >> '-m 1' results in cherry-pick refusing to handle merge commits, while >> specifying '-m 1' fails on non-merge commits. > > Allowing "-m1" even when picking a single parent commit, because > the 1-st parent is defined for such a commit, makes sense, espeially > when running a cherry-pick on a range, exactly for the above reason. > It is slightly less so when cherry-picking a single commit, but not > by a large margin. [...] >> +} else if (1 < opts->mainline) >> +/* Non-first parent explicitly specified as mainline for >> + * non-merge commit */ > > Style. > > /* >* Our multi-line comments are to be >* formatted like this. >*/ Ah, sorry for that. Will you fix it for me? Thanks! -- Sergey
Re: [PATCH v3 1/7] git-rebase.txt: document incompatible options
> @@ -487,6 +510,52 @@ recreates the topic branch with fresh commits so it can > be remerged > successfully without needing to "revert the reversion" (see the > link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for > details). > > +INCOMPATIBLE OPTIONS > + > + > +git-rebase has many flags that are incompatible with each other, > +predominantly due to the fact that it has three different underlying > +implementations: > + > + * one based on linkgit:git-am[1] (the default) > + * one based on linkgit:git-merge-recursive[1] (merge backend) Referencing a non-existing man page via the 'linkgit' macro causes 'make check-docs' to complain: LINT lint-docs ./git-rebase.txt:511: no such source: git-merge-recursive[1]
Re: [PATCH] submodule.c: report the submodule that an error occurs in
> When an error occurs in updating the working tree of a submodule in
> submodule_move_head, tell the user which submodule the error occurred in.
>
> The call to read-tree contains a super-prefix, such that the read-tree
> will correctly report any path related issues, but some error messages
> do not contain a path, for example:
>
> ~/gerrit$ git checkout --recurse-submodules origin/master
> ~/gerrit$ fatal: failed to unpack tree object
> 07672f31880ba80300b38492df9d0acfcd6ee00a
>
> Give the hint which submodule has a problem.
>
> Signed-off-by: Stefan Beller
> ---
> submodule.c | 2 +-
> t/lib-submodule-update.sh | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 939d6870ecd..ebd092a14fd 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1668,7 +1668,7 @@ int submodule_move_head(const char *path,
> argv_array_push(, new_head ? new_head : empty_tree_oid_hex());
>
> if (run_command()) {
> - ret = -1;
> + ret = error(_("Submodule '%s' could not be updated."), path);
This is a translated error message ...
> goto out;
> }
>
> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
> index 1f38a85371a..e27f5d8541d 100755
> --- a/t/lib-submodule-update.sh
> +++ b/t/lib-submodule-update.sh
> @@ -781,7 +781,8 @@ test_submodule_recursing_with_args_common() {
> (
> cd submodule_update &&
> git branch -t invalid_sub1 origin/invalid_sub1 &&
> - test_must_fail $command invalid_sub1 &&
> + test_must_fail $command invalid_sub1 2>err &&
> + grep sub1 err &&
... so the test should use 'test_i18ngrep' to check it.
> test_superproject_content origin/add_sub1 &&
> test_submodule_content sub1 origin/add_sub1
> )
> --
> 2.18.0.rc2.346.g013aa6912e-goog
>
>
PLEASE READ YOUR MESSAGE!
Hello Dear Friend I have a business proposal for you which involves a huge amount of money that run into millions of US dollars that i want to transfer to your country for safe keeping and investment this is not a spam, its a real business that will benefit both of us. Please try as much as possible to get back to me on this i will explain more to you when you finally respond!(kabiruwahid1...@gmail.com)!!Yours truly Mr Kabiru Wahid!
Re: [PATCH] Documentation: Spelling and grammar fixes
On Fri, Jun 22, 2018 at 2:50 AM Ville Skyttä wrote: > Signed-off-by: Ville Skyttä > --- > diff --git a/Documentation/config.txt b/Documentation/config.txt > @@ -354,7 +354,7 @@ advice.*:: > ignoredHook:: > - Advice shown if an hook is ignored because the hook is not > + Advice shown if a hook is ignored because the hook is not British vs. American English, I believe. Since the project leans toward[1] American English, this change is probably reasonable in the larger scope of this patch. [1]: https://public-inbox.org/git/xmqq4m9gpebm@gitster.mtv.corp.google.com/ > diff --git a/Documentation/technical/api-gitattributes.txt > b/Documentation/technical/api-gitattributes.txt > @@ -146,7 +146,7 @@ To get the values of all attributes associated with a > file: > * Iterate over the `attr_check.items[]` array to examine >the attribute names and values. The name of the attribute > - described by a `attr_check.items[]` object can be retrieved via > + described by an `attr_check.items[]` object can be retrieved via This also collapses the space-space to a single space, which is good. All the other changes look fine, as well. Thanks.
Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'
On Thu, Jun 21, 2018 at 04:45:49PM -0500, Taylor Blau wrote: > > but not this: > > > > $ ./git grep --column -v --not -e scalable --and --not -e fast -- > > README.md > > README.md:13:Git - fast, scalable, distributed revision control system > > README.md:16:Git is a fast, scalable, distributed revision control system > > with an > > > > I wasn't sure where we landed in the discussion on "how much crazy stuff > > to support". But AFAIK, the code in this iteration handles every crazy > > case already except this one. If we're going to care about OR, maybe we > > should just cover all cases. > > Good catch. As I understand it, we need to short-circuit AND because an > --invert or a --not above it in the expression tree would cause it to be > turned into an OR by de Morgan's Law, and therefore be susceptible to > the same reasoning that caused me to remove short-circuiting on OR. Right, exactly. > > Unfortunately %z isn't portable. You have to use: > > > > xsnprintf(buf, sizeof(buf), "%"PRIuMAX, (uintmax_t)cno); > > I see. Per your next note, I've changed this to "%"PRIuMAX (and the > appropriate cast into uintmax_t), and will send another patch out in the > future changing it _back_ to "%zu" and figure out how folks feel about > it. That sounds perfect. I think the time may be good for a "%zu" weather-balloon, but it's best not to tangle it up with your other changes. -Peff
[PATCH] Documentation: Spelling and grammar fixes
Signed-off-by: Ville Skyttä
---
Documentation/SubmittingPatches | 4 ++--
Documentation/config.txt | 2 +-
Documentation/git-bisect-lk2009.txt | 2 +-
Documentation/git-imap-send.txt | 4 ++--
Documentation/git-notes.txt | 2 +-
Documentation/git-status.txt | 2 +-
Documentation/git-svn.txt | 2 +-
Documentation/giteveryday.txt | 2 +-
Documentation/gitsubmodules.txt | 2 +-
Documentation/glossary-content.txt| 2 +-
Documentation/technical/api-directory-listing.txt | 2 +-
Documentation/technical/api-gitattributes.txt | 2 +-
Documentation/technical/commit-graph-format.txt | 2 +-
13 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 248854440..b44fd51f2 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -298,7 +298,7 @@ smaller project it is a good discipline to follow it.
The sign-off is a simple line at the end of the explanation for
the patch, which certifies that you wrote it or otherwise have
-the right to pass it on as a open-source patch. The rules are
+the right to pass it on as an open-source patch. The rules are
pretty simple: if you can certify the below D-C-O:
[[dco]]
@@ -403,7 +403,7 @@ don't demand). +git log -p {litdd}
_$area_you_are_modifying_+ would
help you find out who they are.
. You get comments and suggestions for improvements. You may
- even get them in a "on top of your change" patch form.
+ even get them in an "on top of your change" patch form.
. Polish, refine, and re-send to the list and the people who
spend their time to improve your patch. Go back to step (2).
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab641bf5a..98ffbd208 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -354,7 +354,7 @@ advice.*::
Advice on what to do when you've accidentally added one
git repo inside of another.
ignoredHook::
- Advice shown if an hook is ignored because the hook is not
+ Advice shown if a hook is ignored because the hook is not
set as executable.
waitingForEditor::
Print a message to the terminal whenever Git is waiting for
diff --git a/Documentation/git-bisect-lk2009.txt
b/Documentation/git-bisect-lk2009.txt
index 78479b003..0f9ef2f25 100644
--- a/Documentation/git-bisect-lk2009.txt
+++ b/Documentation/git-bisect-lk2009.txt
@@ -1103,7 +1103,7 @@ _
Combining test suites, git bisect and other systems together
-We have seen that test suites an git bisect are very powerful when
+We have seen that test suites and git bisect are very powerful when
used together. It can be even more powerful if you can combine them
with other systems.
diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 032613c42..7b157441e 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -68,8 +68,8 @@ imap.tunnel::
to the server. Required when imap.host is not set.
imap.host::
- A URL identifying the server. Use a `imap://` prefix for non-secure
- connections and a `imaps://` prefix for secure connections.
+ A URL identifying the server. Use an `imap://` prefix for non-secure
+ connections and an `imaps://` prefix for secure connections.
Ignored when imap.tunnel is set, but required otherwise.
imap.user::
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index e8dec1b3c..df2b64dbb 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -199,7 +199,7 @@ OPTIONS
.git/NOTES_MERGE_REF symref is updated to the resulting commit.
--abort::
- Abort/reset a in-progress 'git notes merge', i.e. a notes merge
+ Abort/reset an in-progress 'git notes merge', i.e. a notes merge
with conflicts. This simply removes all files related to the
notes merge.
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index c4467ffb9..d9f422d56 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -106,7 +106,7 @@ It is optional: it defaults to 'traditional'.
The possible options are:
+
- 'traditional' - Shows ignored files and directories, unless
- --untracked-files=all is specifed, in which case
+ --untracked-files=all is specified, in which case
individual files in ignored directories are
displayed.
- 'no' - Show no ignored files.
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index
