Re: [PATCH v2 0/4] stripspace: Implement and use --count-lines option

[Matthieu Moy]

Tobias Klauser  writes:

>   - Split patch 2/3 into two patches: patch 2/4 switches git stripspace
> to use parse-options and patch 3/4 introduces the new option.

Much better now.

>   - Implement line counting in cmd_stripbuf() instead of (ab-)using
> strbuf_stripspace() for it.

Also short and sweet, I like it.

>   - Drop -C short option
>   - Correct example command output in documentation.
>   - Adjust commit messages to not include links to the wiki, fully
> describe the motivation in the commit message instead.

Good.

I read the patches again, and the whole series is now

Reviewed-by: Matthieu Moy 

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pull: add angle brackets to usage string

[Ralf Thielow]

Alex Henrie  wrote:
> ---
>  builtin/pull.c   | 2 +-
>  contrib/examples/git-pull.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/pull.c b/builtin/pull.c
> index a39bb0a..bf3fd3f 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -66,7 +66,7 @@ static int parse_opt_rebase(const struct option *opt, const 
> char *arg, int unset
>  }
>  
>  static const char * const pull_usage[] = {
> - N_("git pull [options] [ [...]]"),
> + N_("git pull [] [ [...]]"),
>   NULL
>  };


There seem to be three more places left missing these angle brances
at the usage string.
Junio, feel free to squash this or treat it as a separate patch
on top, if suitable.

-- >8 --
From: Ralf Thielow 
Date: Fri, 16 Oct 2015 19:09:57 +0200
Subject: [PATCH] am, credential-cache: add angle brackets to usage string

Signed-off-by: Ralf Thielow 
---
 builtin/am.c   | 4 ++--
 credential-cache.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 4f77e07..98992cd 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2218,8 +2218,8 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
int in_progress;
 
const char * const usage[] = {
-   N_("git am [options] [(|)...]"),
-   N_("git am [options] (--continue | --skip | --abort)"),
+   N_("git am [] [(|)...]"),
+   N_("git am [] (--continue | --skip | --abort)"),
NULL
};
 
diff --git a/credential-cache.c b/credential-cache.c
index 8689a15..f4afdc6 100644
--- a/credential-cache.c
+++ b/credential-cache.c
@@ -88,7 +88,7 @@ int main(int argc, const char **argv)
int timeout = 900;
const char *op;
const char * const usage[] = {
-   "git credential-cache [options] ",
+   "git credential-cache [] ",
NULL
};
struct option options[] = {
-- 
2.6.1.339.g81d1034


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pull: add angle brackets to usage string

[Junio C Hamano]

Alex Henrie  writes:

> 2015-10-16 10:36 GMT-06:00 Junio C Hamano :
>> Makes sense, as all the other  in the usage string are
>> bracketted.
>>
>> Does it make sense to do this for contrib/examples, which is the
>> historical record, though?
>
> I didn't know that contrib/examples was a historical record. The last
> patch I submitted, b7447679e84ed973430ab19fce87f56857b83068, also
> modified contrib/examples.

Yes, but that fixes historical "mistake", no?

With this, you are breaking historical practice by changing only one
instance to deviate from the then-current practice of saying
'options' without brackets.  It is based on the point of view that
considers anything inside  and a fixed string 'options' are
meant to be replaced by intelligent readers, which is as valid as
the more recent practice to consider only things inside 
are placeholders.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing

[Junio C Hamano]

Tobias Klauser  writes:

> Use parse-options to parse command-line options instead of a
> hand-crafted implementation.
>
> This is a preparatory patch to simplify the introduction of the
> --count-lines option in a follow-up patch.

The second paragraph is probably of much lessor importance than one
thing you forgot to mention: the users can now use a unique prefix
of the option and say "stripspace --comment".

> +enum stripspace_mode {
> + STRIP_DEFAULT = 0,
> + STRIP_COMMENTS,
> + COMMENT_LINES
> +};
>  
>  int cmd_stripspace(int argc, const char **argv, const char *prefix)
>  {
>   struct strbuf buf = STRBUF_INIT;
> - int strip_comments = 0;
> - enum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = 
> STRIP_SPACE;
> -
> - if (argc == 2) {
> - if (!strcmp(argv[1], "-s") ||
> - !strcmp(argv[1], "--strip-comments")) {
> - strip_comments = 1;
> - } else if (!strcmp(argv[1], "-c") ||
> -!strcmp(argv[1], "--comment-lines")) {
> - mode = COMMENT_LINES;
> - } else {
> - mode = INVAL;
> - }
> - } else if (argc > 1) {
> - mode = INVAL;
> - }
> -
> - if (mode == INVAL)
> - usage(usage_msg);

When given "git stripspace -s blorg", we used to set mode to INVAL
and then showed the correct usage.  But we no longer have a check
that corresponds to the old INVAL thing, do we?  Perhaps check argc
to detect presence of an otherwise ignored non-option argument
immediately after parse_options() returns?

> - if (strip_comments || mode == COMMENT_LINES)
> + enum stripspace_mode mode = STRIP_DEFAULT;
> +
> + const struct option options[] = {
> + OPT_CMDMODE('s', "strip-comments", ,
> + N_("skip and remove all lines starting with comment 
> character"),
> + STRIP_COMMENTS),
> + OPT_CMDMODE('c', "comment-lines", ,
> + N_("prepend comment character and blank to each 
> line"),
> + COMMENT_LINES),
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, prefix, options, stripspace_usage,
> +  PARSE_OPT_KEEP_DASHDASH);

What is the point of keep-dashdash here?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option

[Junio C Hamano]

Stefan Beller  writes:

> so maybe
> fetch.recurseSubmoduleJobs
> fetch.submoduleJobs
> fetch.jobs
> fetch.connectionsToUse

"git remote update" is another example that may want to run multiple
independent 'git fetch' in parallel.  I think "When the operation I
ask involves fetching from multiple places, I want N instances of
them to be executed", regardless of the kind of operation ("remote
update" or "submodule update", etc.), would match the end-user's
point of view the best, if you want to give them "set this single
thing to apply to all of them" fallback default.

If you want to give them a finer-grained control, you would need to
differentiate what kind of fetch would use N tasks (as opposed to
other kind of fetch that uses M tasks) and the name would need to
have "submodule" in it for that differentiation.

>> So if you want
>>
>> [submodule]
>> fetchParallel = 16
>> updateParallel = 4
>
> So you would have different settings here for only slightly different things?

I was just showing you that it is _possible_ if you want to give
finer control.  For example, you can define:

 * 'submodule.parallel', if defined gives the values for the
   following more specific ones if they aren't given.

 * 'submodule.fetchParallel' specifies how many tasks are run in
   'fetch --recurse-submodules'.

 * 'submodule.fetchParallel' specifies how many tasks are run in
   'submodule update'.

so that those who want finer controls can, and those who don't can
set a single one to apply to all.

If you want to start with a globally single setting, that is
perfectly fine.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing

[Junio C Hamano]

Junio C Hamano  writes:

>> -if (mode == INVAL)
>> -usage(usage_msg);
>
> When given "git stripspace -s blorg", we used to set mode to INVAL
> and then showed the correct usage.  But we no longer have a check
> that corresponds to the old INVAL thing, do we?  Perhaps check argc
> to detect presence of an otherwise ignored non-option argument
> immediately after parse_options() returns?

Perhaps like this.

diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index ac1ab3d..a8b7a93 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -40,8 +40,9 @@ int cmd_stripspace(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
-   argc = parse_options(argc, argv, prefix, options, stripspace_usage,
-PARSE_OPT_KEEP_DASHDASH);
+   argc = parse_options(argc, argv, prefix, options, stripspace_usage, 0);
+   if (argc)
+   usage_with_options(stripspace_usage, options);
 
if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
git_config(git_default_config, NULL);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] stripspace: Implement --count-lines option

[Junio C Hamano]

Tobias Klauser  writes:

>> So this is your output code, which gives only the number of lines
>> without the cleaned up result.
>
> This should better be a simple printf("%zu\n", lines) I guess?

I think we actively avoid using %z conversion that is only C99.

If you really want to, you could count in size_t and use %lu with
appropriate casting, which I think is what we do in the rest of the
codebase.

For this one, I think it is sufficient to just count in int and
print as int with %d, though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option

[Stefan Beller]

On Mon, Oct 12, 2015 at 4:50 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> There is core.preloadIndex to enable parallel index preload, but
>> that is boolean and not giving fine control to the user. We want to give
>> fine control to the user here I'd assume.
>
> I'd approach this as "fetching multiple submodules at a time", if I
> were deciding its name.
>

so maybe
fetch.recurseSubmoduleJobs
fetch.submoduleJobs
fetch.jobs
fetch.connectionsToUse

Eventually we want to also be parallel in git fetch --all, when using
the latter two
we could reuse these then too, no need to support different options for
fetch --all and fetch --recurseSubmodules.


> So if you want
>
> [submodule]
> fetchParallel = 16
> updateParallel = 4

So you would have different settings here for only slightly different things?
So the series I sent out yesterday evening, would make use of updateParallel
for parallel cloning then instead?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pull: add angle brackets to usage string

[Junio C Hamano]

Ralf Thielow  writes:

> There seem to be three more places left missing these angle brances
> at the usage string.
> Junio, feel free to squash this or treat it as a separate patch
> on top, if suitable.

Thanks. Will queue.

>
> -- >8 --
> From: Ralf Thielow 
> Date: Fri, 16 Oct 2015 19:09:57 +0200
> Subject: [PATCH] am, credential-cache: add angle brackets to usage string
>
> Signed-off-by: Ralf Thielow 
> ---
>  builtin/am.c   | 4 ++--
>  credential-cache.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 4f77e07..98992cd 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -2218,8 +2218,8 @@ int cmd_am(int argc, const char **argv, const char 
> *prefix)
>   int in_progress;
>  
>   const char * const usage[] = {
> - N_("git am [options] [(|)...]"),
> - N_("git am [options] (--continue | --skip | --abort)"),
> + N_("git am [] [(|)...]"),
> + N_("git am [] (--continue | --skip | --abort)"),
>   NULL
>   };
>  
> diff --git a/credential-cache.c b/credential-cache.c
> index 8689a15..f4afdc6 100644
> --- a/credential-cache.c
> +++ b/credential-cache.c
> @@ -88,7 +88,7 @@ int main(int argc, const char **argv)
>   int timeout = 900;
>   const char *op;
>   const char * const usage[] = {
> - "git credential-cache [options] ",
> + "git credential-cache [] ",
>   NULL
>   };
>   struct option options[] = {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usage: do not insist that standard input must come from a file

[Junio C Hamano]

The synopsys text and the usage string of subcommands that read list
of things from the standard input are often shown like this:

git gostak [--distim] < 

This is problematic in a number of ways:

 * The way to use these commands is more often to feed them the
   output from another command, not feed them from a file.

 * Manual pages outside Git, commands that operate on the data read
   from the standard input, e.g "sort", "grep", "sed", etc., are not
   described with such a "< redirection-from-file" in their synopsys
   text.  Our doing so introduces inconsistency.

 * We do not insist on where the output should go, by saying

git gostak [--distim] <  > 

 * As it is our convention to enclose placeholders inside ,
   the redirection operator followed by a placeholder filename
   becomes very hard to read, both in the documentation and in the
   help text.

Let's clean them all up, after making sure that the documentation
clearly describes the modes that take information from the standard
input and what kind of things are expected on the input.

Signed-off-by: Junio C Hamano 
---

 * It is very likely that I missed some commands, as I worked from a
   list of files that showed hits from this:

$ perl -e '
while (<>) {
if (/^SYNOPSIS/../^DESCRIPTION/) {
my $bra = ($_ =~ y//>/);
if ($bra != $ket) {
print "$_\n";
}
}
}' Documentation/*.txt

   An unrelated tangent, but we may want to turn show-index.c to a
   built-in (or perhaps deprecate and remove it).

 Documentation/git-cat-file.txt  |  2 +-
 Documentation/git-check-attr.txt|  4 ++--
 Documentation/git-check-ignore.txt  |  2 +-
 Documentation/git-commit-tree.txt   |  2 +-
 Documentation/git-fmt-merge-msg.txt |  2 +-
 Documentation/git-get-tar-commit-id.txt | 12 +++-
 Documentation/git-hash-object.txt   |  2 +-
 Documentation/git-mktag.txt |  5 +++--
 Documentation/git-patch-id.txt  |  4 +++-
 Documentation/git-show-index.txt|  7 ---
 Documentation/git-show-ref.txt  |  2 +-
 Documentation/git-stripspace.txt|  9 +
 Documentation/git-unpack-objects.txt|  2 +-
 builtin/cat-file.c  |  2 +-
 builtin/check-attr.c|  2 +-
 builtin/check-ignore.c  |  2 +-
 builtin/commit-tree.c   |  2 +-
 builtin/get-tar-commit-id.c |  2 +-
 builtin/hash-object.c   |  2 +-
 builtin/mktag.c |  2 +-
 builtin/patch-id.c  |  2 +-
 builtin/show-ref.c  |  2 +-
 builtin/stripspace.c|  4 ++--
 builtin/unpack-objects.c|  2 +-
 show-index.c|  2 +-
 25 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 3105fc0..eb3d694 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv ) 
-'git cat-file' (--batch | --batch-check) [--follow-symlinks] < 

+'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
 ---
diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 00e2aa2..07aacf2 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git check-attr' [-a | --all | attr...] [--] pathname...
-'git check-attr' --stdin [-z] [-a | --all | attr...] < 
+'git check-attr' --stdin [-z] [-a | --all | attr...]
 
 DESCRIPTION
 ---
@@ -28,7 +28,7 @@ OPTIONS
Consider `.gitattributes` in the index only, ignoring the working tree.
 
 --stdin::
-   Read file names from stdin instead of from the command-line.
+   Read pathnames from stdin instead of from the command-line.
 
 -z::
The output format is modified to be machine-parseable.
diff --git a/Documentation/git-check-ignore.txt 
b/Documentation/git-check-ignore.txt
index e35cd04..149b166 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git check-ignore' [options] pathname...
-'git check-ignore' [options] --stdin < 
+'git check-ignore' [options] --stdin
 
 DESCRIPTION
 ---
diff --git a/Documentation/git-commit-tree.txt 
b/Documentation/git-commit-tree.txt
index a0b5457..48c33d7 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -9,7 +9,7 @@ git-commit-tree - Create a new commit object
 SYNOPSIS
 
 [verse]
-'git commit-tree'  [(-p )...] < changelog
+'git commit-tree'  [(-p )...]
 'git commit-tree' 

Re: [BUG] t7063-status-untracked-cache flaky?

[Junio C Hamano]

David Turner  writes:

> The problem is:
>
> trash directory.t7063-status-untracked-cache$ diff trace trace.expect 
> 3,4c3,4
> < directory invalidation: 1
> < opendir: 1
> ---
>> directory invalidation: 2
>> opendir: 2
>
>
> I can repro on a SSD.
>
> I had to try many times to reproduce (I think even more the second
> time). I just ran the test in a while loop until it failed.
>
> I suspect that the kernel might be a bit slow to update the mtime on the
> directory, but I have not yet been able to repro, and I don't understand
> why it only happens in this one test, since sparseness should be
> completely unrelated.

If you are invalidating your cache based on mtime of the directory,
an operation within mtime granularity can cause you to miss it,
unless you try to be conservative and do the "assume invalid if
racy".  And if you do "assume invalid if racy", the result will
become timing dependent.  You may end up invalidating more than
absolutely necessary, i.e. the test writer may have known that at
least 1 must be definitely stale at that point in the test, but
if another directory was iffy and the code played safe, you would
see one more invalidation than expected (which would lead to
scanning the directory, hence one more opendir).

Is that what is going on here?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git tag --contains now takes a long time

[Jerry Snitselaar]

Is this known and being looked into? I've seen a jump from 12 seconds
to over 9 minutes with running git tag --contains on my kernel repo.


snits ~/dev/linux> git --version
git version 2.6.1.145.gb27dacc

snits ~/dev/linux> time git tag --contains 825fcfc
next-20151012
next-20151013
v4.3-rc5

real9m4.765s
user8m56.157s
sys 0m2.450s



snits ~/dev/linux> git --version
git version 2.5.0.275.gac4cc86

snits ~/dev/linux> time git tag --contains 825fcfc
next-20151012
next-20151013
v4.3-rc5

real0m12.842s
user0m11.536s
sys 0m1.098s



b7cc53e92c806b73e14b03f60c17b7c29e52b4a4 is the first bad commit
commit b7cc53e92c806b73e14b03f60c17b7c29e52b4a4
Author: Karthik Nayak 
Date:   Fri Sep 11 20:36:16 2015 +0530

   tag.c: use 'ref-filter' APIs

   Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting
   and printing of refs. This removes most of the code used in 'tag.c'
   replacing it with calls to the 'ref-filter' library.

   Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'
   to filter out tags based on the options set.

   For printing tags we use 'show_ref_array_item()' function provided by
   'ref-filter'.

   We improve the sorting option provided by 'tag.c' by using the sorting
   options provided by 'ref-filter'. This causes the test 'invalid sort
   parameter on command line' in t7004 to fail, as 'ref-filter' throws an
   error for all sorting fields which are incorrect. The test is changed
   to reflect the same.

   Modify documentation for the same.

   Mentored-by: Christian Couder 
   Mentored-by: Matthieu Moy 
   Signed-off-by: Karthik Nayak 
   Signed-off-by: Junio C Hamano 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usage: do not insist that standard input must come from a file

[Junio C Hamano]

Jonathan Nieder  writes:

>>  --stdin::
>> -Read file names from stdin instead of from the command-line.
>> +Read pathnames from stdin instead of from the command-line.
>
> Here I have to read the description of "-z" to understand that pathnames
> come one per line.  How about
>
>   Read pathnames from stdin, one per line, instead of from the command
>   line.

Thanks, that would be better (and for all the other one-per-line
type of stuff).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/12] git submodule update: Use its own list implementation.

[Junio C Hamano]

Stefan Beller  writes:

> Discussions turned out that we cannot parallelize the whole loop below
> `git submodule--helper list` in `git submodule update`, because some
> changes should be done only one at a time, such as messing up a submodule
> and leave it up to the user to cleanup the conflicted rebase or merge.
>
> The submodules which are need to be cloned however do not expect to create
> problems which require attention by the user one at a time, so we want to
> parallelize that first.
>
> To do so we will start with a literal copy of `git submodule--helper list`
> and port over features gradually.

I am not sure what you mean by this.

Surely, the current implementation of "update" does the fetching and
updating as a single unit of task and iterate over these tasks, and
we would rather want to instead have one iteration of submodules to
do the fetching part (without doing other things that can fail and
have to get attention of the end user), followed by another
iteration that does the "other things", in order to get closer to
the end goal of doing the fetch in parallel and then doing the
remainder one-module-at-a-time sequencially.

I would imagine that the logical first step towards the end goal, if
I understood you correctly, would be to split that single large loop
that does a fetch and other things for a single module in each
iteration into two, one that iterates and fetches all, followed by a
new one that does the checkout/merge.

What I do not understand is why that requires a different kind of
enumerator (unless this is a kind of premature optimization, knowing
that the set of modules iterated by these two loops are slightly
different or something).

>  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> diff --git a/git-submodule.sh b/git-submodule.sh
> index bb8b2c7..d2d80e2 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -656,7 +656,7 @@ cmd_update()
>   fi
>  
>   cloned_modules=
> - git submodule--helper list --prefix "$wt_prefix" "$@" | {
> + git submodule--helper list-or-clone --prefix "$wt_prefix" "$@" | {
>   err=
>   while read mode sha1 stage sm_path
>   do
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usage: do not insist that standard input must come from a file

[Jonathan Nieder]

Junio C Hamano wrote:

> The synopsys text and the usage string of subcommands that read list

nit: s/synopsys/synopsis/

[...]
> +++ b/Documentation/git-cat-file.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  
>  [verse]
>  'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | 
> -p |  | --textconv ) 
> -'git cat-file' (--batch | --batch-check) [--follow-symlinks] < 
> 
> +'git cat-file' (--batch | --batch-check) [--follow-symlinks]

Makes sense.  The BATCH OUTPUT section explains the input format.

[...]
> +++ b/Documentation/git-check-attr.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  
>  [verse]
>  'git check-attr' [-a | --all | attr...] [--] pathname...
> -'git check-attr' --stdin [-z] [-a | --all | attr...] < 
> +'git check-attr' --stdin [-z] [-a | --all | attr...]
>
>  DESCRIPTION
>  ---
> @@ -28,7 +28,7 @@ OPTIONS
>   Consider `.gitattributes` in the index only, ignoring the working tree.
>
>  --stdin::
> - Read file names from stdin instead of from the command-line.
> + Read pathnames from stdin instead of from the command-line.

Here I have to read the description of "-z" to understand that pathnames
come one per line.  How about

Read pathnames from stdin, one per line, instead of from the command
line.

?

[...]
> +++ b/Documentation/git-check-ignore.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  
>  [verse]
>  'git check-ignore' [options] pathname...
> -'git check-ignore' [options] --stdin < 
> +'git check-ignore' [options] --stdin

Likewise.

Read pathnames from stdin, one per line, instead of from the command
line.

[...]
> +++ b/Documentation/git-commit-tree.txt
> @@ -9,7 +9,7 @@ git-commit-tree - Create a new commit object
>  SYNOPSIS
>  
>  [verse]
> -'git commit-tree'  [(-p )...] < changelog
> +'git commit-tree'  [(-p )...]

The DESCRIPTION explains the input --- good.

[...]
> +++ b/Documentation/git-fmt-merge-msg.txt
> @@ -9,7 +9,7 @@ git-fmt-merge-msg - Produce a merge commit message
>  SYNOPSIS
>  
>  [verse]
> -'git fmt-merge-msg' [-m ] [--log[=] | --no-log] 
> <$GIT_DIR/FETCH_HEAD
> +'git fmt-merge-msg' [-m ] [--log[=] | --no-log]

The input format isn't described anywhere, unless I think of looking
at the "-F" option.  More importantly, the expected usage isn't described
anywhere.

Perhaps an EXAMPLES section would help?

EXAMPLE
---

--
$ git fetch origin master
$ git fmt-merge-msg --log <$GIT_DIR/FETCH_HEAD
--

Print a log message describing a merge of the "master" branch from
the "origin" remote.

> --- a/Documentation/git-get-tar-commit-id.txt
> +++ b/Documentation/git-get-tar-commit-id.txt
> @@ -9,17 +9,19 @@ git-get-tar-commit-id - Extract commit ID from an archive 
> created using git-arch
>  SYNOPSIS
>  
>  [verse]
> -'git get-tar-commit-id' < 
> +'git get-tar-commit-id'

Looks sensible.

[...]
> +Read an archive created by 'git archive' from the standard input and

Perhaps 'Reads a tar archive'?  Otherwise I would be tempted to pass in
a zip or tgz file.

[...]
> +++ b/Documentation/git-hash-object.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  
>  [verse]
>  'git hash-object' [-t ] [-w] [--path=|--no-filters] [--stdin 
> [--literally]] [--] ...
> -'git hash-object' [-t ] [-w] --stdin-paths [--no-filters] < 
> 
> +'git hash-object' [-t ] [-w] --stdin-paths [--no-filters]

Has the same ambiguity as the other --stdin option descriptions.

Read pathnames from stdin, one per line, instead of from the command
line.

[...]
> +++ b/Documentation/git-mktag.txt
> @@ -9,7 +9,7 @@ git-mktag - Creates a tag object
> -A tag signature file has a very simple fixed format: four lines of
> +A tag signature file, to be fed from the standard input, has a

s/from the/to this command's/

[...]
> +++ b/Documentation/git-patch-id.txt
> @@ -8,10 +8,12 @@ git-patch-id - Compute unique ID for a patch
[...]
> +Read a patch from the standard input, and compute the patch ID for it.

Punctuation nit: there shouldn't be a comma before "and".

[...]
> +++ b/Documentation/git-show-index.txt
> @@ -9,13 +9,14 @@ git-show-index - Show packed archive index
>  SYNOPSIS
>  
>  [verse]
> -'git show-index' < idx-file
> +'git show-index'
>  
>  
>  DESCRIPTION
>  ---
> -Reads given idx file for packed Git archive created with
> -'git pack-objects' command, and dumps its contents.
> +Read an idx file for packed Git archive created with
> +'git pack-objects' command from the standard input, and
> +dump its contents.

Unrelated nit: this sounds like it's talking about "git archive"
on first reading.  Maybe:

Reads the idx file generated alongside a Git packfile by
'git pack-objects' and dumps a description of the pack's
contents.

[...]
> +++ b/Documentation/git-show-ref.txt
> @@ -11,7 +11,7 @@ SYNOPSIS
>  'git show-ref' [-q|--quiet] [--verify] [--head] [-d|--dereference]
>[-s|--hash[=]] 

Re: [PATCH 03/12] git submodule update: Move branch calculation to where it's needed

[Junio C Hamano]

Stefan Beller  writes:

> The branch variable is used only once so calculate it only when needed.
>
> Signed-off-by: Stefan Beller 
> ---

Makes sense.

>  git-submodule.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index eea27f8..56a0524 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -668,7 +668,6 @@ cmd_update()
>   fi
>   name=$(git submodule--helper name "$sm_path") || exit
>   url=$(git config submodule."$name".url)
> - branch=$(get_submodule_config "$name" branch master)
>   if ! test -z "$update"
>   then
>   update_module=$update
> @@ -718,6 +717,7 @@ Maybe you want to use 'update --init'?")"
>   die "$(eval_gettext "Unable to fetch in 
> submodule path '\$sm_path'")"
>   fi
>   remote_name=$(clear_local_git_env; cd "$sm_path" && 
> get_default_remote)
> + branch=$(get_submodule_config "$name" branch master)
>   sha1=$(clear_local_git_env; cd "$sm_path" &&
>   git rev-parse --verify 
> "${remote_name}/${branch}") ||
>   die "$(eval_gettext "Unable to find current 
> ${remote_name}/${branch} revision in submodule path '\$sm_path'")"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/12] git submodule update: Use its own list implementation.

[Stefan Beller]

On Fri, Oct 16, 2015 at 2:02 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Discussions turned out that we cannot parallelize the whole loop below
>> `git submodule--helper list` in `git submodule update`, because some
>> changes should be done only one at a time, such as messing up a submodule
>> and leave it up to the user to cleanup the conflicted rebase or merge.
>>
>> The submodules which are need to be cloned however do not expect to create
>> problems which require attention by the user one at a time, so we want to
>> parallelize that first.
>>
>> To do so we will start with a literal copy of `git submodule--helper list`
>> and port over features gradually.
>
> I am not sure what you mean by this.
>
> Surely, the current implementation of "update" does the fetching and
> updating as a single unit of task and iterate over these tasks, and
> we would rather want to instead have one iteration of submodules to
> do the fetching part (without doing other things that can fail and
> have to get attention of the end user), followed by another
> iteration that does the "other things", in order to get closer to
> the end goal of doing the fetch in parallel and then doing the
> remainder one-module-at-a-time sequencially.

I differentiated a bit more and moved the clone parts only.
Fetching should also be no problem. I initially assumed that to be a
problem too.

>
> I would imagine that the logical first step towards the end goal, if
> I understood you correctly, would be to split that single large loop
> that does a fetch and other things for a single module in each
> iteration into two, one that iterates and fetches all, followed by a
> new one that does the checkout/merge.

That was also one of the patch series I wrote (not sent to list)
1) split up into 2 phases
2) rewrite first phase in C
3) parallelize the first phase.

This series merges 1 and 2, so you don't have to review
the same functionality two times.

>
> What I do not understand is why that requires a different kind of
> enumerator (unless this is a kind of premature optimization, knowing
> that the set of modules iterated by these two loops are slightly
> different or something).

It is just moving all code before the clone step into the C part, so
we can call the clone in C.

>
>>  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index bb8b2c7..d2d80e2 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -656,7 +656,7 @@ cmd_update()
>>   fi
>>
>>   cloned_modules=
>> - git submodule--helper list --prefix "$wt_prefix" "$@" | {
>> + git submodule--helper list-or-clone --prefix "$wt_prefix" "$@" | {
>>   err=
>>   while read mode sha1 stage sm_path
>>   do
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] t7063-status-untracked-cache flaky?

[David Turner]

On Fri, 2015-10-16 at 17:22 +0200, Torsten Bögershausen wrote:
> On 15.10.15 09:52, Lars Schneider wrote:
> > Hi,
> > 
> > I noticed that "t7063-status-untracked-cache.sh" occasionally fails with 
> > "not ok 24 - test sparse status with untracked cache".
>
> I can't reproduce it here.
> Do you want to give us some info about your setup ?
> OS ?
> Harddisk, SSD, Fusion ?
> Does "debug=t verbose=t ./t7063-status-untracked-cache.sh >xx.txt
2>&1" 
> give any more information ?

(rearranged to bottom-post)

The problem is:

trash directory.t7063-status-untracked-cache$ diff trace trace.expect 
3,4c3,4
< directory invalidation: 1
< opendir: 1
---
> directory invalidation: 2
> opendir: 2


I can repro on a SSD.

I had to try many times to reproduce (I think even more the second
time). I just ran the test in a while loop until it failed.

I suspect that the kernel might be a bit slow to update the mtime on the
directory, but I have not yet been able to repro, and I don't understand
why it only happens in this one test, since sparseness should be
completely unrelated.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/12] git submodule update: Announce skipping submodules on stderr

[Junio C Hamano]

Stefan Beller  writes:

> Signed-off-by: Stefan Beller 
> ---
>  git-submodule.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 8b0eb9a..578ec48 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -684,7 +684,7 @@ cmd_update()
>  
>   if test "$update_module" = "none"
>   then
> - echo "Skipping submodule '$displaypath'"
> + echo >&2 "Skipping submodule '$displaypath'"
>   continue
>   fi

Makes sense, but see 02/12.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/12] git submodule update: Announce skipping submodules on stderr

[Stefan Beller]

On Fri, Oct 16, 2015 at 1:37 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Signed-off-by: Stefan Beller 
>> ---
>>  git-submodule.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 8b0eb9a..578ec48 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -684,7 +684,7 @@ cmd_update()
>>
>>   if test "$update_module" = "none"
>>   then
>> - echo "Skipping submodule '$displaypath'"
>> + echo >&2 "Skipping submodule '$displaypath'"
>>   continue
>>   fi
>
> Makes sense, but see 02/12.

The patch (I can't see a reply there) ?

I split them on purpose. This one uses echo as opposed to say and has
no tests to fail.

So this patch documents, that there are no breaking tests. I can just change it
2/12 tells another story: We codified the behavior in tests and rely on it, so
we need to carefully decide if that's a breaking change.



>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/12] git submodule update: Announce uninitialized modules on stderr

[Junio C Hamano]

Stefan Beller  writes:

> Signed-off-by: Stefan Beller 
> ---
>  git-submodule.sh   |  2 +-
>  t/t7400-submodule-basic.sh | 12 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 578ec48..eea27f8 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -693,7 +693,7 @@ cmd_update()
>   # Only mention uninitialized submodules when its
>   # path have been specified
>   test "$#" != "0" &&
> - say "$(eval_gettext "Submodule path '\$displaypath' not 
> initialized
> + say >&2 "$(eval_gettext "Submodule path '\$displaypath' 
> not initialized
>  Maybe you want to use 'update --init'?")"
>   continue
>   fi

There are quite a few other calls to "say" in this script, and you
are changing only this one to emit it to the standard error output.

My quick eyeballing of the script tells me that most of them, other
than the ones that are used in cmd_status to report the information
that the user asked to be shown on the standard output, are of "Now
I am doing this" kind fo output, which I feel are the same category
as this one that shouldn't be on the standard output.

Another thing (which relates to the one in 01/12) is that not all
output from this command comes from "say".

Perhaps the first thing to do before doing 01/12 is to sift these
messages into types and have them consistently use helpers designed
for different purposes, e.g.

 - a progress, like this one, the one in 01/12, and many other uses
   of "say"; which may want to become e.g. "say_progress".

 - an error or a warning, like "Could not remove working tree", "not
   initialized, maybe you want to do 'init' first?"; which may want
   to become something else e.g. "say_warning".

 - the real output from the program, e.g. output from cmd_status,
   would use yet another, e.g. "printf '%s\n'".

instead of converting each message that you happened to have noticed.

Note that "say" is squelched under GIT_QUIET (i.e. --quiet).  The
former two helpers we would want to make quiet (or for errors we may
not---I don't know offhand).  I do not think of any valid reason why
we want to squelch the output from cmd_status under --quiet; it is
not like the the while loop on the downstream of "list |" pipe tells
some status via its exit code.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] t7063-status-untracked-cache flaky?

[David Turner]

On Fri, 2015-10-16 at 13:34 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > The problem is:
> >
> > trash directory.t7063-status-untracked-cache$ diff trace trace.expect 
> > 3,4c3,4
> > < directory invalidation: 1
> > < opendir: 1
> > ---
> >> directory invalidation: 2
> >> opendir: 2
> >
> >
> > I can repro on a SSD.
> >
> > I had to try many times to reproduce (I think even more the second
> > time). I just ran the test in a while loop until it failed.
> >
> > I suspect that the kernel might be a bit slow to update the mtime on the
> > directory, but I have not yet been able to repro, and I don't understand
> > why it only happens in this one test, since sparseness should be
> > completely unrelated.
> 
> If you are invalidating your cache based on mtime of the directory,
> an operation within mtime granularity can cause you to miss it,
> unless you try to be conservative and do the "assume invalid if
> racy".  And if you do "assume invalid if racy", the result will
> become timing dependent.  You may end up invalidating more than
> absolutely necessary, i.e. the test writer may have known that at
> least 1 must be definitely stale at that point in the test, but
> if another directory was iffy and the code played safe, you would
> see one more invalidation than expected (which would lead to
> scanning the directory, hence one more opendir).
> 
> Is that what is going on here?

The test has a sleep to avoid this.  Looking at it, I think it just
needs one more sleep.  Testing now; if correct, I'll send a patch.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] stripspace: Implement --count-lines option

[Tobias Klauser]

On 2015-10-15 at 18:52:54 +0200, Matthieu Moy  
wrote:
> Tobias Klauser  writes:
> 
> > --- a/Documentation/git-stripspace.txt
> > +++ b/Documentation/git-stripspace.txt
> > @@ -9,7 +9,7 @@ git-stripspace - Remove unnecessary whitespace
> >  SYNOPSIS
> >  
> >  [verse]
> > -'git stripspace' [-s | --strip-comments] < input
> > +'git stripspace' [-s | --strip-comments] [-C | --count-lines] < input
> 
> I'm not sure it's a good idea to introduce a one-letter shortcut (-C).
> In scripts, --count-lines is self-explanatory hence more readable than
> -C (which is even more confusing since "git -C foo stripspace" and "git
> stripspace -C" have totally different meanings. Outside scripts, I'm not
> sure the command would be useful. I'd rather avoid polluting the
> one-letter-option namespace.

Ok, I'll drop the -C. Didn't consider the `git -C stripspace' case, so
that's definitely unwanted.

> > +Use 'git stripspace --count-lines' to obtain:
> > +
> > +-
> > +|5$
> > +-
> 
> In the examples above, I read the | as part of the input (unlike $ which
> is used only to show the end of line). So the | should not be here. I
> don't think you need the $ either, the --count-lines option is no longer
> about trailing whitespaces.

Will drop both | and $. Seems I didn't check the output careful enough,
both don't make sense for this option.

> > +static const char * const usage_msg[] = {
> 
> Stick the * to usage_msg please.

Will change in v2.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 02/26] refs: make repack_without_refs and is_branch public

[Michael Haggerty]

On 10/15/2015 09:46 PM, David Turner wrote:
> is_branch was already non-static, but this patch declares it in the
> header.

The commit message no longer reflects the patch.

> Signed-off-by: Ronnie Sahlberg 
> Signed-off-by: David Turner 
> Signed-off-by: Junio C Hamano 
> ---
>  refs.c | 5 +++--
>  refs.h | 2 ++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index fe71ea0..84abc82 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2816,8 +2816,9 @@ int pack_refs(unsigned int flags)
>  
>  /*
>   * Rewrite the packed-refs file, omitting any refs listed in
> - * 'refnames'. On error, leave packed-refs unchanged, write an error
> - * message to 'err', and return a nonzero value.
> + * 'refnames'. On error, packed-refs will be unchanged, the return
> + * value is nonzero, and a message about the error is written to the
> + * 'err' strbuf.

^^^ ?

It is preferable for docstrings to be written in imperative form, so in
my opinion this is a step backwards...

...literally. Your "new" version comes from an older version of Git; it
was changed in

79e4d8a9b8 repack_without_refs(): make function private (2015-06-22)

to the imperative form.

Assuming you are using `git-format-patch` to prepare your patches, it is
always a good idea to read over the prepared email files before sending
them to the ML, to check for bloopers like this.

>   *
>   * The refs in 'refnames' needn't be sorted. `err` must not be NULL.
>   */
> diff --git a/refs.h b/refs.h
> index 7367a7f..8408bef 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -237,6 +237,8 @@ int pack_refs(unsigned int flags);
>  int verify_refname_available(const char *newname, struct string_list *extra,
>struct string_list *skip, struct strbuf *err);
>  
> +extern int is_branch(const char *refname);
> +
>  /*
>   * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.
>   * REF_NODEREF: act on the ref directly, instead of dereferencing
> 

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fetch: only show "Fetching remote" when verbose mode is enabled

[Paul Wise]

By default when fetching one remote nothing is printed unless there
are changes that need fetching. This makes fetching multiple remotes
be just as verbose as fetching a single remote.

This is needed when fetching multiple repositories using the myrepos
tool in minimal output mode, where myrepos only prints the repository
names when git fetch prints some output. For example in the output below
the cgit and git-remote-* lines would be hidden if git fetch were
silent by default when fetching multiple remotes, since the default
for myrepos is to fetch all remotes for git repositories.

pabs@chianamo ~ $ mr -m fetch
mr fetch: /home/pabs/cgit
Fetching origin

mr fetch: /home/pabs/git
Fetching origin
>From https://github.com/git/git
 - [tag update]  junio-gpg-pub -> junio-gpg-pub
Fetching hg
>From https://github.com/SRabbelier/git
 - [tag update]  junio-gpg-pub -> junio-gpg-pub

mr fetch: /home/pabs/git-remote-bzr
Fetching origin

mr fetch: /home/pabs/git-remote-hg
Fetching origin

Signed-off-by: Paul Wise 
---
 builtin/fetch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9a3869f..fc33667 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1067,7 +1067,7 @@ static int fetch_multiple(struct string_list *list)
for (i = 0; i < list->nr; i++) {
const char *name = list->items[i].string;
argv_array_push(, name);
-   if (verbosity >= 0)
+   if (verbosity >= 1)
printf(_("Fetching %s\n"), name);
if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
error(_("Could not fetch %s"), name);
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Use the alternates of the source repository for dissociating clone

[Alexander Riesen]

Resend. The previous replies didn't make it to git-ml because of html part 
inserted by gmail.


On 10/15/2015 11:59 PM, Junio C Hamano wrote:

Are you talking about making a clone of a repository that was
created with "clone --reference", to borrow from the same
third repository the original is borrowing from?

Yes. Or the alternates file was created manually in the source repository.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] merge: fix cache_entry use-after-free

[David Turner]

On Thu, 2015-10-15 at 13:51 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> >> > +static inline void drop_ce_ref(struct cache_entry *ce)
> >> > +{
> >> > +if (ce != NULL) {
> >> > +assert(ce->ref_count >= 0);
> >> 
> >> Shouldn't this be "> 0" to prevent double frees?
> >
> > No.  If the ref_count is 1, then there is still some reference to the
> > ce.  If it is 0, there is no reference to it, and the next check (< 1)
> > will succeed and the ce will get freed.  
> >
> >> > +if (--ce->ref_count < 1) {
> >> > +free(ce);
> >> > +}
> >> > +}
> >> > +}
> 
> Hmm, but it still feels fuzzy, no?  I cannot tell from the above
> exchange if a ce with ref_count==0 upon entry to this function is
> supposed to have somebody pointing at it, somebody just assigned
> NULL (or another pointer) to the last pointer that was pointing at
> it, or what else.  If the expected calling sequence were:
> 
>   drop_ce_ref(thing->ce);
>   thing->ce = NULL;
> 
> and the original thing->ce were the last reference to the cache
> entry about to be freed, its refcnt is better be 1 not 0.  And when
> this function decrements refcnt down to 0, the cache entry is freed.
> 
> Admittedly, if the original refcnt were 0, with signed refcnt, it
> will decrement to -1 and it will be freed, too, but I do not think
> that is what was intended---refcnt is initialized to 0 upon creating
> an unreferenced cache entry, and set_index_entry() and friends that
> store a pointer to anything calls add_ce_ref(), so I read the code
> as intending to make "0 means no pointer points at it".
> 
> As far as I can tell, the only effect of this assert() that uses >=0
> not >0 is to avoid triggering it on this calling sequence:
> 
> new_ce = new_cache_entry();
> drop_ce_ref(new_ce);
> 
> that is, you create because you _might_ use it, and then later
> decide not to use it (and the free() part wouldn't have worked
> correctly with unsigned refcnt ;-).

You and René are right: there is an off-by-one here.  Will fix and
re-roll.  Thanks.

> By the way, the log message says "During merges, we would previously
> free entries that we no longer need in the destination index.  But
> those entries might also be stored in the dir_entry cache," as if
> this issue were present and waiting to trigger for all merges for
> all people.  Given that Linus does hundreds of merges in a day
> during the merge window (and I do several dozens a day), I am quite
> surprised that nobody noticed this issue.  If there is a more
> specific condition that allows this bug to trigger (e.g. "dir-entry
> cache is used only under such and such conditions") that the log
> message does not talk about to explain why this bug was not seen
> widely, it would be a good thing to add.  It is very puzzling
> otherwise.

We also do dozens or hundreds of merges per day and only saw this quite
rarely.  Interestingly, we could only trigger it with an alternate
hashing function for git's hashmap implementation, and only once a
certain bug in that implementation was *fixed*.  But that was just a
trigger; it was certainly not the cause.  The bug would, in general,
have caused more hash collisions due to worse mixing, but I believe it
is possible that some particular collision would have been present in
the non-bugged version of the code but not in the bugged version.

It may have also had something to do with a case-insensitive filesystem;
we never saw anyone reproduce it on anything but OS X, and even there it
was quite difficult to reproduce.

In short, I don't think we know why the bug was not seen widely.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] strbuf: make stripspace() part of strbuf

[Tobias Klauser]

Thanks for the review.

On 2015-10-15 at 19:36:17 +0200, Junio C Hamano  wrote:
> Tobias Klauser  writes:
> 
> > Rename stripspace() to strbuf_stripspace() and move it to the strbuf
> > module as suggested in [1].
> >
> > Also switch all current users of stripspace() to the new function name
> > and  keep a temporary wrapper inline function for topic branches still
> > using stripspace().
> >
> > [1] 
> > https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#make_.27stripspace.28.29.27_part_of_strbuf
> 
> I think Matthieu already mentioned this, but please explain why it
> is a good idea to do this in your own words here, without forcing
> readers to go to other places to find out.  Giving credit to an
> external page for giving you an inspiration to do something is good,
> but the proposed log message needs to justify itself.  In other
> words, when you are challenged to defend this change, you are not
> allowed to say "SmallProjectIdeas page said it is a good thing to
> do.  I just did it without questioning it." ;-)  Instead you are
> expected to justify it yourself.

Yes, makes sense. I'll adjust the commit message for v2 to justify the
change for itself and move the link below --- as Matthieu suggested.

> > Signed-off-by: Tobias Klauser 
> > ---
> 
> A good rule of thumb to see if it is a good time to do this kind of
> change is to do this:
> 
>   $ git log -p maint..pu | grep stripspace
> 
> which shows only one mention in a context, so this change may cause
> textual conflict with something else somewhere nearby but won't hurt
> other topics in flight.

Ok, thanks for the hint. I'll check again before resubmitting v2.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Philip Oakley]

From: "Johannes Schindelin" 

Hi Francois-Xavier,

On Thu, 15 Oct 2015, Francois-Xavier Le Bail wrote:


On 13/10/2015 15:29, Philip Oakley wrote:

> Thus the only sha1 numbers that could be used are those that are
> within the (possibly implied) instruction sheet (which will list the
> current sha1s that will be converted by rebase to new sha1's).

Yes.


So what happens for commits that are in the pick list but then end up not
being rewritten at all, e.g. when a patch has been applied upstream (and
the --cherry logic did not detect that) and then you end up with a "No
changes to commit"? And what if a patch ends up in merge conflicts and the
user just skips it? And what if the referenced commit is to be picked
*afterwards* due to the commits being reordered?


My policy (bikeshed) for these style of occurrences would be that such 
'disappeared sha1 refs' should be considered as equivalent to a 'merge 
conflict' "known"<>"unknown", and drop the user into the appropriate review 
code path so the user can fix it up.


A sha1 ref can only 'disappear' if it was known before hand, that is, it 
must have been reachable from the tip of the original rebase.


Only those commits between the original rebase tip and its merge-base with 
the destination (e.g. --onto) are candidates for re-write. When taken along 
with the minimum (config) length for a sha1 it should be pretty robust to 
false positives.


In the case of --orphan branch rebasing one does get left and right roots 
for the 'merge-base' which is a particular corner case.




It would appear that the strategy you propose is still too ill-defined to
make for a robust feature.

Ciao,
Johannes

P.S.: The recommended way to refer to a commit is not only using the SHA-1
but also mentioning the one-line, and even the date. That way, even
rebased commits can found most of the time. This is not fool-proof, by
far, of course, but still better than trying to rewrite a SHA-1 and
failing.



In terms of re-writing a quoted --one-line ref, the tool must also be told 
(config option) the few valid quoting commands the user wishes to re-write, 
so that if the sha1 is part of a full quote then the whole quote can be 
replaced by a fresh quote of the updated commit (especially in the --onto 
case).




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 03/26] refs-be-files.c: rename refs to refs-be-files

[Michael Haggerty]

On 10/15/2015 09:46 PM, David Turner wrote:
> From: Ronnie Sahlberg 
> 
> Rename refs.c to refs-be-files.c to indicate that this file now
> holds the implementation for the files based refs backend.
> A smaller portion of the code in this file is backend agnostic and will
> be moved to a a new refs.c file that will hold all the common refs code

"a" is repeated above.

> that is shared across all backends.
> 
> A second reason for first moving all the code to the new file and then
> move the backend agnostic code back to refs.c instead of the other way
> around is because the code that will eventually remain in this new
> refs-be-files.c file is so entangled that it would then be very
> difficult to break the split up into small independent patches/chunks.
> [...]

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] strbuf: make stripspace() part of strbuf

[Tobias Klauser]

On 2015-10-15 at 18:42:23 +0200, Matthieu Moy  
wrote:
> Tobias Klauser  writes:
> 
> > Also switch all current users of stripspace() to the new function name
> > and  keep a temporary wrapper inline function for topic branches still
> > using stripspace().
> 
> Since you have this temporary wrapper, it would have made sense to split
> the patch into 1) move and rename the function, and 2) change the
> callsites to strbuf_stripspace. This way 2) would be absolutely trivial
> to review.
> 
> OTOH, this patch is already easy to review, so you may consider it's OK
> like this.

Ok, in this case will keep the patch as is for v2, but with the adjusted
commit message as indicated in your and Junio's review.

> Reviewed-by: Matthieu Moy 

Will add it to v2, thanks!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] stripspace: Implement --count-lines option

[Tobias Klauser]

On 2015-10-15 at 18:52:54 +0200, Matthieu Moy  
wrote:
> Tobias Klauser  writes:
> > +static const char * const usage_msg[] = {
> 
> Stick the * to usage_msg please.

Just noticed while looking at how other sub-commands define this, the vast
majority use "const char * const" and not "const char const *".

Also it would change the meaning. The following wouldn't produce a
compiler warning:

static const char const *usage_msg[] = { ... };
...
usage_msg[0] = "Foobar"

while

static const char * const usage_msg[] = { ... };
...
usage_msg[0] = "Foobar"

would produce one:

builtin/stripspace.c:36:2: error: assignment of read-only location 
‘usage_msg[0]’

Even though I don't expect anyone to modify usage_msg at runtime I think
it'd be better to stick with the current version.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] stripspace: Implement --count-lines option

[Matthieu Moy]

Tobias Klauser  writes:

> On 2015-10-15 at 18:52:54 +0200, Matthieu Moy  
> wrote:
>> Tobias Klauser  writes:
>> > +static const char * const usage_msg[] = {
>> 
>> Stick the * to usage_msg please.
>
> Just noticed while looking at how other sub-commands define this, the vast
> majority use "const char * const" and not "const char const *".

Oops, I read your code too quickly. We stick the * to variable names
when it follows the star, but I didn't see the "const", sorry.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: submodule: allow submodule directory in gitignore

[Aleksey Komarov]

On 12.10.2015 13:30, Aleksey Komarov wrote:

> Now, I'm trying to add a submodule to my repository, but fail to understand 
> why
> my .gitignore prevents it from being added. I use the following command to 
> check
> if my submodule will be ignored or not:
> 
> $ git add --dry-run --ignore-missing c/

By the way I've just consulted documentation[1].
Can --ignore-missing option be used for checking not already present
directories, in addition to ordinary files?

[1] https://git-scm.com/docs/git-add

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/4] stripspace: Implement and use --count-lines option

[Tobias Klauser]

(1) Move the stripspace() function to the strbuf module adding a prefix
and changing all users accordingly. Also introduce a wrapper in case
any topic branches still depend on the old name.

(2) Switch git stripspace to use parse-options in order to simplify
introducing new command line options (as in the following patch). In
v1 this was folded into patch (3) and is now split out for v2.

(3) Introduce option --count-lines to git stripspace and add the
corresponding documentation and tests.

(4) Change git-rebase--interactive.sh to replace commands like:

git stripspace ... | wc -l

with:

git stripspace --count-lines ...

This patch set implements some of the project ideas around git stripspace
suggested on https://git.wiki.kernel.org/index.php/SmallProjectsIdeas

v1 -> v2:

  - Thanks to Junio and Matthieu for the review.
  - Split patch 2/3 into two patches: patch 2/4 switches git stripspace
to use parse-options and patch 3/4 introduces the new option.
  - Implement line counting in cmd_stripbuf() instead of (ab-)using
strbuf_stripspace() for it.
  - Drop -C short option
  - Correct example command output in documentation.
  - Adjust commit messages to not include links to the wiki, fully
describe the motivation in the commit message instead.

Tobias Klauser (4):
  strbuf: make stripspace() part of strbuf
  stripspace: Use parse-options for command-line parsing
  stripspace: Implement --count-lines option
  git rebase -i: Use newly added --count-lines option for stripspace

 Documentation/git-stripspace.txt |  14 +++-
 builtin/am.c |   2 +-
 builtin/branch.c |   2 +-
 builtin/commit.c |   6 +-
 builtin/merge.c  |   2 +-
 builtin/notes.c  |   6 +-
 builtin/stripspace.c | 137 +--
 builtin/tag.c|   2 +-
 git-rebase--interactive.sh   |   6 +-
 strbuf.c |  66 +++
 strbuf.h |  11 +++-
 t/t0030-stripspace.sh|  36 ++
 12 files changed, 181 insertions(+), 109 deletions(-)

-- 
2.6.1.148.g7927db1


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/4] stripspace: Implement --count-lines option

[Tobias Klauser]

Implement the --count-lines options for git stripspace to be able to
omit calling:

  git stripspace --strip-comments < infile | wc -l

e.g. in git-rebase--interactive.sh. The above command can now be
replaced by:

  git stripspace --strip-comments --count-lines < infile

This will make it easier to port git-rebase--interactive.sh to C later
on.

Furthermore, add the corresponding documentation and tests.

Signed-off-by: Tobias Klauser 
---

Implements the small project idea from
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27

 Documentation/git-stripspace.txt | 14 --
 builtin/stripspace.c | 18 +++---
 t/t0030-stripspace.sh| 36 
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt
index 60328d5..79900b8 100644
--- a/Documentation/git-stripspace.txt
+++ b/Documentation/git-stripspace.txt
@@ -9,8 +9,8 @@ git-stripspace - Remove unnecessary whitespace
 SYNOPSIS
 
 [verse]
-'git stripspace' [-s | --strip-comments] < input
-'git stripspace' [-c | --comment-lines] < input
+'git stripspace' [-s | --strip-comments] [--count-lines] < input
+'git stripspace' [-c | --comment-lines] [--count-lines] < input
 
 DESCRIPTION
 ---
@@ -44,6 +44,10 @@ OPTIONS
be terminated with a newline. On empty lines, only the comment character
will be prepended.
 
+--count-lines::
+   Output the number of resulting lines after stripping. This is equivalent
+   to calling 'git stripspace | wc -l'.
+
 EXAMPLES
 
 
@@ -88,6 +92,12 @@ Use 'git stripspace --strip-comments' to obtain:
 |The end.$
 -
 
+Use 'git stripspace --count-lines' to obtain:
+
+-
+5
+-
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index ac1ab3d..487523f 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -14,8 +14,8 @@ static void comment_lines(struct strbuf *buf)
 }
 
 static const char * const stripspace_usage[] = {
-   N_("git stripspace [-s | --strip-comments] < input"),
-   N_("git stripspace [-c | --comment-lines] < input"),
+   N_("git stripspace [-s | --strip-comments] [--count-lines] < input"),
+   N_("git stripspace [-c | --comment-lines] [--count-lines] < input"),
NULL
 };
 
@@ -29,6 +29,7 @@ int cmd_stripspace(int argc, const char **argv, const char 
*prefix)
 {
struct strbuf buf = STRBUF_INIT;
enum stripspace_mode mode = STRIP_DEFAULT;
+   int count_lines = 0;
 
const struct option options[] = {
OPT_CMDMODE('s', "strip-comments", ,
@@ -37,6 +38,7 @@ int cmd_stripspace(int argc, const char **argv, const char 
*prefix)
OPT_CMDMODE('c', "comment-lines", ,
N_("prepend comment character and blank to each 
line"),
COMMENT_LINES),
+   OPT_BOOL(0, "count-lines", _lines, N_("print line 
count")),
OPT_END()
};
 
@@ -54,7 +56,17 @@ int cmd_stripspace(int argc, const char **argv, const char 
*prefix)
else
comment_lines();
 
-   write_or_die(1, buf.buf, buf.len);
+   if (!count_lines)
+   write_or_die(1, buf.buf, buf.len);
+   else {
+   size_t i, lines;
+
+   for (i = lines = 0; i < buf.len; i++) {
+   if (buf.buf[i] == '\n')
+   lines++;
+   }
+   printf("%zu\n", lines);
+   }
strbuf_release();
return 0;
 }
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index 29e91d8..9c00cb9 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -438,4 +438,40 @@ test_expect_success 'avoid SP-HT sequence in commented 
line' '
test_cmp expect actual
 '
 
+test_expect_success '--count-lines with newline only' '
+   printf "0\n" >expect &&
+   printf "\n" | git stripspace --count-lines >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success '--count-lines with single line' '
+   printf "1\n" >expect &&
+   printf "foo\n" | git stripspace --count-lines >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success '--count-lines with single line preceeded by empty line' '
+   printf "1\n" >expect &&
+   printf "\nfoo" | git stripspace --count-lines >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success '--count-lines with single line followed by empty line' '
+   printf "1\n" >expect &&
+   printf "foo\n\n" | git stripspace --count-lines >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success '--count-lines with multiple lines and consecutive 
newlines' '
+   printf "5\n" >expect &&
+   printf "\none\n\n\nthree\nfour\nfive\n" | git stripspace --count-lines 

[PATCH v2 2/4] stripspace: Use parse-options for command-line parsing

[Tobias Klauser]

Use parse-options to parse command-line options instead of a
hand-crafted implementation.

This is a preparatory patch to simplify the introduction of the
--count-lines option in a follow-up patch.

Signed-off-by: Tobias Klauser 
---
 builtin/stripspace.c | 56 
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index f677093..ac1ab3d 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "parse-options.h"
 #include "strbuf.h"
 
 static void comment_lines(struct strbuf *buf)
@@ -12,41 +13,44 @@ static void comment_lines(struct strbuf *buf)
free(msg);
 }
 
-static const char *usage_msg = "\n"
-"  git stripspace [-s | --strip-comments] < input\n"
-"  git stripspace [-c | --comment-lines] < input";
+static const char * const stripspace_usage[] = {
+   N_("git stripspace [-s | --strip-comments] < input"),
+   N_("git stripspace [-c | --comment-lines] < input"),
+   NULL
+};
+
+enum stripspace_mode {
+   STRIP_DEFAULT = 0,
+   STRIP_COMMENTS,
+   COMMENT_LINES
+};
 
 int cmd_stripspace(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
-   int strip_comments = 0;
-   enum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = 
STRIP_SPACE;
-
-   if (argc == 2) {
-   if (!strcmp(argv[1], "-s") ||
-   !strcmp(argv[1], "--strip-comments")) {
-   strip_comments = 1;
-   } else if (!strcmp(argv[1], "-c") ||
-  !strcmp(argv[1], "--comment-lines")) {
-   mode = COMMENT_LINES;
-   } else {
-   mode = INVAL;
-   }
-   } else if (argc > 1) {
-   mode = INVAL;
-   }
-
-   if (mode == INVAL)
-   usage(usage_msg);
-
-   if (strip_comments || mode == COMMENT_LINES)
+   enum stripspace_mode mode = STRIP_DEFAULT;
+
+   const struct option options[] = {
+   OPT_CMDMODE('s', "strip-comments", ,
+   N_("skip and remove all lines starting with comment 
character"),
+   STRIP_COMMENTS),
+   OPT_CMDMODE('c', "comment-lines", ,
+   N_("prepend comment character and blank to each 
line"),
+   COMMENT_LINES),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options, stripspace_usage,
+PARSE_OPT_KEEP_DASHDASH);
+
+   if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
git_config(git_default_config, NULL);
 
if (strbuf_read(, 0, 1024) < 0)
die_errno("could not read the input");
 
-   if (mode == STRIP_SPACE)
-   strbuf_stripspace(, strip_comments);
+   if (mode == STRIP_DEFAULT || mode == STRIP_COMMENTS)
+   strbuf_stripspace(, mode == STRIP_COMMENTS);
else
comment_lines();
 
-- 
2.6.1.148.g7927db1


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/4] strbuf: make stripspace() part of strbuf

[Tobias Klauser]

Rename stripspace() to strbuf_stripspace() and move it to the strbuf
module. The function is also used in other builtins than stripspace, so
it makes sense to have it in a more generic place. Since it operates on
an strbuf and the function is declared in strbuf.h, move it to strbuf.c
and add the corresponding prefix to its name.

Also switch all current users of stripspace() to the new function name
and keep a temporary wrapper inline function for any topic branches
still using stripspace().

Reviewed-by: Matthieu Moy 
Signed-off-by: Tobias Klauser 
---

Implements the small project idea from
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#make_.27stripspace.28.29.27_part_of_strbuf

 builtin/am.c |  2 +-
 builtin/branch.c |  2 +-
 builtin/commit.c |  6 ++---
 builtin/merge.c  |  2 +-
 builtin/notes.c  |  6 ++---
 builtin/stripspace.c | 69 ++--
 builtin/tag.c|  2 +-
 strbuf.c | 66 +
 strbuf.h | 11 -
 9 files changed, 88 insertions(+), 78 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 3bd4fd7..7b8e11e 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1343,7 +1343,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
strbuf_addstr(, "\n\n");
if (strbuf_read_file(, am_path(state, "msg"), 0) < 0)
die_errno(_("could not read '%s'"), am_path(state, "msg"));
-   stripspace(, 0);
+   strbuf_stripspace(, 0);
 
if (state->signoff)
am_signoff();
diff --git a/builtin/branch.c b/builtin/branch.c
index 01f9530..b99a436 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -592,7 +592,7 @@ static int edit_branch_description(const char *branch_name)
strbuf_release();
return -1;
}
-   stripspace(, 1);
+   strbuf_stripspace(, 1);
 
strbuf_addf(, "branch.%s.description", branch_name);
status = git_config_set(name.buf, buf.len ? buf.buf : NULL);
diff --git a/builtin/commit.c b/builtin/commit.c
index 63772d0..dca09e2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -775,7 +775,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
s->hints = 0;
 
if (clean_message_contents)
-   stripspace(, 0);
+   strbuf_stripspace(, 0);
 
if (signoff)
append_signoff(, ignore_non_trailer(), 0);
@@ -1014,7 +1014,7 @@ static int template_untouched(struct strbuf *sb)
if (!template_file || strbuf_read_file(, template_file, 0) <= 0)
return 0;
 
-   stripspace(, cleanup_mode == CLEANUP_ALL);
+   strbuf_stripspace(, cleanup_mode == CLEANUP_ALL);
if (!skip_prefix(sb->buf, tmpl.buf, ))
start = sb->buf;
strbuf_release();
@@ -1726,7 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
wt_status_truncate_message_at_cut_line();
 
if (cleanup_mode != CLEANUP_NONE)
-   stripspace(, cleanup_mode == CLEANUP_ALL);
+   strbuf_stripspace(, cleanup_mode == CLEANUP_ALL);
if (template_untouched() && !allow_empty_message) {
rollback_index_files();
fprintf(stderr, _("Aborting commit; you did not edit the 
message.\n"));
diff --git a/builtin/merge.c b/builtin/merge.c
index a0edaca..e6741f3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -806,7 +806,7 @@ static void prepare_to_commit(struct commit_list 
*remoteheads)
abort_commit(remoteheads, NULL);
}
read_merge_msg();
-   stripspace(, 0 < option_edit);
+   strbuf_stripspace(, 0 < option_edit);
if (!msg.len)
abort_commit(remoteheads, _("Empty commit message."));
strbuf_release(_msg);
diff --git a/builtin/notes.c b/builtin/notes.c
index 3608c64..bb23d55 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -192,7 +192,7 @@ static void prepare_note_data(const unsigned char *object, 
struct note_data *d,
if (launch_editor(d->edit_path, >buf, NULL)) {
die(_("Please supply the note contents using either -m 
or -F option"));
}
-   stripspace(>buf, 1);
+   strbuf_stripspace(>buf, 1);
}
 }
 
@@ -215,7 +215,7 @@ static int parse_msg_arg(const struct option *opt, const 
char *arg, int unset)
if (d->buf.len)
strbuf_addch(>buf, '\n');
strbuf_addstr(>buf, arg);
-   stripspace(>buf, 0);
+   strbuf_stripspace(>buf, 0);
 
d->given = 1;
return 0;
@@ -232,7 +232,7 @@ static int parse_file_arg(const struct option *opt, const 
char *arg, int unset)
die_errno(_("cannot read '%s'"), arg);
} else if (strbuf_read_file(>buf, arg, 1024) < 0)

[PATCH v2 4/4] git rebase -i: Use newly added --count-lines option for stripspace

[Tobias Klauser]

Use the newly added --count-lines option for 'git stripspace' to count
lines instead of piping the entire output to 'wc -l'.

Signed-off-by: Tobias Klauser 
---

Implements the small project idea from
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27

 git-rebase--interactive.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d65c06e..f80da30 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -120,9 +120,9 @@ mark_action_done () {
sed -e 1q < "$todo" >> "$done"
sed -e 1d < "$todo" >> "$todo".new
mv -f "$todo".new "$todo"
-   new_count=$(git stripspace --strip-comments <"$done" | wc -l)
+   new_count=$(git stripspace --strip-comments --count-lines <"$done")
echo $new_count >"$msgnum"
-   total=$(($new_count + $(git stripspace --strip-comments <"$todo" | wc 
-l)))
+   total=$(($new_count + $(git stripspace --strip-comments --count-lines 
<"$todo")))
echo $total >"$end"
if test "$last_count" != "$new_count"
then
@@ -1243,7 +1243,7 @@ test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
 test -n "$cmd" && add_exec_commands "$todo"
 
-todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
+todocount=$(git stripspace --strip-comments --count-lines <"$todo")
 todocount=${todocount##* }
 
 cat >>"$todo" 

Re: [BUG] t7063-status-untracked-cache flaky?

[Torsten Bögershausen]

I can't reproduce it here.
Do you want to give us some info about your setup ?
OS ?
Harddisk, SSD, Fusion ?
Does "debug=t verbose=t ./t7063-status-untracked-cache.sh >xx.txt 2>&1" 
give any more information ?


On 15.10.15 09:52, Lars Schneider wrote:
> Hi,
> 
> I noticed that "t7063-status-untracked-cache.sh" occasionally fails with "not 
> ok 24 - test sparse status with untracked cache".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] merge: fix cache_entry use-after-free

[Junio C Hamano]

David Turner  writes:

> We also do dozens or hundreds of merges per day and only saw this quite
> rarely.  Interestingly, we could only trigger it with an alternate
> hashing function for git's hashmap implementation, and only once a
> certain bug in that implementation was *fixed*.  But that was just a
> trigger; it was certainly not the cause.  The bug would, in general,
> have caused more hash collisions due to worse mixing, but I believe it
> is possible that some particular collision would have been present in
> the non-bugged version of the code but not in the bugged version.
>
> It may have also had something to do with a case-insensitive filesystem;
> we never saw anyone reproduce it on anything but OS X, and even there it
> was quite difficult to reproduce.
>
> In short, I don't think we know why the bug was not seen widely.

It has been a long time since I looked at name-hash.c and I am fuzzy
on what the four functions (cache|index)_(file|dir)_exists are meant
for, but I have this feeling that the original premise of the patch,
"we may free a ce because we no longer use it in the index, but it
may still need to keep a reference to it in name-hash" may be wrong
in the first place.  The proposed "fix" conceptually feels wrong.

The whole point of the name-hash is so that we can detect collisions
in two names, one of which wants to have a file in one place while
the other wants to have a directory, at the same path in the index.
The pathnames and cache entries registered in the name-hash have to
be the ones that are relevant to the index in quesition.  If a new
ce will be added to the index, the name-hash will have to know about
its path (and that is what CE_HASHED bit is about).  On the other
hand, if you are going to remove an existing ce from the index, its
sub-paths should no longer prevent other cache entries to be there.

E.g. if you have "a/b", it must prevent "A" from entering the index
and the name-hash helps you to do so; when you remove "a/b", then
name-hash must now allow "A" to enter the index.  So "a/b" must be
removed from the name-hash by calling remove_name_hash() and normal
codepaths indeed do so.

I do not doubt the existence of "use-after-free bug" you observed,
but I am not convinced that refcounting is "fixing" the problem; it
smells like papering over a different bug that is the root cause of
the use-after-free.

For example, if we forget to "unhash" a ce from name-hash when we
remove a ce from the index (or after we "hashed" it, expecting to
add it to the index, but in the end decided not to add to the index,
perhaps), we would see a now-freed ce still in the name-hash.
Checking a path against the name-hash in such a state would have to
use the ce->name from that stale ce, which is a use-after-free bug.

In such a situation, isn't the real cause of the bug the fact that
the stale ce that is no longer relevant to the true index contents
still in name-hash?  The refcounting does not fix the fact that a
ce->name of a stale ce that is no longer relevant being used for D/F
collision checking.

I am not saying that I found such a codepath that forgets to unhash,
but from the overall design and purpose of the name-hash subsystem,
anything that deliberately _allows_ a stale ce that does not exist
in the index in there smells like a workaround going in a wrong
direction.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git tag --contains now takes a long time

[Junio C Hamano]

Jerry Snitselaar  writes:

> Is this known and being looked into? I've seen a jump from 12 seconds
> to over 9 minutes with running git tag --contains on my kernel repo.

Thanks for a report.  Yes, it seems that there is a recent
regression on the 'master' branch, not yet in any released version,
and we can observe with a much smaller repository X-<.  E.g.

git tag --contains v2.5.0~12

in git itself seems to have got 10 times slower.


> snits ~/dev/linux> git --version
> git version 2.6.1.145.gb27dacc
>
> snits ~/dev/linux> time git tag --contains 825fcfc
> next-20151012
> next-20151013
> v4.3-rc5
>
> real  9m4.765s
> user  8m56.157s
> sys   0m2.450s
>
>
>
> snits ~/dev/linux> git --version
> git version 2.5.0.275.gac4cc86
>
> snits ~/dev/linux> time git tag --contains 825fcfc
> next-20151012
> next-20151013
> v4.3-rc5
>
> real  0m12.842s
> user  0m11.536s
> sys   0m1.098s
>
>
>
> b7cc53e92c806b73e14b03f60c17b7c29e52b4a4 is the first bad commit
> commit b7cc53e92c806b73e14b03f60c17b7c29e52b4a4
> Author: Karthik Nayak 
> Date:   Fri Sep 11 20:36:16 2015 +0530
>
>tag.c: use 'ref-filter' APIs
>
>Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting
>and printing of refs. This removes most of the code used in 'tag.c'
>replacing it with calls to the 'ref-filter' library.
>
>Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'
>to filter out tags based on the options set.
>
>For printing tags we use 'show_ref_array_item()' function provided by
>'ref-filter'.
>
>We improve the sorting option provided by 'tag.c' by using the sorting
>options provided by 'ref-filter'. This causes the test 'invalid sort
>parameter on command line' in t7004 to fail, as 'ref-filter' throws an
>error for all sorting fields which are incorrect. The test is changed
>to reflect the same.
>
>Modify documentation for the same.
>
>Mentored-by: Christian Couder 
>Mentored-by: Matthieu Moy 
>Signed-off-by: Karthik Nayak 
>Signed-off-by: Junio C Hamano 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usage: do not insist that standard input must come from a file

[Jonathan Nieder]

Junio C Hamano wrote:

> Here is what I prepared based on your review in an interdiff form.

For what it's worth,
Reviewed-by: Jonathan Nieder 

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git v2.6.2

[Junio C Hamano]

The latest maintenance release Git v2.6.2 is now available at
the usual places.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.6.2'
tag and the 'maint' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git



Git v2.6.2 Release Notes


Fixes since v2.6.1
--

 * There were some classes of errors that "git fsck" diagnosed to its
   standard error that did not cause it to exit with non-zero status.

 * A test script for the HTTP service had a timing dependent bug,
   which was fixed.

 * Performance-measurement tests did not work without an installed Git.

 * On a case insensitive filesystems, setting GIT_WORK_TREE variable
   using a random cases that does not agree with what the filesystem
   thinks confused Git that it wasn't inside the working tree.

 * When "git am" was rewritten as a built-in, it stopped paying
   attention to user.signingkey, which was fixed.

 * After "git checkout --detach", "git status" reported a fairly
   useless "HEAD detached at HEAD", instead of saying at which exact
   commit.

 * "git rebase -i" had a minor regression recently, which stopped
   considering a line that begins with an indented '#' in its insn
   sheet not a comment, which is now fixed.

 * Description of the "log.follow" configuration variable in "git log"
   documentation is now also copied to "git config" documentation.

 * Allocation related functions and stdio are unsafe things to call
   inside a signal handler, and indeed killing the pager can cause
   glibc to deadlock waiting on allocation mutex as our signal handler
   tries to free() some data structures in wait_for_pager().  Reduce
   these unsafe calls.

 * The way how --ref/--notes to specify the notes tree reference are
   DWIMmed was not clearly documented.

 * Customization to change the behaviour with "make -w" and "make -s"
   in our Makefile was broken when they were used together.

 * The Makefile always runs the library archiver with hardcoded "crs"
   options, which was inconvenient for exotic platforms on which
   people want to use programs with totally different set of command
   line options.

 * The ssh transport, just like any other transport over the network,
   did not clear GIT_* environment variables, but it is possible to
   use SendEnv and AcceptEnv to leak them to the remote invocation of
   Git, which is not a good idea at all.  Explicitly clear them just
   like we do for the local transport.

 * "git blame --first-parent v1.0..v2.0" was not rejected but did not
   limit the blame to commits on the first parent chain.

 * Very small number of options take a parameter that is optional
   (which is not a great UI element as they can only appear at the end
   of the command line).  Add notice to documentation of each and
   every one of them.

Also contains typofixes, documentation updates and trivial code
clean-ups.



Changes since v2.6.1 are as follows:

Alex Henrie (2):
  merge: grammofix in please-commit-before-merge message
  pull: enclose  in brackets in the usage string

Christian Couder (2):
  quote: fix broken sq_quote_buf() related comment
  quote: move comment before sq_quote_buf()

Eric N. Vander Weele (1):
  log: Update log.follow doc and add to config.txt

Jacob Keller (1):
  notes: correct documentation of DWIMery for notes references

Jeff King (3):
  git_connect: clear GIT_* environment for ssh
  git_connect: clarify conn->use_shell flag
  blame: handle --first-parent

Johannes Schindelin (1):
  setup: fix "inside work tree" detection on case-insensitive filesystems

John Keeping (2):
  Makefile: fix MAKEFLAGS tests with multiple flags
  Documentation: fix section header mark-up

Junio C Hamano (3):
  Makefile: allow $(ARFLAGS) specified from the command line
  fsck: exit with non-zero when problems are found
  Git 2.6.2

Matthieu Moy (7):
  Documentation: use 'keyid' consistently, not 'key-id'
  Documentation/grep: fix documentation of -O
  Documentation: explain optional arguments better
  t3203: test 'detached at' after checkout --detach
  status: don't say 'HEAD detached at HEAD'
  rebase-i: explicitly accept tab as separator in commands
  rebase-i: loosen over-eager check_bad_cmd check

Michael J Gruber (1):
  t2026: rename worktree prune test

Nguyễn Thái Ngọc Duy (1):
  ls-remote.txt: delete unsupported option

Renee Margaret McConahy (1):
  am: configure gpg at startup

Stephan Beyer (2):
  t5561: get rid of 

Re: [PATCH] usage: do not insist that standard input must come from a file

[Junio C Hamano]

Here is what I prepared based on your review in an interdiff form.
A few points to note:

 * Stole the "fmt-merge-msg" example verbatim ;-)

 * The description of "show-ref --exclude" mode should be much
   clearer now.

Thanks.

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 07aacf2..aa3b2bf 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -28,7 +28,8 @@ OPTIONS
Consider `.gitattributes` in the index only, ignoring the working tree.
 
 --stdin::
-   Read pathnames from stdin instead of from the command-line.
+   Read pathnames from the standard input, one per line,
+   instead of from the command-line.
 
 -z::
The output format is modified to be machine-parseable.
diff --git a/Documentation/git-check-ignore.txt 
b/Documentation/git-check-ignore.txt
index 149b166..59531ab 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -35,7 +35,8 @@ OPTIONS
for each given pathname.
 
 --stdin::
-   Read file names from stdin instead of from the command-line.
+   Read pathnames from the standard input, one per line,
+   instead of from the command-line.
 
 -z::
The output format is modified to be machine-parseable (see
diff --git a/Documentation/git-fmt-merge-msg.txt 
b/Documentation/git-fmt-merge-msg.txt
index 9be6df3..6526b17 100644
--- a/Documentation/git-fmt-merge-msg.txt
+++ b/Documentation/git-fmt-merge-msg.txt
@@ -57,6 +57,18 @@ merge.summary::
Synonym to `merge.log`; this is deprecated and will be removed in
the future.
 
+EXAMPLE
+---
+
+--
+$ git fetch origin master
+$ git fmt-merge-msg --log <$GIT_DIR/FETCH_HEAD
+--
+
+Print a log message describing a merge of the "master" branch from
+the "origin" remote.
+
+
 SEE ALSO
 
 linkgit:git-merge[1]
diff --git a/Documentation/git-get-tar-commit-id.txt 
b/Documentation/git-get-tar-commit-id.txt
index 51804b0..ac44d85 100644
--- a/Documentation/git-get-tar-commit-id.txt
+++ b/Documentation/git-get-tar-commit-id.txt
@@ -15,10 +15,10 @@ SYNOPSIS
 DESCRIPTION
 ---
 
-Read an archive created by 'git archive' from the standard input and
-extracts the commit ID stored in it.  It reads only the first 1024
-bytes of input, thus its runtime is not influenced by the size of
-the archive very much.
+Read a tar archive created by 'git archive' from the standard input
+and extract the commit ID stored in it.  It reads only the first
+1024 bytes of input, thus its runtime is not influenced by the size
+of the tar archive very much.
 
 If no commit ID is found, 'git get-tar-commit-id' quietly exists with a
 return code of 1.  This can happen if the archive had not been created
diff --git a/Documentation/git-hash-object.txt 
b/Documentation/git-hash-object.txt
index 45e5ece..814e744 100644
--- a/Documentation/git-hash-object.txt
+++ b/Documentation/git-hash-object.txt
@@ -35,7 +35,8 @@ OPTIONS
Read the object from standard input instead of from a file.
 
 --stdin-paths::
-   Read file names from stdin instead of from the command-line.
+   Read file names from the standard input, one per line, instead
+   of from the command-line.
 
 --path::
Hash object as it were located at the given path. The location of
diff --git a/Documentation/git-mktag.txt b/Documentation/git-mktag.txt
index f4cc202..fa6a756 100644
--- a/Documentation/git-mktag.txt
+++ b/Documentation/git-mktag.txt
@@ -20,8 +20,8 @@ The output is the new tag's  identifier.
 
 Tag Format
 --
-A tag signature file, to be fed from the standard input, has a
-very simple fixed format: four lines of
+A tag signature file, to be fed to this command's standard input,
+has a very simple fixed format: four lines of
 
   object 
   type 
diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt
index 7816479..cf71fba 100644
--- a/Documentation/git-patch-id.txt
+++ b/Documentation/git-patch-id.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-Read a patch from the standard input, and compute the patch ID for it.
+Read a patch from the standard input and compute the patch ID for it.
 
 A "patch ID" is nothing but a sum of SHA-1 of the file diffs associated with a
 patch, with whitespace and line numbers ignored.  As such, it's "reasonably
diff --git a/Documentation/git-show-index.txt b/Documentation/git-show-index.txt
index b3e7488..a8a9509 100644
--- a/Documentation/git-show-index.txt
+++ b/Documentation/git-show-index.txt
@@ -14,7 +14,7 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-Read an idx file for packed Git archive created with
+Read the idx file for a Git packfile created with
 'git pack-objects' command from the standard input, and
 dump its contents.
 
diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index 2c08b08..7b74340 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -23,8 +23,9 @@ particular 

Git bug report: git doesn't change working directory

[Khải]

I'm using Windows 10. Before I install git 2.6.1.windows.1, I have 
installed git 1.9.5.github.0 (by installing GitHub Desktop), it works just 
fine.
But when I installed git 2.6.1.windows.1 (from git-scm.com), I'm not able 
to use git anymore:
 - The powershell console displayed [(unknown)] instead of [master], even 
when I changed working directory to my project, it still display 
[(unknown)]
 - When I "git add" (or "git commit", "git push"), It told me an error: 
"fatal: Not a git repository: 'C:\Program Files\Git'"
After that, I returned to the old version of git - git 1.9.5.github.0, but 
the fault still there.
I think git didn't change its working directory, I have no idea.
Now, I'm not able to work with git.
Please response me as soon as possible, thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pull: add angle brackets to usage string

[Junio C Hamano]

Alex Henrie  writes:

> Signed-off-by: Alex Henrie 
> ---

Makes sense, as all the other  in the usage string are
bracketted.

Does it make sense to do this for contrib/examples, which is the
historical record, though?  The first one I found with

$ less contrib/examples/*

was this:

#!/bin/sh

OPTIONS_KEEPDASHDASH=t
OPTIONS_SPEC="\
git-checkout [options] [] [...]

and the next one (clean) follows the same pattern.

I'd discard the part of the patch for contrib/ and queue.

Thanks.

>  builtin/pull.c   | 2 +-
>  contrib/examples/git-pull.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index a39bb0a..bf3fd3f 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -66,7 +66,7 @@ static int parse_opt_rebase(const struct option *opt, const 
> char *arg, int unset
>  }
>  
>  static const char * const pull_usage[] = {
> - N_("git pull [options] [ [...]]"),
> + N_("git pull [] [ [...]]"),
>   NULL
>  };
>  
> diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> index 6b3a03f..bcf362e 100755
> --- a/contrib/examples/git-pull.sh
> +++ b/contrib/examples/git-pull.sh
> @@ -8,7 +8,7 @@ SUBDIRECTORY_OK=Yes
>  OPTIONS_KEEPDASHDASH=
>  OPTIONS_STUCKLONG=Yes
>  OPTIONS_SPEC="\
> -git pull [options] [ [...]]
> +git pull [] [ [...]]
>  
>  Fetch one or more remote refs and integrate it/them with the current HEAD.
>  --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/4] stripspace: Implement and use --count-lines option

[Junio C Hamano]

Tobias Klauser  writes:

Be consistent with the subjects, please.

>   strbuf: make stripspace() part of strbuf

s/make/make/ ;-)

>   stripspace: Use parse-options for command-line parsing

s/Use/use/

>   stripspace: Implement --count-lines option

s/Implement/implement/

>   git rebase -i: Use newly added --count-lines option for stripspace

s/Use/use/


>  Documentation/git-stripspace.txt |  14 +++-
>  builtin/am.c |   2 +-
>  builtin/branch.c |   2 +-
>  builtin/commit.c |   6 +-
>  builtin/merge.c  |   2 +-
>  builtin/notes.c  |   6 +-
>  builtin/stripspace.c | 137 
> +--
>  builtin/tag.c|   2 +-
>  git-rebase--interactive.sh   |   6 +-
>  strbuf.c |  66 +++
>  strbuf.h |  11 +++-
>  t/t0030-stripspace.sh|  36 ++
>  12 files changed, 181 insertions(+), 109 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pull: add angle brackets to usage string

[Alex Henrie]

2015-10-16 10:36 GMT-06:00 Junio C Hamano :
> Makes sense, as all the other  in the usage string are
> bracketted.
>
> Does it make sense to do this for contrib/examples, which is the
> historical record, though?

I didn't know that contrib/examples was a historical record. The last
patch I submitted, b7447679e84ed973430ab19fce87f56857b83068, also
modified contrib/examples.

-Alex
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html