Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[Junio C Hamano]

"W. Trevor King"  writes:

> On Thu, Oct 12, 2017 at 10:17:51AM +0900, Junio C Hamano wrote:
>> "W. Trevor King"  writes:
>> 
>> > Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
>> > merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
>> > add a --signoff flag, 2017-07-04).
>> 
>> I cannot find a verb in the above.
>
> I'd meant it as either a continuation of the subject line, or with an

Never do that.  The title should be able to stand on its own, and
must not be an early part of incomplete sentence.

> Much nicer, thanks.  I'll add a patch to v2 to make the same change to
> t7614.
> ...
> Sounds good.  I'll add a patch to v2 to make the same change to the
> existing t5521 --allow-unrelated-histories test.

Please don't, unless you are actively working on the features that
they test.  We do not have infinite amount of bandwidth to deal with
changes for the sake of perceived consistency and no other real
gain.

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> Should we document this special case treatment of color.* in -c
>> somewhere?  E.g.
>
> Perhaps, although I'd save that until we actually add the new option
> to "git" potty, which hasn't happened yet, if I were thinking about
> adding some text like that.  Also I'd call that --default-color=always
> or something like that, to avoid having to answer: what are the
> differences between these two --color thing in the following?
>
> git --color=foo cmd --color=bar

I agree that the color.status text in the example doc is unfortunate.
But the surprising thing I found when writing that doc is that
color.status ("git status", "git commit --dry-run") and
color.interactive are the only items that needed it (aside from
color.ui that needed it for those two).  All the other commands that
use color already accept

git cmd --color=bar

color.interactive applies to multiple commands (e.g. "git clean"), so
it would take a little more chasing down to make them all use
OPT__COLOR.

Heading off to sleep, can look more tomorrow.

I don't think we can get around documenting this -c special case
behavior, though.

diff --git i/builtin/commit.c w/builtin/commit.c
index d75b3805ea..fc5b7cd538 100644
--- i/builtin/commit.c
+++ w/builtin/commit.c
@@ -1345,6 +1345,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
struct object_id oid;
static struct option builtin_status_options[] = {
OPT__VERBOSE(, N_("be verbose")),
+   OPT__COLOR(_color, N_("use color")),
OPT_SET_INT('s', "short", _format,
N_("show status concisely"), STATUS_FORMAT_SHORT),
OPT_BOOL('b', "branch", _branch,
@@ -1595,6 +1596,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
static struct option builtin_commit_options[] = {
OPT__QUIET(, N_("suppress summary after successful 
commit")),
OPT__VERBOSE(, N_("show diff in commit message 
template")),
+   OPT__COLOR(_color, N_("use color")),
 
OPT_GROUP(N_("Commit message options")),
OPT_FILENAME('F', "file", , N_("read message from 
file")),

Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

On Thu, Oct 12, 2017 at 10:17:51AM +0900, Junio C Hamano wrote:
> "W. Trevor King"  writes:
> 
> > Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
> > merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
> > add a --signoff flag, 2017-07-04).
> 
> I cannot find a verb in the above.

I'd meant it as either a continuation of the subject line, or with an
implicit leading “I did this…” :p.  I can reword if you like, maybe
just “Following” → “Follow”?  Something more drastic?

> > The order of options in merge-options.txt isn't clear to me, but
> > I've put --signoff between --log and --stat as somewhat
> > alphabetized and having an "add to the commit message" function
> > like --log.
> >
> > The tests aren't as extensive as t7614-merge-signoff.sh, but they
> > exercises both the --signoff and --no-signoff options.  There may
> > be a more efficient way to set them up (like
> > t7614-merge-signoff.sh's test_setup), but with all the pull
> > options packed into a single test script it seemed easiest to just
> > copy/paste the duplicate setup code.
>
> The above two paragraphs read more like "requesting help for hints
> to improve this patch" than commit log message.  Perhaps move them
> below the three-dash line and instead describe what you actually did
> here (if they were worth explaining, that is)?

I think something about merge-options.txt ordering should end up in
the history of that content.  Reading through:

  $ git log Documentation/merge-options.txt

only turned up 690b2975 (Documentation/merge-options.txt: group "ff"
related options together, 2012-02-22) discussing option order (it
suggested grouping similar options together, although --ff and
--ff-only would also be close alphabetically).

I agree that the first paragraph you quote above doesn't have me
coming down firmly in favor of a particular ordering strategy, but I
think having something like it in the Git history will help whoever
ends up giving merge-options.txt a well-defined strategy by showing I
didn't have any strong opinions to account for ;).  Silence can mean
“doesn't have a strong opinion”, but sometimes it means “feels the
choice is so obvious that it doesn't need explicit motivation”.

I'm fine moving the second paragraph you quote below the fold in a v2,
although you're calling for more tests below, and it won't apply
anymore once I've added those :).

> > 09c2cb87 didn't motivate the addition of --allow-unrelated-histories
> > to pull; only citing the reason from e379fdf3 (merge: refuse to create
> > too cool a merge by default, 2016-03-18) gave for *not* including it.
> > I like having both exposed in pull because while the fetch-and-merge
> > approach might be a more popular way to judge "how well they fit
> > together", you can also do that after an optimistic pull.  And in
> > cases where an optimistic pull is likely to succeed, suggesting it is
> > easier to explain to Git newbies than a FETCH_HEAD merge.
> 
> I find this paragraph totally unrelated to what the patch does.
> Save it for the patch you add to pass --allow-unrelated-histories
> given to pull down to underlying merge, perhaps?

09c2cb87 is your commit in master (v2.9.0-rc0~88^2) that is doing just
that.  I haven't gone through the list history to figure out why it
ended up getting landed with its current commit message; “Prepare a
patch to make it a reality, just in case it is needed” sounds more
like it was “here's the code in case folks want it, I'll reroll the
motivation if they do”.  This paragraph was aiming to motivate both
the --signoff pass-through I'm adding here and (retroactively) the
--allow-unrelated-histories pass-through you added there.  I'll add
more context in v2 to try to make that more clear.

> > +   cat >expected <<-EOF &&
> > +   Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e 
> > "s/>.*/>/")
> > +   EOF
> 
>   echo "Signed-off-by: $GIT_COMMITER_NAME <$GIT_COMMITTER_EMAIL>" >expect

Much nicer, thanks.  I'll add a patch to v2 to make the same change to
t7614.

> > +   git init src &&
> > +   (
> > +   cd src &&
> > +   test_commit one
> > +   ) &&
> 
> I suspect somebody will suggest "test_commit -C" ;-)

Sounds good.  I'll add a patch to v2 to make the same change to the
existing t5521 --allow-unrelated-histories test.

> > +   git clone src dst &&
> > +   (
> > +   cd src &&
> > +   test_commit two
> > +   ) &&
> > +   (
> > +   cd dst &&
> > +   git pull --signoff --no-ff &&
> > +   git cat-file commit HEAD | tail -n1 >../actual
> 
> I think it makes it more robust to replace "tail" with "collect all
> the signed-off-by lines" like the other test (below) does.  Perhaps
> have a helper function and use it in both?
> 
>   get_signoff () {
>   git cat-file commit "$1" | sed -n -e '/^Signed-off-by: /p'
>   }
> 
> Some may say "cat-file can fail, and having it on the LHS of a pipe
> hides its 

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Junio C Hamano]

Jonathan Nieder  writes:

> Should we document this special case treatment of color.* in -c
> somewhere?  E.g.

Perhaps, although I'd save that until we actually add the new option
to "git" potty, which hasn't happened yet, if I were thinking about
adding some text like that.  Also I'd call that --default-color=always
or something like that, to avoid having to answer: what are the
differences between these two --color thing in the following?

git --color=foo cmd --color=bar

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Jonathan Nieder]

Junio C Hamano wrote:

[...]
> --- a/color.c
> +++ b/color.c
> @@ -307,8 +307,21 @@ int git_config_colorbool(const char *var, const char 
> *value)
>   if (value) {
>   if (!strcasecmp(value, "never"))
>   return 0;
> - if (!strcasecmp(value, "always"))
> - return var ? GIT_COLOR_AUTO : 1;
> + if (!strcasecmp(value, "always")) {
> + /*
> +  * Command-line options always respect "always".
> +  * Likewise for "-c" config on the command-line.
> +  */
> + if (!var ||
> + current_config_scope() == CONFIG_SCOPE_CMDLINE)
> + return 1;
> +
> + /*
> +  * Otherwise, we're looking at on-disk config;
> +  * downgrade to auto.
> +  */
> + return GIT_COLOR_AUTO;
> + }

Yes, this looks good to me.

Should we document this special case treatment of color.* in -c
somewhere?  E.g.

Signed-off-by: Jonathan Nieder 

diff --git i/Documentation/config.txt w/Documentation/config.txt
index 13ce76d48b..d7bd6b169c 100644
--- i/Documentation/config.txt
+++ w/Documentation/config.txt
@@ -1067,11 +1067,15 @@ clean.requireForce::
-i or -n.   Defaults to true.
 
 color.branch::
-   A boolean to enable/disable color in the output of
-   linkgit:git-branch[1]. May be set to `false` (or `never`) to
-   disable color entirely, `auto` (or `true` or `always`) in which
-   case colors are used only when the output is to a terminal.  If
-   unset, then the value of `color.ui` is used (`auto` by default).
+   When to use color in the output of linkgit:git-branch[1].
+   May be set to `never` (or `false`) to disable color entirely,
+   or `auto` (or `true`) in which case colors are used only when
+   the output is to a terminal.  If unset, then the value of
+   `color.ui` is used (`auto` by default).
++
+The value `always` is a historical synonym for `auto`, except when
+passed on the command line using `-c`, where it is a historical synonym
+for `--color=always`.
 
 color.branch.::
Use customized color for branch coloration. `` is one of
@@ -1084,10 +1088,13 @@ color.diff::
Whether to use ANSI escape sequences to add color to patches.
If this is set to `true` or `auto`, linkgit:git-diff[1],
linkgit:git-log[1], and linkgit:git-show[1] will use color
-   when output is to the terminal. The value `always` is a
-   historical synonym for `auto`.  If unset, then the value of
+   when output is to the terminal. If unset, then the value of
`color.ui` is used (`auto` by default).
 +
+The value `always` is a historical synonym for `auto`, except when
+passed on the command line using `-c`, where it is a historical
+synonym for `--color=always`.
++
 This does not affect linkgit:git-format-patch[1] or the
 'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
 command line with the `--color[=]` option.
@@ -1118,10 +1125,14 @@ color.decorate.::
branches, remote-tracking branches, tags, stash and HEAD, respectively.
 
 color.grep::
-   When set to `always`, always highlight matches.  When `false` (or
+   When to highlight matches using color. When `false` (or
`never`), never.  When set to `true` or `auto`, use color only
when the output is written to the terminal.  If unset, then the
value of `color.ui` is used (`auto` by default).
++
+The value `always` is a historical synonym for `auto`, except when
+passed on the command line using `-c`, where it is a historical synonym
+for `--color=always`.
 
 color.grep.::
Use customized color for grep colorization.  `` specifies which
@@ -1153,9 +1164,11 @@ color.interactive::
When set to `true` or `auto`, use colors for interactive prompts
and displays (such as those used by "git-add --interactive" and
"git-clean --interactive") when the output is to the terminal.
-   When false (or `never`), never show colors. The value `always`
-   is a historical synonym for `auto`.  If unset, then the value of
-   `color.ui` is used (`auto` by default).
+   When false (or `never`), never show colors.
++
+The value `always` is a historical synonym for `auto`, except when
+passed on the command line using `-c`, where it means to use color
+regardless of whether output is to the terminal.
 
 color.interactive.::
Use customized color for 'git add --interactive' and 'git clean
@@ -1168,18 +1181,24 @@ color.pager::
use (default is true).
 
 color.showBranch::
-   A boolean to enable/disable color in the output of
-   linkgit:git-show-branch[1]. May be set to `always`,
-   `false` (or `never`) or `auto` (or `true`), in which case colors are 
used
-   

Re: [PATCH 2/2] color: discourage use of ui.color=always

[Jonathan Nieder]

Junio C Hamano wrote:

> Warn when we read such a configuration from a file, and nudge the
> users to spell them 'auto' instead.
>
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/config.txt | 2 +-
>  color.c  | 7 +++
>  2 files changed, 8 insertions(+), 1 deletion(-)

This warning only kicks in when `always` is being silently downgraded
to `auto`.  It makes sense to me.

Reviewed-by: Jonathan Nieder 

Re: [PATCH] doc: emphasize stash "--keep-index" stashes staged content

[Jonathan Nieder]

Hi,

Robert P. J. Day wrote:

> It's not immediately obvious from the man page that the "--keep-index"
> option still adds the staged content to the stash, so make that
> abundantly clear.
>
> Signed-off-by: Robert P. J. Day 
> ---
>
> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 00f95fee1..037144037 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -68,8 +68,8 @@ entries and working tree files are then rolled back to the 
> state in
>  HEAD only for these files, too, leaving files that do not match the
>  pathspec intact.
>  +
> -If the `--keep-index` option is used, all changes already added to the
> -index are left intact.
> +If the `--keep-index` option is used, all changes already staged in the
> +index are left intact in the index, while still being added to the stash.

Aside from Junio's note about "in the index" vs "in the working tree":

The "Testing partial commits" item in the EXAMPLES section explains
what --keep-index is useful for.  I wonder if some allusion to that
would make the explanation in the OPTIONS section easier to
understand.

Something that I end up still curious about when reading this
description is what will happen when I "git stash pop".  Will it apply
only the changes that were stashed away and removed from the working
tree, or will it apply the changes that were kept in the index, too?
If the latter, why?  Is there some way I can turn that behavior off?

E.g. in the "Testing partial commits" example, it seems like the
natural behavior for "git stash pop" would be just restore the changes
that were removed from the working tree.  That would also match an
assumption of save/push and pop being symmetrical ('inverse
operations').

Is this related to "git stash pop --index"?  I notice that the
EXAMPLES section doesn't give any examples of that option.

Thanks,
Jonathan

Re: [PATCH v2 5/5] Add tests around status handling of ignored arguments

[Junio C Hamano]

Jameson Miller  writes:

> Add tests for status handling of '--ignored=matching` and
> `--untracked-files=normal`.
>
> Signed-off-by: Jameson Miller 
> ---

Hmph, having some tests in 3/5, changes in 4/5 and even more tests
in 5/5 makes me as a reader a bit confused, as the description for
these two test patches does not make it clear how they are related
and how they are different.  Is it that changes in 1/5 alone does
not fulfill the promise made by documentation added at 2/5 so 3/5
only has tests for behaviour that works with 1/5 alone but is broken
with respect to what 2/5 claims until 4/5 is applied, and these "not
working with 1/5 alone, but works after 4/5" are added in this step?

>  t/t7519-ignored-mode.sh | 60 
> -
>  1 file changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
> index 76e91427b0..6be7701d79 100755
> --- a/t/t7519-ignored-mode.sh
> +++ b/t/t7519-ignored-mode.sh
> @@ -116,10 +116,68 @@ test_expect_success 'Verify status behavior on ignored 
> folder containing tracked
>   ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
>   ignored_dir/tracked &&
>   git add -f ignored_dir/tracked &&
> - test_tick &&
>   git commit -m "Force add file in ignored directory" &&
>   git status --porcelain=v2 --ignored=matching --untracked-files=all 
> >output &&
>   test_i18ncmp expect output
>  '
>  
> +test_expect_success 'Verify matching ignored files with 
> --untracked-files=normal' '
> + test_when_finished "git clean -fdx" &&
> + cat >expect <<-\EOF &&
> + ? expect
> + ? output
> + ? untracked_dir/
> + ! ignored_dir/
> + ! ignored_files/ignored_1.ign
> + ! ignored_files/ignored_2.ign
> + EOF
> +
> + mkdir ignored_dir ignored_files untracked_dir &&
> + touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
> + ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
> + untracked_dir/untracked &&
> + git status --porcelain=v2 --ignored=matching --untracked-files=normal 
> >output &&
> + test_i18ncmp expect output
> +'
> +
> +test_expect_success 'Verify matching ignored files with 
> --untracked-files=normal' '
> + test_when_finished "git clean -fdx" &&
> + cat >expect <<-\EOF &&
> + ? expect
> + ? output
> + ? untracked_dir/
> + ! ignored_dir/
> + ! ignored_files/ignored_1.ign
> + ! ignored_files/ignored_2.ign
> + EOF
> +
> + mkdir ignored_dir ignored_files untracked_dir &&
> + touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
> + ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
> + untracked_dir/untracked &&
> + git status --porcelain=v2 --ignored=matching --untracked-files=normal 
> >output &&
> + test_i18ncmp expect output
> +'
> +
> +test_expect_success 'Verify status behavior on ignored folder containing 
> tracked file' '
> + test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
> + cat >expect <<-\EOF &&
> + ? expect
> + ? output
> + ! ignored_dir/ignored_1
> + ! ignored_dir/ignored_1.ign
> + ! ignored_dir/ignored_2
> + ! ignored_dir/ignored_2.ign
> + EOF
> +
> + mkdir ignored_dir &&
> + touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
> + ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
> + ignored_dir/tracked &&
> + git add -f ignored_dir/tracked &&
> + git commit -m "Force add file in ignored directory" &&
> + git status --porcelain=v2 --ignored=matching --untracked-files=normal 
> >output &&
> + test_i18ncmp expect output
> +'
> +
>  test_done

Re: [PATCH v2 4/5] Expand support for ignored arguments on status

[Junio C Hamano]

Jameson Miller  writes:

> Teach status command to handle matching ignored mode when showing
> untracked files with the normal option.
>
> Signed-off-by: Jameson Miller 
> ---
>  dir.c | 20 ++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index b9af87eca9..8636d080b2 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct 
> dir_struct *dir,
>  {
>   int exclude;
>   int has_path_in_index = !!index_file_exists(istate, path->buf, 
> path->len, ignore_case);
> + enum path_treatment path_treatment;
>  
>   if (dtype == DT_UNKNOWN)
>   dtype = get_dtype(de, istate, path->buf, path->len);
> @@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct 
> dir_struct *dir,
>   return path_none;
>   case DT_DIR:
>   strbuf_addch(path, '/');
> - return treat_directory(dir, istate, untracked, path->buf, 
> path->len,
> -baselen, exclude, pathspec);
> + path_treatment = treat_directory(dir, istate, untracked,
> +  path->buf, path->len,
> +  baselen, exclude, pathspec);
> + /*
> +  * If we are only want to return directories that
> +  * match an exclude pattern, and this directories does

s/are //; s/directories/directory/

> +  * not match an exclude pattern but all contents are
> +  * excluded, then indicate that we should recurse into
> +  * this directory (instead of marking the directory
> +  * itself as an ignored path)
> +  */
> + if (!exclude &&
> + path_treatment == path_excluded &&
> + (dir->flags & DIR_SHOW_IGNORED_TOO) &&
> + (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
> + return path_recurse;
> + return path_treatment;

The required change to the code is surprisingly small ;-) and it is
well explained in the comment.  Good job.


>   case DT_REG:
>   case DT_LNK:
>   return exclude ? path_excluded : path_untracked;

Re: [PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[小川恭史]

As you point,

git stash

without any argument is equivalent to both of

git stash save
git stash push

. The original sentence is correct.

Re: [PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[小川恭史]

As you point,

git stash

without any argument is equivalent to both of

git stash save
git stash push

. The original sentence is correct.

2017-10-12 12:31 GMT+09:00 小川恭史 :
> As you point,
>
> git stash
>
> without any argument is equivalent to both of
> git stash save and
>
>
> 2017-10-12 9:53 GMT+09:00 Junio C Hamano :
>> Takahito Ogawa  writes:
>>
>>> @@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD`
>>> commit.
>>>  The modifications stashed away by this command can be listed with
>>>  `git stash list`, inspected with `git stash show`, and restored
>>>  (potentially on top of a different commit) with `git stash apply`.
>>> -Calling `git stash` without any arguments is equivalent to `git stash
>>> save`.
>>> +Calling `git stash` without any arguments is equivalent to `git stash
>>> push`.
>>
>> Hmph.  Is there any difference between
>>
>> git stash save
>> git stash push
>>
>> without any other argument?  Aren't they equivalent to
>>
>> git stash
>>
>> without any argument, which is what this sentence explains?
>>
>

Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option

[Junio C Hamano]

Christian Couder  writes:

>> Can somebody explain what is going on?
>>
>> I am guessing that Thais and marius are different people (judging by
>> the fact that one CC's a message to the other).  Are you two
>> collaborating on this change, or something?
>
> I guess that Thais decided to work on this, because we ask Outreachy
> applicants to search for #leftoverbits mentions in the mailing list
> archive to find small tasks they could work on.
>
> In this case it looks like Marius sent a patch a few hours before
> Thais also sent one.

... after seeing Marius's already working on it, I think.

> Thais, I am sorry, but as Marius sent a patch first, I think it is
> better if you search for another different small task to work on.

In general, I do not mind seeing people working together well, and
it is one of the more important skills necessary in the open source
community.  I however tend to agree with you that this is a bit too
small a topic for multiple people to be working on.

> Also please keep Peff and me in cc.

Yup, that is always a good idea.

Re: git repack leaks disk space on ENOSPC

[Jonathan Nieder]

Hi Andreas,

Andreas Krey wrote:

> I observed (again) an annoying behavior of 'git repack':

Do you have context for this 'again'?  E.g. was this discussed
previously on-list?

> When the new pack file cannot be fully written because
> the disk gets full beforehand, the tmp_pack file isn't
> deleted, meaning the disk stays full:
>
>   $ df -h .; git repack -ad; df -h .; ls -lart .git/objects/pack/tmp*; rm 
> .git/objects/pack/tmp*; df -h .
>   FilesystemSize  Used Avail Use% Mounted on
>   /dev/mapper/vg02-localworkspaces  250G  245G  5.1G  98% /workspaces/calvin
>   Counting objects: 4715349, done.
>   Delta compression using up to 8 threads.
>   Compressing objects: 100% (978051/978051), done.
>   fatal: sha1 file '.git/objects/pack/tmp_pack_xB7DMt' write error: No space 
> left on device
>   FilesystemSize  Used Avail Use% Mounted on
>   /dev/mapper/vg02-localworkspaces  250G  250G   20K 100% /workspaces/calvin
>   -r--r--r-- 1 andrkrey users 5438435328 Oct 11 17:03 
> .git/objects/pack/tmp_pack_xB7DMt
>   rm: remove write-protected regular file 
> '.git/objects/pack/tmp_pack_xB7DMt'? y
>   FilesystemSize  Used Avail Use% Mounted on
>   /dev/mapper/vg02-localworkspaces  250G  245G  5.1G  98% /workspaces/calvin
>
> git version 2.15.0.rc0

I can imagine this behavior of retaining tmp_pack being useful for
debugging in some circumstances, but I agree with you that it is
certainly not a good default.

Chasing this down, I find:

  pack-write.c::create_tmp_packfile chooses the filename
  builtin/pack-objects.c::write_pack_file writes to it and the .bitmap, calling
  pack-write.c::finish_tmp_packfile to rename it into place

Nothing tries to install an atexit handler to do anything special to it
on exit.

The natural thing, I'd expect, would be for pack-write to use the
tempfile API (see tempfile.h) to create and finish the file.  That way,
we'd get such atexit handlers for free.  If we want a way to keep temp
files for debugging on abnormal exit, we could set that up separately as
a generic feature of the tempfile API (e.g. an envvar
GIT_KEEP_TEMPFILES_ON_FAILURE), making that an orthogonal topic.

Does using create_tempfile there seem like a good path forward to you?
Would you be interested in working on it (either writing a patch with
such a fix or a test in t/ to make sure it keeps working)?

Thanks,
Jonathan

Re: Unable to use --patch with git add

[Jonathan Nieder]

Hi Ayush,

Ayush Goel wrote:

> Thank you for your mail. It works :)
>
> For future reference, is there a page for known issues of git?

We usually try not to have known issues that would require such a
warning for long.  And when we fail, reminders like yours are very
welcome, though a search through the mailing list archive for an
existing thread to reply to instead of starting a new one is always
welcome.

Sorry for the trouble.

Sincerely,
Jonathan

Re: [PATCH v2 2/5] Update documentation for new directory and status logic

[Junio C Hamano]

Jameson Miller  writes:

> Signed-off-by: Jameson Miller 
> ---
>  Documentation/git-status.txt  | 21 +-
>  Documentation/technical/api-directory-listing.txt | 27 
> +++
>  2 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 9f3a78a36c..fc282e0a92 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
>   (and suppresses the output of submodule summaries when the config option
>   `status.submoduleSummary` is set).
>  
> ---ignored::
> +--ignored[=]::
>   Show ignored files as well.
> ++
> +The mode parameter is used to specify the handling of ignored files.
> +It is optional: it defaults to 'traditional'.
> ++
> +The possible options are:
> ++
> + - 'traditional' - Shows ignored files and directories, unless
> +   --untracked-files=all is specifed, in which case
> +   individual files in ignored directories are
> +   displayed.
> + - 'no'  - Show no ignored files.
> + - 'matching'- Shows ignored files and directories matching an
> +   ignore pattern.
> ++
> +When 'matching' mode is specified, paths that explicity match an
> +ignored pattern are shown. If a directory matches an ignore pattern,
> +then it is shown, but not paths contained in the ignored directory. If
> +a directory does not match an ignore pattern, but all contents are
> +ignored, then the directory is not shown, but all contents are shown.

Well explained.

> diff --git a/Documentation/technical/api-directory-listing.txt 
> b/Documentation/technical/api-directory-listing.txt
> index 6c77b4920c..7fae00f44f 100644
> --- a/Documentation/technical/api-directory-listing.txt
> +++ b/Documentation/technical/api-directory-listing.txt
> @@ -22,16 +22,20 @@ The notable options are:
>  
>  `flags`::
>  
> - A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
> + A bit-field of options:
>  
>  `DIR_SHOW_IGNORED`:::
>  
> - Return just ignored files in `entries[]`, not untracked files.
> + Return just ignored files in `entries[]`, not untracked
> + files. This flag is mutually exclusive with
> + `DIR_SHOW_IGNORED_TOO`.
>  
>  `DIR_SHOW_IGNORED_TOO`:::
>  
> - Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
> - in addition to untracked files in `entries[]`.
> + Similar to `DIR_SHOW_IGNORED`, but return ignored files in
> + `ignored[]` in addition to untracked files in
> + `entries[]`. This flag is mutually exclusive with
> + `DIR_SHOW_IGNORED`.
>  
>  `DIR_KEEP_UNTRACKED_CONTENTS`:::
>  
> @@ -39,6 +43,21 @@ The notable options are:
>   untracked contents of untracked directories are also returned in
>   `entries[]`.
>  
> +`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
> +
> + Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
> + this is set, returns ignored files and directories that match
> + an exclude pattern. If a directory matches an exclude pattern,
> + then the directory is returned and the contained paths are
> + not. A directory that does not match an exclude pattern will
> + not be returned even if all of its contents are ignored. In
> + this case, the contents are returned as individual entries.
> ++
> +If this is set, files and directories that explicity match an ignore
> +pattern are reported. Implicity ignored directories (directories that
> +do not match an ignore pattern, but whose contents are all ignored)
> +are not reported, instead all of the contents are reported.

Makes me wonder if DIR_SHOW_IGNORED* should be splt out into a short
enum.  We have:

 - Do not show ignored ones (0)

 - Collect ignored ones (DIR_SHOW_IGNORED)

 - Collect ignored and untracked ones separately (DIR_SHOW_IGNORED_TOO)

 - Collect ignored and duntracked ones separately, but limit them to
   those mach exclude patterns explicitly 
(DIR_SHOW_IGNORED_TOO|...MODE_MATCHING)

so we need two bits to fit a 4-possiblity enum.

Then we do not have to worry about saying quirky things like A and B
are incompatible, and C makes sense only when B is set, etc.

>  `DIR_COLLECT_IGNORED`:::
>  
>   Special mode for git-add. Return ignored files in `ignored[]` and

Re: [PATCH v2 1/5] Teach status options around showing ignored files

[Junio C Hamano]

Jameson Miller  writes:

> This change teaches the status command more options to control which
> ignored files are reported independently of the which untracked files
> are reported by allowing the `--ignored` option to take additional
> arguments.  Currently, the shown ignored files are linked to how
> untracked files are reported. Both ignored and untracked files are
> controlled by arguments to `--untracked-files` option. This makes it
> impossible to show all untracked files individually, but show ignored
> "files and directories" (that is, ignored directories are shown as 1
> entry, instead of having all contents shown).

The description makes sense.  And it makes sense to show a directory
known to contain only ignored paths as just one entry, instead of
exploding that to individual files.

> Our application (Visual Studio) has a specific set of requirements
> about how it wants untracked / ignored files reported by git status.

This sentence does not read well.  VS has no obligation to read from
"git status", so there is no "specific set of requirements" that
make us care.  If the output from "status" is insufficient you could
be reading from "ls-files --ignored", for example, if you want more
details than "git status" gives you.

The sentence, and the paragraph that immediately follows it, need a
serious attitude adjustment ;-), even though it is good as read as
an explanation of what the proposed output wants to show.

> The reason for controlling these behaviors separately is that there
> can be a significant performance impact to scanning the contents of
> excluded directories. Additionally, knowing whether a directory
> explicitly matches an exclude pattern can help the application make
> decisions about how to handle the directory. If an ignored directory
> itself matches an exclude pattern, then the application will know that
> any files underneath the directory must be ignored as well.

While the above description taken standalone makes sense, doesn't
the "we want all paths listed, without abbreviated to the directory
and requiring the reader to infer from the fact that aidrectory is
shown that everything in it are ignored" the log message stated
earlier contradict another change you earlier sent, that avoids
scanning a directory that is known to be completely untracked
(i.e. no path under it in the index) and return early once an
untracked file is found in it?

> Signed-off-by: Jameson Miller 
> ---
>  builtin/commit.c | 31 +--
>  dir.c| 24 
>  dir.h|  3 ++-
>  wt-status.c  | 11 ---
>  wt-status.h  |  8 +++-
>  5 files changed, 66 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index d75b3805ea..98d84d0277 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
>  static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
>  static int config_commit_verbose = -1; /* unspecified */
>  static int no_post_rewrite, allow_empty_message;
> -static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
> +static char *untracked_files_arg, *force_date, *ignore_submodule_arg, 
> *ignored_arg;
>  static char *sign_commit;
>  
>  /*
> @@ -139,7 +139,7 @@ static const char *cleanup_arg;
>  static enum commit_whence whence;
>  static int sequencer_in_use;
>  static int use_editor = 1, include_status = 1;
> -static int show_ignored_in_status, have_option_m;
> +static int have_option_m;
>  static struct strbuf message = STRBUF_INIT;
>  
>  static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
> @@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char 
> *name)
>   die(_("--author '%s' is not 'Name ' and matches no existing 
> author"), name);
>  }
>  
> +static void handle_ignored_arg(struct wt_status *s)
> +{
> + if (!ignored_arg)
> + ; /* default already initialized */
> + else if (!strcmp(ignored_arg, "traditional"))
> + s->show_ignored_mode = SHOW_TRADITIONAL_IGNORED;
> + else if (!strcmp(ignored_arg, "no"))
> + s->show_ignored_mode = SHOW_NO_IGNORED;
> + else if (!strcmp(ignored_arg, "matching"))
> + s->show_ignored_mode = SHOW_MATCHING_IGNORED;
> + else
> + die(_("Invalid ignored mode '%s'"), ignored_arg);
> +}
>  
>  static void handle_untracked_files_arg(struct wt_status *s)
>  {
> @@ -1363,8 +1376,10 @@ int cmd_status(int argc, const char **argv, const char 
> *prefix)
> N_("mode"),
> N_("show untracked files, optional modes: all, normal, no. 
> (Default: all)"),
> PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
> - OPT_BOOL(0, "ignored", _ignored_in_status,
> -  N_("show ignored files")),
> + { OPTION_STRING, 0, 

Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option

[Christian Couder]

On Thu, Oct 12, 2017 at 3:21 AM, Junio C Hamano  wrote:
> "Thais D. Braz"  writes:
>
>> ---
>>  Documentation/git-push.txt | 3 +++
>>  builtin/push.c | 9 -
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> Can somebody explain what is going on?
>
> I am guessing that Thais and marius are different people (judging by
> the fact that one CC's a message to the other).  Are you two
> collaborating on this change, or something?

I guess that Thais decided to work on this, because we ask Outreachy
applicants to search for #leftoverbits mentions in the mailing list
archive to find small tasks they could work on.

In this case it looks like Marius sent a patch a few hours before
Thais also sent one.

Thais, I am sorry, but as Marius sent a patch first, I think it is
better if you search for another different small task to work on.
Also please keep Peff and me in cc.

Thanks.

Re: [PATCH v2 0/5] Teach Status options around showing ignored files

[Junio C Hamano]

Jameson Miller  writes:

> Changes in v2:
>
> Addressed review comments from the original series:
>
> * Made fixes / simplifications suggested for documentation
>
> * Removed test_tick from test scripts
>
> * Merged 2 commits (as suggested)
>
> Jameson Miller (5):
>   Teach status options around showing ignored files
>   Update documentation for new directory and status logic
>   Add tests for git status `--ignored=matching`
>   Expand support for ignored arguments on status
>   Add tests around status handling of ignored arguments

Please make sure these titles mix well when they appear together
with changes from other people that are completely unrelated to
them.  Keep in mind that your "git status" is *not* the most
important thing in the world (of course, it is no less important
than others, either).  Perhaps

status: add new options to show ignored files differently
status: document logic to show new directory
status: test --ignored=matching

etc.  Rules of thumb:

 (1) choose ": " prefix appropriately
 (2) keep them short and to the point
 (3) word that follow ": " prefix is not capitalized
 (4) no need for full-stop at the end of the title

[PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Junio C Hamano]

From: Jeff King 

An earlier patch downgraded "always" that comes via the ui.color
configuration variable to "auto", in order to work around an
unfortunate regression to "git add -i".

That "fix" however regressed other third-party tools that depend on
"git -c color.ui=always cmd" as the way to defeat any end-user
configuration and force coloured output from git subcommand, even
when the output does not go to a terminal.

It is a bit ugly to treat "-c color.ui=always" from the command line
differently from a definition that comes from on-disk configuration
files, but it is a moral equivalent of "--color=always" option given
to the subcommand from the command line, i.e. a signal that tells us
that the script writer knows what s/he is doing.  So let's take that
route to unbreak this case while defeating a (now declared to be)
misguided color.ui that is set to always in the configuration file.

NEEDS-SIGN-OFF-BY: Jeff King 
Signed-off-by: Junio C Hamano 
---
 color.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/color.c b/color.c
index 17e2713f96..5b06c76bdc 100644
--- a/color.c
+++ b/color.c
@@ -307,8 +307,21 @@ int git_config_colorbool(const char *var, const char 
*value)
if (value) {
if (!strcasecmp(value, "never"))
return 0;
-   if (!strcasecmp(value, "always"))
-   return var ? GIT_COLOR_AUTO : 1;
+   if (!strcasecmp(value, "always")) {
+   /*
+* Command-line options always respect "always".
+* Likewise for "-c" config on the command-line.
+*/
+   if (!var ||
+   current_config_scope() == CONFIG_SCOPE_CMDLINE)
+   return 1;
+
+   /*
+* Otherwise, we're looking at on-disk config;
+* downgrade to auto.
+*/
+   return GIT_COLOR_AUTO;
+   }
if (!strcasecmp(value, "auto"))
return GIT_COLOR_AUTO;
}
-- 
2.15.0-rc1-151-g44fe2f342f

[PATCH 0/2] Piling more kludge on top of color.ui

[Junio C Hamano]

Earlier we added a patch to unconditionally downgrade 'always' that
is set to the color.ui configuration variable.  This was done to
correc the unintended regression to "git add -i" that was caused by
two earlier mistakes that we no longer can undo.

 - The "add -i" command wants to parse output from "git diff-index"
   plumbing.  The plumbing commands started paying attention to
   color configuration variables (which is a mistaken "solution" to
   cover another mistake), which caused people who have color.ui set
   to "always" to see breakage, as their "git diff-index" started
   coloring its output even when "git add -i" wanted to read it and
   parse it (without expecting to see any colors in its input);

 - The mistake the mistaken "solution" wanted to cover was that some
   time ago we started to automatically color the output (i.e. color
   when the output goes to the terminal) by default, but did so even
   to the plumbing commands.  As many plumbing commands do not even
   have their own color control, it made it impossible to disable
   this auto-coloring--a mistaken "solution" was to pay attention to
   "git -c color.ui=never" from the command line.

It turns out that there are many third-party scripts that do want to
read colored output from our tools and the way they do so is to run
"git -c color.ui=always cmd", which is a way to defeat any end-user
settings and force coloured output reliably---at least they thought
that they can rely on it working, that is.  We saw one report of
such a private tool getting broken on list, and I've seen another
one inside $work.

Let's keep "git -c color.ui=always cmd" form "working", while
downgrading the setting made in the configuration files to "auto",
to placate both camps.  Let's also discourage use of 'always' and
leave the door open for us to introduce "git --default-color=
cmd" later as a substitute for "git -c color.ui=".

Jeff King (1):
  color: downgrade "always" to "auto" only for on-disk configuration

Junio C Hamano (1):
  color: discourage use of ui.color=always

 Documentation/config.txt |  2 +-
 color.c  | 24 ++--
 2 files changed, 23 insertions(+), 3 deletions(-)

-- 
2.15.0-rc1-151-g44fe2f342f

[PATCH 2/2] color: discourage use of ui.color=always

[Junio C Hamano]

Warn when we read such a configuration from a file, and nudge the
users to spell them 'auto' instead.

Signed-off-by: Junio C Hamano 
---
 Documentation/config.txt | 2 +-
 color.c  | 7 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index cb0f951ddc..ba01b8d3df 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1178,7 +1178,7 @@ color.ui::
or the `--color` option. Set it to `true` or `auto` to enable
color when output is written to the terminal (this is also the
default since Git 1.8.4). The value `always` is a historical
-   synonym for `auto`.
+   synonym for `auto` and its use is discouraged.
 
 column.ui::
Specify whether supported commands should output in columns.
diff --git a/color.c b/color.c
index 5b06c76bdc..5119f9bca0 100644
--- a/color.c
+++ b/color.c
@@ -308,6 +308,8 @@ int git_config_colorbool(const char *var, const char *value)
if (!strcasecmp(value, "never"))
return 0;
if (!strcasecmp(value, "always")) {
+   static int warn_once;
+
/*
 * Command-line options always respect "always".
 * Likewise for "-c" config on the command-line.
@@ -320,6 +322,11 @@ int git_config_colorbool(const char *var, const char 
*value)
 * Otherwise, we're looking at on-disk config;
 * downgrade to auto.
 */
+   if (!warn_once) {
+   warn_once++;
+   warning("setting '%s' to '%s' is no longer 
valid; "
+   "set it to 'auto' instead", var, value);
+   }
return GIT_COLOR_AUTO;
}
if (!strcasecmp(value, "auto"))
-- 
2.15.0-rc1-151-g44fe2f342f

Hello dear,

[Makena Jelani]

Hello dear,
I am Miss Makena Jelani. I have very important thing to discuss with
you please, this information is very vital. Contact me with my
privarte email so we can talk ( makenajel...@hotmail.com )
Makena.

Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option

[Junio C Hamano]

"Thais D. Braz"  writes:

> ---
>  Documentation/git-push.txt | 3 +++
>  builtin/push.c | 9 -
>  2 files changed, 11 insertions(+), 1 deletion(-)

Can somebody explain what is going on?  

I am guessing that Thais and marius are different people (judging by
the fact that one CC's a message to the other).  Are you two
collaborating on this change, or something?

It puzzles me to see almost identical change sent to the list
without much explanation from multiple parties, with no apparent
inter-developer coordination.

Thanks.

Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[Junio C Hamano]

"W. Trevor King"  writes:

> Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
> merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
> add a --signoff flag, 2017-07-04).

I cannot find a verb in the above.

> The order of options in merge-options.txt isn't clear to me, but I've
> put --signoff between --log and --stat as somewhat alphabetized and
> having an "add to the commit message" function like --log.
>
> The tests aren't as extensive as t7614-merge-signoff.sh, but they
> exercises both the --signoff and --no-signoff options.  There may be a
> more efficient way to set them up (like t7614-merge-signoff.sh's
> test_setup), but with all the pull options packed into a single test
> script it seemed easiest to just copy/paste the duplicate setup code.

The above two paragraphs read more like "requesting help for hints
to improve this patch" than commit log message.  Perhaps move them
below the three-dash line and instead describe what you actually did
here (if they were worth explaining, that is)?

> 09c2cb87 didn't motivate the addition of --allow-unrelated-histories
> to pull; only citing the reason from e379fdf3 (merge: refuse to create
> too cool a merge by default, 2016-03-18) gave for *not* including it.
> I like having both exposed in pull because while the fetch-and-merge
> approach might be a more popular way to judge "how well they fit
> together", you can also do that after an optimistic pull.  And in
> cases where an optimistic pull is likely to succeed, suggesting it is
> easier to explain to Git newbies than a FETCH_HEAD merge.

I find this paragraph totally unrelated to what the patch does.
Save it for the patch you add to pass --allow-unrelated-histories
given to pull down to underlying merge, perhaps?

>
> Signed-off-by: W. Trevor King 
> ---
>  Documentation/git-merge.txt |  8 
>  Documentation/merge-options.txt | 10 ++
>  builtin/pull.c  |  8 
>  t/t5521-pull-options.sh | 43 
> +
>  4 files changed, 61 insertions(+), 8 deletions(-)
> ...
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index ded8f98dbe..d95789ab8c 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -165,4 +165,47 @@ test_expect_success 'git pull 
> --allow-unrelated-histories' '
>   )
>  '
>  
> +test_expect_success 'git pull --signoff add a sign-off line' '
> + test_when_finished "rm -fr src dst actual expected" &&
> + cat >expected <<-EOF &&
> + Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e 
> "s/>.*/>/")
> + EOF

echo "Signed-off-by: $GIT_COMMITER_NAME <$GIT_COMMITTER_EMAIL>" >expect

or

git var GIT_COMMITTER_IDENT |
sed -e 's/^\([^>]*>\).*/Signed-off-by: \1/' >expect

> + git init src &&
> + (
> + cd src &&
> + test_commit one
> + ) &&

I suspect somebody will suggest "test_commit -C" ;-)

> + git clone src dst &&
> + (
> + cd src &&
> + test_commit two
> + ) &&
> + (
> + cd dst &&
> + git pull --signoff --no-ff &&
> + git cat-file commit HEAD | tail -n1 >../actual

I think it makes it more robust to replace "tail" with "collect all
the signed-off-by lines" like the other test (below) does.  Perhaps
have a helper function and use it in both?

get_signoff () {
git cat-file commit "$1" | sed -n -e '/^Signed-off-by: /p'
}

Some may say "cat-file can fail, and having it on the LHS of a pipe
hides its failure", advocating for something like:

get_signoff () {
git cat-file commit "$1" >sign-off-temp &&
sed -n -e '/^Signed-off-by: /p' sign-off-temp
}

> + ) &&
> + test_cmp expected actual
> +'

> +test_expect_success 'git pull --no-signoff flag cancels --signoff flag' '
> + test_when_finished "rm -fr src dst actual" &&
> + git init src &&
> + (
> + cd src &&
> + test_commit one
> + ) &&
> + git clone src dst &&
> + (
> + cd src &&
> + test_commit two
> + ) &&
> + (
> + cd dst &&
> + git pull --signoff --no-signoff --no-ff &&
> + git cat-file commit HEAD | sed -n /Signed-off-by/p >../actual
> + ) &&
> + test_must_be_empty actual
> +'
> +
>  test_done

I think "--signoff" and "--signoff --no-signoff" are reasonable
minimum things to test.  Two more cases, i.e. running it without
either and with "--no-signoff" alone, to ensure that the sign-off
mechanism does not kick in would make it even better.

Thanks.

Re: [RFC] deprecate git stash save?

[Junio C Hamano]

Thomas Gummerer  writes:

> git stash push is the newer interface for creating a stash.  While we
> are still keeping git stash save around for the time being, it's better
> to point new users of git stash to the more modern (and more feature
> rich) interface, instead of teaching them the older version that we
> might want to phase out in the future.

With devil's advocate hat on, because the primary point of "stash"
being "clear the desk quickly", I do not necessarily agree that
"more feature rich" is a plus and something we should nudge newbies
towards.

Re: [PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[Junio C Hamano]

Takahito Ogawa  writes:

> @@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` 
> commit.
>  The modifications stashed away by this command can be listed with
>  `git stash list`, inspected with `git stash show`, and restored
>  (potentially on top of a different commit) with `git stash apply`.
> -Calling `git stash` without any arguments is equivalent to `git stash save`.
> +Calling `git stash` without any arguments is equivalent to `git stash push`.

Hmph.  Is there any difference between

git stash save
git stash push

without any other argument?  Aren't they equivalent to

git stash

without any argument, which is what this sentence explains?

Re: [PATCH] doc: emphasize stash "--keep-index" stashes staged content

[Junio C Hamano]

"Robert P. J. Day"  writes:

> -If the `--keep-index` option is used, all changes already added to the
> -index are left intact.
> +If the `--keep-index` option is used, all changes already staged in the
> +index are left intact in the index, while still being added to the stash.

I do not think "left intact in the index" is an improvement.  The
primary reason why people use --keep-index is to get a working tree
that has _only_ the changes that are already added to the index and
nothing else, so that the state in the index can be built and/or
tested.  So if you want to add anything to that sentence, it should
stress the fact that changes are left intact "in the working tree".
Adding "in the index" without doing so makes the result even more
confusing, not less, by allowing a mis-read "left intact _only_ in
the index?  so the working tree files are reverted to the HEAD's
version?"

I am still undecided as to the value of saying ", while still being
added...".

Re: v2.15.0-rc1 test failure

[Ramsay Jones]

On 11/10/17 23:34, Adam Dinwoodie wrote:
[snip]
> Hi Ramsay,
> 
> I assume, given you're emailing me, that this is a Cygwin failure?

Yes, sorry, I should have made that clear.

> t0021.15 has PERL as a requirement, and I see semi-regular failures from
> Git tests that are Perl-based in one way or another (git-svn tests are
> the most common problems).  I've not spotted t0021 failing in that way,
> but it sounds like the same class of problem.

Yep, many moons ago, I used to run the svn tests (on Linux and cygwin)
which would fail intermittently on cygwin. I didn't notice any problem
with perl though.

> I dig into these failures when I see them, mostly by running the script
> a few hundred times until I get the failure again, and they've always
> been Perl itself segfaulting.  That points to the problem being in
> Cygwin's Perl package rather than Git, and it's very unlikely to be
> anything that's got worse in v2.15.0.

Since I stopped running the svn tests, the number of intermittent test failures 
on cygwin have dropped significantly, but haven't gone away
completely.

I just finished the second test-suite run and, of course, t0021 ran
without problem this time. Hmm, I don't think I have time to chase
this down at the moment. I will keep your 'perl hypothesis' in mind
for next time, however.

Thanks.

ATB,
Ramsay Jones

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Junio C Hamano]

Heiko Voigt  writes:

> This "default" value thing got me thinking in a different direction. We
> could use a scheme like that to get names (and values) for submodules
> that are missing from the .gitmodules file. If we decide that we need to
> handle them.

Yes, I suspect that would improve things quite a bit in a repository
where it added a new submodule by filling the gap between the time
when a gitlink is added and an entry in .gitmodules is added.  The
latter needs to happen if the result of the work done in that
repository is pushed out elsewhere---otherwise it won't usable by
other people.

Re: v2.15.0-rc1 test failure

[Jonathan Nieder]

Hi,

Adam Dinwoodie wrote:

> t0021.15 has PERL as a requirement, and I see semi-regular failures from
> Git tests that are Perl-based in one way or another (git-svn tests are
> the most common problems).  I've not spotted t0021 failing in that way,
> but it sounds like the same class of problem.
>
> I dig into these failures when I see them, mostly by running the script
> a few hundred times until I get the failure again, and they've always
> been Perl itself segfaulting.  That points to the problem being in
> Cygwin's Perl package rather than Git, and it's very unlikely to be
> anything that's got worse in v2.15.0.

That reminds me of https://bugs.debian.org/868738, which I tracked down
to perl's "die" helper using errno to determine the exit status instead
of deterministically using 128.  I wasn't able to track it down further
than that.

t/t0021/rot13-filter.pl doesn't have any similar suspect constructs, but
thought I should mention it anyway.

Thanks,
Jonathan

Re: v2.15.0-rc1 test failure

[Adam Dinwoodie]

On Wed, Oct 11, 2017 at 11:15:57PM +0100, Ramsay Jones wrote:
> Hi Adam,
> 
> I had a test failure on the v2.15.0-rc1 build tonight.
> The test in question being t0021-conversion.sh #15
> ('required process filter should filter data'). I didn't
> have any test failures on v2.15.0-rc0, and I don't see
> any change that would have affected this test.
> 
> Also, I ran this test by hand (well, in a shell loop) at
> least 70 times tonight (after the test-suite run), without
> any failures, so ... (unfortunately, I don't have a trash
> directory to look at. :( )
> 
> I have just kicked off another full test-suite run.
> 
> Just a heads up! ;-)

Hi Ramsay,

I assume, given you're emailing me, that this is a Cygwin failure?

t0021.15 has PERL as a requirement, and I see semi-regular failures from
Git tests that are Perl-based in one way or another (git-svn tests are
the most common problems).  I've not spotted t0021 failing in that way,
but it sounds like the same class of problem.

I dig into these failures when I see them, mostly by running the script
a few hundred times until I get the failure again, and they've always
been Perl itself segfaulting.  That points to the problem being in
Cygwin's Perl package rather than Git, and it's very unlikely to be
anything that's got worse in v2.15.0.

I've never managed to get further than pinning the blame on Perl,
though.  If you manage to reproduce the failure and it turns out to be
something different, or you manage to dig into the failure in Perl
itself, that'd be some very interesting news.

Adam

Re: v2.15.0-rc1 test failure

[Ramsay Jones]

On 11/10/17 23:15, Ramsay Jones wrote:
> Hi Adam,
> 
> I had a test failure on the v2.15.0-rc1 build tonight.
> The test in question being t0021-conversion.sh #15
> ('required process filter should filter data'). I didn't
> have any test failures on v2.15.0-rc0, and I don't see
> any change that would have affected this test.
> 
> Also, I ran this test by hand (well, in a shell loop) at
> least 70 times tonight (after the test-suite run), without
> any failures, so ... (unfortunately, I don't have a trash
> directory to look at. :( )
> 
> I have just kicked off another full test-suite run.
> 
> Just a heads up! ;-)

Oops, for mailing list reader, I should have made clear that
this failure is only on cygwin. :-D

ATB,
Ramsay Jones

v2.15.0-rc1 test failure

[Ramsay Jones]

Hi Adam,

I had a test failure on the v2.15.0-rc1 build tonight.
The test in question being t0021-conversion.sh #15
('required process filter should filter data'). I didn't
have any test failures on v2.15.0-rc0, and I don't see
any change that would have affected this test.

Also, I ran this test by hand (well, in a shell loop) at
least 70 times tonight (after the test-suite run), without
any failures, so ... (unfortunately, I don't have a trash
directory to look at. :( )

I have just kicked off another full test-suite run.

Just a heads up! ;-)

ATB,
Ramsay Jones

Re: Enhancement request: git-push: Allow (configurable) default push-option

[Stefan Beller]

On Wed, Oct 11, 2017 at 2:18 AM, Marius Paliga  wrote:
> Found one possible issue when looking for duplicates, we need to use
>   "unsorted_string_list_has_string" instead of "string_list_has_string"
>
> -   if (!string_list_has_string(_options,
> item->string))
> +   if
> (!unsorted_string_list_has_string(_options, item->string)) {
>
> New (fixed) patch follows...
>
>
> Signed-off-by: Marius Paliga 

Yay, thanks for working on this!

Junio gave good advice to the patch itself (the code), another thing
is the commit message, which follows the formalities with the sign off,
but the content is not addressing the target audience.

The current commit message is written as an improvement compared
to the previous patch and for readers who are reviewing the patch
right now.

Commit messages are read by people later in time, also they do not
care about the different iterations of the patch, as only the final iteration
matters.

I think for the commit message you can borrow from the very first email
you sent to the list, maybe something like:

builtin/push.c: add push.pushOption config

Currently push options need to be given explicitly, via
the command line as "git push --push-option".

Some code review systems [which?] need specific push options
nearly all the time, so the UX of Git would be enhanced if push
options could be configured instead of given each time on the\
command line.

Add the config option push.pushOption, which is a multi
string option, containing push options that are sent by default.

When push options are set in the system wide config
(/etc/gitconfig), they can be unset(?) later in the more specific
repository config by setting the string to the empty string.

Add tests and documentation as well.

Signed-off-by ...

[PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option

[Thais D. Braz]

---
 Documentation/git-push.txt | 3 +++
 builtin/push.c | 9 -
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 3e76e99f3..e1036feaf 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -161,6 +161,9 @@ already exists on the remote side.
Transmit the given string to the server, which passes them to
the pre-receive as well as the post-receive hook. The given string
must not contain a NUL or LF character.
+   Can be configured using "git config push.optionDefault ".
+   After configured git push will always be executed silently
+   with --push-options .
 
 --receive-pack=::
 --exec=::
diff --git a/builtin/push.c b/builtin/push.c
index 2ac810422..ae3efafce 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -467,11 +467,18 @@ static int git_push_config(const char *k, const char *v, 
void *cb)
 {
int *flags = cb;
int status;
+   struct string_list push_options = STRING_LIST_INIT_DUP;
 
status = git_gpg_config(k, v, NULL);
if (status)
return status;
 
+   const struct string_list *optionsDefault = 
git_config_get_value_multi("push.optionDefault");
+   for (int i = 0; i < optionsDefault->nr; i++) {
+   string_list_insert(_options, 
optionsDefault->items[i].string);
+   }
+
+
if (!strcmp(k, "push.followtags")) {
if (git_config_bool(k, v))
*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;
@@ -515,7 +522,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
int push_cert = -1;
int rc;
const char *repo = NULL;/* default repository */
-   struct string_list push_options = STRING_LIST_INIT_DUP;
+   static struct string_list push_options = STRING_LIST_INIT_DUP;
const struct string_list_item *item;
 
struct option options[] = {
-- 
2.15.0.rc0.39.g2f0e14e.dirty

[PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option

[Thais D. Braz]

---
 Documentation/git-push.txt | 3 +++
 builtin/push.c | 9 -
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 3e76e99f3..e1036feaf 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -161,6 +161,9 @@ already exists on the remote side.
Transmit the given string to the server, which passes them to
the pre-receive as well as the post-receive hook. The given string
must not contain a NUL or LF character.
+   Can be configured using "git config push.optionDefault ".
+   After configured git push will always be executed silently
+   with --push-options .
 
 --receive-pack=::
 --exec=::
diff --git a/builtin/push.c b/builtin/push.c
index 2ac810422..ae3efafce 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -467,11 +467,18 @@ static int git_push_config(const char *k, const char *v, 
void *cb)
 {
int *flags = cb;
int status;
+   struct string_list push_options = STRING_LIST_INIT_DUP;
 
status = git_gpg_config(k, v, NULL);
if (status)
return status;
 
+   const struct string_list *optionsDefault = 
git_config_get_value_multi("push.optionDefault");
+   for (int i = 0; i < optionsDefault->nr; i++) {
+   string_list_insert(_options, 
optionsDefault->items[i].string);
+   }
+
+
if (!strcmp(k, "push.followtags")) {
if (git_config_bool(k, v))
*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;
@@ -515,7 +522,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
int push_cert = -1;
int rc;
const char *repo = NULL;/* default repository */
-   struct string_list push_options = STRING_LIST_INIT_DUP;
+   static struct string_list push_options = STRING_LIST_INIT_DUP;
const struct string_list_item *item;
 
struct option options[] = {
-- 
2.15.0.rc0.39.g2f0e14e.dirty

[no subject]

[Thais D. Braz]

I'll still try to make the test as suggested to complete this demand

[PATCH] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
add a --signoff flag, 2017-07-04).

The order of options in merge-options.txt isn't clear to me, but I've
put --signoff between --log and --stat as somewhat alphabetized and
having an "add to the commit message" function like --log.

The tests aren't as extensive as t7614-merge-signoff.sh, but they
exercises both the --signoff and --no-signoff options.  There may be a
more efficient way to set them up (like t7614-merge-signoff.sh's
test_setup), but with all the pull options packed into a single test
script it seemed easiest to just copy/paste the duplicate setup code.

09c2cb87 didn't motivate the addition of --allow-unrelated-histories
to pull; only citing the reason from e379fdf3 (merge: refuse to create
too cool a merge by default, 2016-03-18) gave for *not* including it.
I like having both exposed in pull because while the fetch-and-merge
approach might be a more popular way to judge "how well they fit
together", you can also do that after an optimistic pull.  And in
cases where an optimistic pull is likely to succeed, suggesting it is
easier to explain to Git newbies than a FETCH_HEAD merge.

Signed-off-by: W. Trevor King 
---
 Documentation/git-merge.txt |  8 
 Documentation/merge-options.txt | 10 ++
 builtin/pull.c  |  8 
 t/t5521-pull-options.sh | 43 +
 4 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4df6431c34..0ada8c856b 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,14 +64,6 @@ OPTIONS
 ---
 include::merge-options.txt[]
 
---signoff::
-   Add Signed-off-by line by the committer at the end of the commit
-   log message.  The meaning of a signoff depends on the project,
-   but it typically certifies that committer has
-   the rights to submit this work under the same license and
-   agrees to a Developer Certificate of Origin
-   (see http://developercertificate.org/ for more information).
-
 -S[]::
 --gpg-sign[=]::
GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 4e32304301..f394622d65 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -51,6 +51,16 @@ set to `no` at the beginning of them.
 With --no-log do not list one-line descriptions from the
 actual commits being merged.
 
+--signoff::
+--no-signoff::
+   Add Signed-off-by line by the committer at the end of the commit
+   log message.  The meaning of a signoff depends on the project,
+   but it typically certifies that committer has
+   the rights to submit this work under the same license and
+   agrees to a Developer Certificate of Origin
+   (see http://developercertificate.org/ for more information).
++
+With --no-signoff do not add a Signed-off-by line.
 
 --stat::
 -n::
diff --git a/builtin/pull.c b/builtin/pull.c
index 6f772e8a22..4469342f51 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -97,6 +97,7 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
 static int opt_allow_unrelated_histories;
+static int opt_signoff;
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -175,6 +176,9 @@ static struct option pull_options[] = {
OPT_SET_INT(0, "allow-unrelated-histories",
_allow_unrelated_histories,
N_("allow merging unrelated histories"), 1),
+   OPT_BOOL(0, "signoff",
+   _signoff,
+   N_("add Signed-off-by:")),
 
/* Options passed to git-fetch */
OPT_GROUP(N_("Options related to fetching")),
@@ -610,6 +614,10 @@ static int run_merge(void)
argv_array_push(, opt_gpg_sign);
if (opt_allow_unrelated_histories > 0)
argv_array_push(, "--allow-unrelated-histories");
+   if (opt_signoff > 0)
+   argv_array_push(, "--signoff");
+   else
+   argv_array_push(, "--no-signoff");
 
argv_array_push(, "FETCH_HEAD");
ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98dbe..d95789ab8c 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -165,4 +165,47 @@ test_expect_success 'git pull --allow-unrelated-histories' 
'
)
 '
 
+test_expect_success 'git pull --signoff add a sign-off line' '
+   test_when_finished "rm -fr src dst actual expected" &&
+   cat >expected <<-EOF &&
+   Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e 
"s/>.*/>/")
+   EOF
+   git init src &&
+   (
+

[PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[Takahito Ogawa]

"git stash" behavior without any arguments was changed in
1ada5020b ("stash: use stash_push for no verb form", 2017-02-28).
This is equivalent to "git stash push" but documents says
"git stash save".

Correct it.

Reviewed-by: Thomas Gummerer 
Signed-off-by: Takahito Ogawa 
---
 Documentation/git-stash.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 00f95fee1..63642c145 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git stash list`, inspected with `git stash show`, and restored
 (potentially on top of a different commit) with `git stash apply`.
-Calling `git stash` without any arguments is equivalent to `git stash save`.
+Calling `git stash` without any arguments is equivalent to `git stash push`.
 A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
-- 
2.13.1

Re: Fwd: how can I conform if I succeed in sending patch to mailing list

[Thomas Gummerer]

On 10/12, 小川恭史 wrote:
> Hello, I found a mistake in documents, fixed it, and send patch to mailing 
> list.
> 
> Sending patches by 'git send-email' with Gmail smtp seemed to be
> successful because CC included my email address and I received it.
> However, I never received email from mailing list. Of course I'm
> subscribing mailing list.
> 
> How can I conform if I succeed in sending patch to mailing list?

I think that's just a feature of the mailing list, where it doesn't
send you an email in which you are already Cc'd in, or something like
that.  I received your mails through the mailing list.

You can check if they arrived on the list at the public archives of
the mailing list (https://public-inbox.org/git/).

> Takahito Ogawa

Re: slight addition to t.gummerer's proposed "git stash" patch

[Robert P. J. Day]

On Wed, 11 Oct 2017, Thomas Gummerer wrote:

> On 10/11, Robert P. J. Day wrote:
> >
> >   was perusing thomas gummerer's proposed "git stash" patch here:
> >
> > https://www.spinics.net/lists/git/msg313993.html
> >
> > and i'd make one more change -- i'd separate the OPTIONS entries for
> > "git stash push" and "git stash save" so they don't end up being
> > rendered all crushed together when displaying the man page:
>
> I for one would like that.  I sent a patch recently [1] that would
> show git stash push first on the man page, which didn't seem to get
> much traction.  This goes a bit further than that, which I'd be happy
> with.
>
> [1]: https://public-inbox.org/git/20171005201029.4173-1-t.gumme...@gmail.com/

  ... snip ...

if you want, just crush my suggestion into your earlier patch and
resubmit it.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: Fwd: how can I conform if I succeed in sending patch to mailing list

[Jonathan Tan]

On Thu, 12 Oct 2017 04:14:18 +0900
小川恭史  wrote:

> Hello, I found a mistake in documents, fixed it, and send patch to mailing 
> list.
> 
> Sending patches by 'git send-email' with Gmail smtp seemed to be
> successful because CC included my email address and I received it.
> However, I never received email from mailing list. Of course I'm
> subscribing mailing list.
> 
> How can I conform if I succeed in sending patch to mailing list?

The easiest way I can think of is to check an online mailing list
archive [1].

I think your patch was received, as you can see in [2].

[1] for example, https://public-inbox.org/git/

[2] https://public-inbox.org/git/?q=aiueogawa217

Re: slight addition to t.gummerer's proposed "git stash" patch

[Thomas Gummerer]

On 10/11, Robert P. J. Day wrote:
> 
>   was perusing thomas gummerer's proposed "git stash" patch here:
> 
> https://www.spinics.net/lists/git/msg313993.html
> 
> and i'd make one more change -- i'd separate the OPTIONS entries for
> "git stash push" and "git stash save" so they don't end up being
> rendered all crushed together when displaying the man page:

I for one would like that.  I sent a patch recently [1] that would
show git stash push first on the man page, which didn't seem to get
much traction.  This goes a bit further than that, which I'd be happy
with.

[1]: https://public-inbox.org/git/20171005201029.4173-1-t.gumme...@gmail.com/

> OPTIONS
>save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] 
> [-a|--all] [-q|--quiet]
>[], push [-p|--patch] [-k|--[no-]keep-index] 
> [-u|--include-untracked] [-a|--all]
>[-q|--quiet] [-m|--message ] [--] [...]
>Save your local modifications to a new stash and roll them back to 
> HEAD (in the working
>tree and in the index). The  part is optional and gives the 
> description along
>with the stashed state.
>... snip ...
> 
> so rather than:
> 
> OPTIONS
> ---
> 
> push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
> [-a|--all] [-q|--quiet] [-m|--message ] [--]
> [...]::
> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
> [-a|--all] [-q|--quiet] []::
> 
> Save your local modifications to a new 'stash entry' and roll them
> back to HEAD (in the working tree and in the index).
> The  part is optional and gives
> the description along with the stashed state.
> ...
> 
> i'd suggest:
> 
> push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
> [-a|--all] [-q|--quiet] [-m|--message ] [--]
> [...]::
> 
> Save your local modifications to a new 'stash entry' and roll them
> back to HEAD (in the working tree and in the index).
> The  part is optional and gives
> the description along with the stashed state.
> ...
> 
> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
> [-a|--all] [-q|--quiet] []::
> 
> This option is deprecated in favour of 'git stash push'.

This sounds good to me.  This can probably be done at the same time
(or after) something like my patch [2], which removes the mentions of
'git stash save' from the man pages, and replaces them with 'git stash
push'.  I guess it would be a bit confusing to see a deprecated
command in the man pages, especially since there is a good (almost
drop-in) replacement :)

[2]: https://public-inbox.org/git/20171005200049.GF30301@hank/#t

> or something like that.
> 
> rday
> 
> -- 
> 
> 
> Robert P. J. Day Ottawa, Ontario, CANADA
> http://crashcourse.ca
> 
> Twitter:   http://twitter.com/rpjday
> LinkedIn:   http://ca.linkedin.com/in/rpjday
> 
> 

Fwd: how can I conform if I succeed in sending patch to mailing list

[小川恭史]

Hello, I found a mistake in documents, fixed it, and send patch to mailing list.

Sending patches by 'git send-email' with Gmail smtp seemed to be
successful because CC included my email address and I received it.
However, I never received email from mailing list. Of course I'm
subscribing mailing list.

How can I conform if I succeed in sending patch to mailing list?

Takahito Ogawa

Re: [PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[Thomas Gummerer]

On 10/11, Thomas Gummerer wrote:
> On 10/12, Takahito Ogawa wrote:
> > "git stash" behavior without any arguments was changed in
> > 1ada5020b ("stash: use stash_push for no verb form", 2017-02-28).
> > This is equivalent to "git stash push" but documents says
> > "git stash save".
> > 
> > Correct it.
> 
> Thanks for fixing this!  I recently sent a patch that would advertise
> git stash push more in general, which would also fix this occurrence [1], 
> but it didn't seem like it got much interest.  However this is
> obviously correct, and should definitely be fixed, while the other
> places can still mention 'git stash save'.
> 
> For what it's worth this is
> 
> Reviewed-by: Thomas Gummerer 

And I forgot to include the link, sorry.  Here it is:

[1]: https://public-inbox.org/git/20171005200049.GF30301@hank/

> 
> > Signed-off-by: Takahito Ogawa 
> > ---
> >  Documentation/git-stash.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> > index 00f95fee1..63642c145 100644
> > --- a/Documentation/git-stash.txt
> > +++ b/Documentation/git-stash.txt
> > @@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` 
> > commit.
> >  The modifications stashed away by this command can be listed with
> >  `git stash list`, inspected with `git stash show`, and restored
> >  (potentially on top of a different commit) with `git stash apply`.
> > -Calling `git stash` without any arguments is equivalent to `git stash 
> > save`.
> > +Calling `git stash` without any arguments is equivalent to `git stash 
> > push`.
> >  A stash is by default listed as "WIP on 'branchname' ...", but
> >  you can give a more descriptive message on the command line when
> >  you create one.
> > -- 
> > 2.13.1
> > 

Re: [PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[Thomas Gummerer]

On 10/12, Takahito Ogawa wrote:
> "git stash" behavior without any arguments was changed in
> 1ada5020b ("stash: use stash_push for no verb form", 2017-02-28).
> This is equivalent to "git stash push" but documents says
> "git stash save".
> 
> Correct it.

Thanks for fixing this!  I recently sent a patch that would advertise
git stash push more in general, which would also fix this occurrence [1], 
but it didn't seem like it got much interest.  However this is
obviously correct, and should definitely be fixed, while the other
places can still mention 'git stash save'.

For what it's worth this is

Reviewed-by: Thomas Gummerer 


> Signed-off-by: Takahito Ogawa 
> ---
>  Documentation/git-stash.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 00f95fee1..63642c145 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` 
> commit.
>  The modifications stashed away by this command can be listed with
>  `git stash list`, inspected with `git stash show`, and restored
>  (potentially on top of a different commit) with `git stash apply`.
> -Calling `git stash` without any arguments is equivalent to `git stash save`.
> +Calling `git stash` without any arguments is equivalent to `git stash push`.
>  A stash is by default listed as "WIP on 'branchname' ...", but
>  you can give a more descriptive message on the command line when
>  you create one.
> -- 
> 2.13.1
> 

Re: [PATCH] column: show auto columns when pager is active

[Martin Ågren]

On 11 October 2017 at 20:36, Kevin Daudt  wrote:
> On Wed, Oct 11, 2017 at 08:12:35PM +0200, Martin Ågren wrote:
>> On 11 October 2017 at 19:23, Kevin Daudt  wrote:
>> I wonder if it's useful to set COLUMNS a bit lower so that this has to
>> split across more than one line (but not six), i.e., to do something
>> non-trivial. I suppose that might lower the chances of some weird
>> breakage slipping through.
>
> Yeah, I was doubting about that, but wouldn't this amount to testing
> whether git column is working properly, instead of just testing whether
> it's being done at all?

Right, I think you'd need a pretty crazy bug in order to slip through
all tests here.

>> These were just the thoughts that occurred to me, not sure if any of
>> them is particularly significant. Thanks for cleaning up after me.
>>
>
> np. Just as I posted earlier, I think you did not actually cause the bug
> (because this has never worked), it just made it visible to more users.

Well, the general bug/behavior was always there, but I regressed a
particular use-case. In 2.14, you could do `git tag` with column.ui=auto
and it would do the columns-thing. But with ff1e72483, the behavior
changed. To add insult to injury, it might be non-obvious that the pager
is running, since with just a few tags, the pager simply exits silently.
So debugging this could probably be quite frustrating.

Martin

Re: Unable to use --patch with git add

[Kevin Daudt]

On Wed, Oct 11, 2017 at 11:16:39PM +0530, Ayush Goel wrote:
> $ git --version
> git version 2.14.2
> 
> What more details can I provide to help debug this?
> 

Hello Ayush,

Run:

  git config --global color.ui auto
  git config --unset --local color.ui #in case it's set locally

This is a known 'breakage' because people have set color.ui to always
(which should not be used anyway).

Kind regards, Kevin

[PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[Takahito Ogawa]

"git stash" behavior without any arguments was changed in
1ada5020b ("stash: use stash_push for no verb form", 2017-02-28).
This is equivalent to "git stash push" but documents says
"git stash save".

Correct it.

Signed-off-by: Takahito Ogawa 
---
 Documentation/git-stash.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 00f95fee1..63642c145 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git stash list`, inspected with `git stash show`, and restored
 (potentially on top of a different commit) with `git stash apply`.
-Calling `git stash` without any arguments is equivalent to `git stash save`.
+Calling `git stash` without any arguments is equivalent to `git stash push`.
 A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
-- 
2.13.1

Re: [PATCH] column: show auto columns when pager is active

[Kevin Daudt]

On Wed, Oct 11, 2017 at 08:12:35PM +0200, Martin Ågren wrote:
> On 11 October 2017 at 19:23, Kevin Daudt  wrote:
> > finalize_colopts in column.c only checks whether the output is a TTY to
> > determine if columns should be enabled with columns set to auto. Also check
> > if the pager is active.
> 
> Maybe you could say something about the difficulties of writing a test
> for `git column` proper. Something like this perhaps:
> 
>   Adding a test for git column is possible but requires some care to
>   work around a race on stdin. See commit 18d8c2693 (test_terminal:
>   redirect child process' stdin to a pty, 2015-08-04). Test git tag
>   instead, since that does not involve stdin, and since that was the
>   original motivation for this patch.

Right, makes sense.
> 
> > Helped-by: Rafael Ascensão 
> > Signed-off-by: Kevin Daudt 
> > ---
> >  column.c |  3 ++-
> >  t/t7006-pager.sh | 14 ++
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> Does the documentation on `column.ui` need to be updated? It talks about
> "if the output is to the terminal". That's similar to the documentation
> on the various `color.*`, so we should be fine, and arguably it's even
> better not to say anything since that makes it consistent.
> 
> > diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> > index f0f1abd1c..44c2ca5d3 100755
> > --- a/t/t7006-pager.sh
> > +++ b/t/t7006-pager.sh
> > @@ -570,4 +570,18 @@ test_expect_success 'command with underscores does not 
> > complain' '
> > test_cmp expect actual
> >  '
> >
> > +test_expect_success TTY 'git tag with auto-columns ' '
> > +   test_commit one &&
> > +   test_commit two &&
> > +   test_commit three &&
> > +   test_commit four &&
> > +   test_commit five &&
> > +   cat >expected <<\EOF &&
> > +initial  one  two  threefour five
> > +EOF
> > +   test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \
> > +   git -p -c column.ui=auto tag --sort=authordate &&
> > +   test_cmp expected actual.tag
> > +'
> > +
> >  test_done
> 
> Since `git tag` pages when it's listing, you don't need the `-p`. But
> it's not like it hurts to have it. Yeah, I know, you needed it with `git
> column`. :-)

Right, it was a bit of a left-over since I assumed the PAGER='cat 
>paginated.out'
from the beginning of the test was in place and I wasn't getting any
output, but it turned out PAGER wasn't set.

> I wonder if it's useful to set COLUMNS a bit lower so that this has to
> split across more than one line (but not six), i.e., to do something
> non-trivial. I suppose that might lower the chances of some weird
> breakage slipping through.

Yeah, I was doubting about that, but wouldn't this amount to testing
whether git column is working properly, instead of just testing whether
it's being done at all?

> This test uses "actual.tag" while most (all?) others in this file use
> "actual". Maybe you worry about checking the "wrong" file, e.g., in case
> the pager doesn't kick in. You could do `rm -f actual` before the
> `test_terminal`-invocation to protect against that.

Yeah, I actually ran into that, but rm-ing it is better, I agree.

> These were just the thoughts that occurred to me, not sure if any of
> them is particularly significant. Thanks for cleaning up after me.
> 

np. Just as I posted earlier, I think you did not actually cause the bug
(because this has never worked), it just made it visible to more users.

Kevin

Re: Unable to use --patch with git add

[Jeff King]

On Wed, Oct 11, 2017 at 11:43:46PM +0530, Ayush Goel wrote:

> Thank you for your mail. It works :)
> 
> For future reference, is there a page for known issues of git?

No, there's just the mailing list archive. But you can search it. E.g.:

  https://public-inbox.org/git/?q=%22add+-p%22

finds the relevant threads.

-Peff

Re: Unable to use --patch with git add

[Ayush Goel]

Hey Jeff,

Thank you for your mail. It works :)

For future reference, is there a page for known issues of git?

On Wed, Oct 11, 2017 at 11:30 PM, Jeff King  wrote:
> On Wed, Oct 11, 2017 at 11:25:52PM +0530, Ayush Goel wrote:
>
>> On Wed, Oct 11, 2017 at 11:19 PM, Santiago Torres  wrote:
>> > It'd be helpful to know:
>> >
>> > - What did you do?
>>
>> I have recently updated to git version 2.14.2. The problem started
>> happening after that.
>> Made changes to a file. Tried `git add -p`.
>
> This is a known issue in v2.14.2 with having "color.ui" set to "always"
> in your config.
>
> There's a fix in v2.15.0-rc1. You can either upgrade to that, or as a
> workaround set "color.ui" to "auto" (or remove it completely from your
> config, as "auto" is the default).
>
> -Peff



-- 
Regards,
Ayush Goel

Re: [PATCH] column: show auto columns when pager is active

[Martin Ågren]

On 11 October 2017 at 19:23, Kevin Daudt  wrote:
> finalize_colopts in column.c only checks whether the output is a TTY to
> determine if columns should be enabled with columns set to auto. Also check
> if the pager is active.

Maybe you could say something about the difficulties of writing a test
for `git column` proper. Something like this perhaps:

  Adding a test for git column is possible but requires some care to
  work around a race on stdin. See commit 18d8c2693 (test_terminal:
  redirect child process' stdin to a pty, 2015-08-04). Test git tag
  instead, since that does not involve stdin, and since that was the
  original motivation for this patch.

> Helped-by: Rafael Ascensão 
> Signed-off-by: Kevin Daudt 
> ---
>  column.c |  3 ++-
>  t/t7006-pager.sh | 14 ++
>  2 files changed, 16 insertions(+), 1 deletion(-)

Does the documentation on `column.ui` need to be updated? It talks about
"if the output is to the terminal". That's similar to the documentation
on the various `color.*`, so we should be fine, and arguably it's even
better not to say anything since that makes it consistent.

> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> index f0f1abd1c..44c2ca5d3 100755
> --- a/t/t7006-pager.sh
> +++ b/t/t7006-pager.sh
> @@ -570,4 +570,18 @@ test_expect_success 'command with underscores does not 
> complain' '
> test_cmp expect actual
>  '
>
> +test_expect_success TTY 'git tag with auto-columns ' '
> +   test_commit one &&
> +   test_commit two &&
> +   test_commit three &&
> +   test_commit four &&
> +   test_commit five &&
> +   cat >expected <<\EOF &&
> +initial  one  two  threefour five
> +EOF
> +   test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \
> +   git -p -c column.ui=auto tag --sort=authordate &&
> +   test_cmp expected actual.tag
> +'
> +
>  test_done

Since `git tag` pages when it's listing, you don't need the `-p`. But
it's not like it hurts to have it. Yeah, I know, you needed it with `git
column`. :-)

I wonder if it's useful to set COLUMNS a bit lower so that this has to
split across more than one line (but not six), i.e., to do something
non-trivial. I suppose that might lower the chances of some weird
breakage slipping through.

This test uses "actual.tag" while most (all?) others in this file use
"actual". Maybe you worry about checking the "wrong" file, e.g., in case
the pager doesn't kick in. You could do `rm -f actual` before the
`test_terminal`-invocation to protect against that.

These were just the thoughts that occurred to me, not sure if any of
them is particularly significant. Thanks for cleaning up after me.

Martin

Re: No log --no-decorate completion?

[Stefan Beller]

On Wed, Oct 11, 2017 at 7:47 AM, Max Rothman  wrote:
> I recently noticed that in the git-completion script, there's
> completion for --decorate={full,yes,no} for git log and family, but
> not for --no-decorate. Is that intentional? If not, I *think* I see
> how it could be added.
>
> Thanks,
> Max

Using git-blame, I found af4e9e8c87 (completion: update am, commit, and log,
2009-10-07) as well as af16bdaa3f (completion: fix and update 'git log
--decorate='
options, 2015-05-01), both of their commit messages do not discuss leaving out
--no-decorate intentionally.

If you give --no- you'd get more than just the completion to --no-decorate,
but all the negated options, I would assume?

So maybe that is why no one added the negated options, yet?

Thanks,
Stefan

[PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[Takahito Ogawa]

"git stash" behavior without any arguments was changed in
1ada5020b ("stash: use stash_push for no verb form", 2017-02-28).
This is equivalent to "git stash push" but document says
"git stash save".

Correct it.

Signed-off-by: Takahito Ogawa 
---
 Documentation/git-stash.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 00f95fee1..63642c145 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git stash list`, inspected with `git stash show`, and restored
 (potentially on top of a different commit) with `git stash apply`.
-Calling `git stash` without any arguments is equivalent to `git stash save`.
+Calling `git stash` without any arguments is equivalent to `git stash push`.
 A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
-- 
2.13.1

[PATCH 1/1] git-stash.txt: correct "git stash" behavior with no arguments

[Takahito Ogawa]

"git stash" behavior without any arguments was changed in
1ada5020b ("stash: use stash_push for no verb form", 2017-02-28).
This is equivalent to "git stash push" but document says
"git stash save".

Correct it.

Signed-off-by: Takahito Ogawa 
---
 Documentation/git-stash.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 00f95fee1..63642c145 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git stash list`, inspected with `git stash show`, and restored
 (potentially on top of a different commit) with `git stash apply`.
-Calling `git stash` without any arguments is equivalent to `git stash save`.
+Calling `git stash` without any arguments is equivalent to `git stash push`.
 A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
-- 
2.13.1

Re: Unable to use --patch with git add

[Jeff King]

On Wed, Oct 11, 2017 at 11:25:52PM +0530, Ayush Goel wrote:

> On Wed, Oct 11, 2017 at 11:19 PM, Santiago Torres  wrote:
> > It'd be helpful to know:
> >
> > - What did you do?
> 
> I have recently updated to git version 2.14.2. The problem started
> happening after that.
> Made changes to a file. Tried `git add -p`.

This is a known issue in v2.14.2 with having "color.ui" set to "always"
in your config.

There's a fix in v2.15.0-rc1. You can either upgrade to that, or as a
workaround set "color.ui" to "auto" (or remove it completely from your
config, as "auto" is the default).

-Peff

Re: Unable to use --patch with git add

[Ayush Goel]

Hello Santiago,

Thank you for picking this up.


On Wed, Oct 11, 2017 at 11:19 PM, Santiago Torres  wrote:
> It'd be helpful to know:
>
> - What did you do?

I have recently updated to git version 2.14.2. The problem started
happening after that.
Made changes to a file. Tried `git add -p`.

> - What did you expect to happen?

Enter the patch add mode of git.

> - What happened instead?

Usual git patch mode is that it shows a patch and expects user to
add/skip/split etc. Instead it never stops to take input from me.

See this ->


git add -p
diff --git a/Tests/OSXTests/TDTStreamingTests.m
b/Tests/OSXTests/TDTStreamingTests.m
index 35757bc..525fe56 100644
--- a/Tests/OSXTests/TDTStreamingTests.m
+++ b/Tests/OSXTests/TDTStreamingTests.m
@@ -116,7 +116,7 @@ static NSTask *compressionServerTask;
 }

 - (void)breathe {
-  NSDate *date = [NSDate dateWithTimeIntervalSinceNow:0.1];
+  NSDate *date = [NSDate dateWithTimeIntervalSinceNow:0.01];
   [[NSRunLoop currentRunLoop] runUntilDate:date];
 }

 Expecting git input mode to stop the process here #
 Instead git exits without any error #

>
> I suspect you are using --patch with a new file, so you probably need to first
> add it with -N or so. This is just a shot in the dark though...
>

Not adding new file.

> Thanks,
> -Santiago.
>
> On Wed, Oct 11, 2017 at 11:16:39PM +0530, Ayush Goel wrote:
>> $ git --version
>> git version 2.14.2
>>
>> What more details can I provide to help debug this?
>>
>> --
>> Regards,
>> Ayush Goel



-- 
Regards,
Ayush Goel

Re: Award

[Abraham, Jimmy]

From: Abraham, Jimmy
Sent: Wednesday, October 11, 2017 11:11 AM
Subject: Award

Reply for details on award.
CONFIDENTIALITY NOTICE: This email message and any accompanying data or files 
is confidential and may contain privileged information intended only for the 
named recipient(s). If you are not the intended recipient(s), you are hereby 
notified that the dissemination, distribution, and or copying of this message 
is strictly prohibited. If you receive this message in error, or are not the 
named recipient(s), please notify the sender at the email address above, delete 
this email from your computer, and destroy any copies in any form immediately. 
Receipt by anyone other than the named recipient(s) is not a waiver of any 
attorney-client, work product, or other applicable privilege.

Re: Unable to use --patch with git add

[Santiago Torres]

It'd be helpful to know:

- What did you do?
- What did you expect to happen?
- What happened instead?

I suspect you are using --patch with a new file, so you probably need to first
add it with -N or so. This is just a shot in the dark though...

Thanks,
-Santiago.

On Wed, Oct 11, 2017 at 11:16:39PM +0530, Ayush Goel wrote:
> $ git --version
> git version 2.14.2
> 
> What more details can I provide to help debug this?
> 
> -- 
> Regards,
> Ayush Goel


signature.asc
Description: PGP signature

Unable to use --patch with git add

[Ayush Goel]

$ git --version
git version 2.14.2

What more details can I provide to help debug this?

-- 
Regards,
Ayush Goel

[PATCH] column: show auto columns when pager is active

[Kevin Daudt]

When columns are set to automatic for git tag and the output is
paginated by git, the output is a single column instead of multiple
columns.

Standard behaviour in git is to honor auto values when the pager is
active, which happens for example with commands like git log showing
colors when being paged.

Since ff1e72483 (tag: change default of `pager.tag` to "on",
2017-08-02), the pager has been enabled by default, exposing this
problem to more people.

finalize_colopts in column.c only checks whether the output is a TTY to
determine if columns should be enabled with columns set to auto. Also check
if the pager is active.

Helped-by: Rafael Ascensão 
Signed-off-by: Kevin Daudt 
---
 column.c |  3 ++-
 t/t7006-pager.sh | 14 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/column.c b/column.c
index ff7bdab1a..ded50337f 100644
--- a/column.c
+++ b/column.c
@@ -5,6 +5,7 @@
 #include "parse-options.h"
 #include "run-command.h"
 #include "utf8.h"
+#include "pager.c"
 
 #define XY2LINEAR(d, x, y) (COL_LAYOUT((d)->colopts) == COL_COLUMN ? \
(x) * (d)->rows + (y) : \
@@ -224,7 +225,7 @@ int finalize_colopts(unsigned int *colopts, int 
stdout_is_tty)
if (stdout_is_tty < 0)
stdout_is_tty = isatty(1);
*colopts &= ~COL_ENABLE_MASK;
-   if (stdout_is_tty)
+   if (stdout_is_tty || pager_in_use())
*colopts |= COL_ENABLED;
}
return 0;
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index f0f1abd1c..44c2ca5d3 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -570,4 +570,18 @@ test_expect_success 'command with underscores does not 
complain' '
test_cmp expect actual
 '
 
+test_expect_success TTY 'git tag with auto-columns ' '
+   test_commit one &&
+   test_commit two &&
+   test_commit three &&
+   test_commit four &&
+   test_commit five &&
+   cat >expected <<\EOF &&
+initial  one  two  threefour five
+EOF
+   test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \
+   git -p -c column.ui=auto tag --sort=authordate &&
+   test_cmp expected actual.tag
+'
+
 test_done
-- 
2.14.2

git repack leaks disk space on ENOSPC

[Andreas Krey]

Hi all,

I observed (again) an annoying behavior of 'git repack':
When the new pack file cannot be fully written because
the disk gets full beforehand, the tmp_pack file isn't
deleted, meaning the disk stays full:

  $ df -h .; git repack -ad; df -h .; ls -lart .git/objects/pack/tmp*; rm 
.git/objects/pack/tmp*; df -h .
  FilesystemSize  Used Avail Use% Mounted on
  /dev/mapper/vg02-localworkspaces  250G  245G  5.1G  98% /workspaces/calvin
  Counting objects: 4715349, done.
  Delta compression using up to 8 threads.
  Compressing objects: 100% (978051/978051), done.
  fatal: sha1 file '.git/objects/pack/tmp_pack_xB7DMt' write error: No space 
left on device
  FilesystemSize  Used Avail Use% Mounted on
  /dev/mapper/vg02-localworkspaces  250G  250G   20K 100% /workspaces/calvin
  -r--r--r-- 1 andrkrey users 5438435328 Oct 11 17:03 
.git/objects/pack/tmp_pack_xB7DMt
  rm: remove write-protected regular file '.git/objects/pack/tmp_pack_xB7DMt'? y
  FilesystemSize  Used Avail Use% Mounted on
  /dev/mapper/vg02-localworkspaces  250G  245G  5.1G  98% /workspaces/calvin

- Andreas

git version 2.15.0.rc0

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Josh Triplett]

On Tue, Oct 10, 2017 at 11:39:21AM -0700, Stefan Beller wrote:
> On Tue, Oct 10, 2017 at 6:03 AM, Heiko Voigt  wrote:
> > but in the long run my goal
> > for submodules is and always was: Make them behave as close to files as
> > possible. And why should a 'git add submodule' not magically do
> > everything it can to make submodules just work? I can look into a patch
> > for that if people agree here...
> 
> I'd love to see this implemented. I cc'd Josh (the author of git-series), who
> may disagree with this, or has some good input how to go forward without
> breaking git-series.

git-series doesn't use the git-submodule command at all, nor does it
construct series trees using git-add or any other git command-line tool;
it constructs gitlinks directly. Most of the time, it doesn't even make
sense to `git checkout` a series branch. Modifying commands like git-add
and similar to automatically manage .gitmodules won't cause any issue at
all, as long as git itself doesn't start rejecting or complaining about
repositories that have gitlinks without a .gitmodules file.

- Josh Triplett

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Heiko Voigt]

On Wed, Oct 11, 2017 at 08:31:37AM +0900, Junio C Hamano wrote:
> Stefan Beller  writes:
> 
> > So you propose to make git-add behave like "git submodule add"
> > (i.e. also add the .gitmodules entry for name/path/URL), which I
> > like from a submodule perspective.
> >
> > However other users of gitlinks might be confused[1], which is why
> > I refrained from "making every gitlink into a submodule". Specifically
> > the more powerful a submodule operation is (the more fluff adds),
> > the harder it should be for people to mis-use it.
> 
> A few questions that come to mind are:
> 
>  - Does "git add sub/" have enough information to populate
>.gitmodules?  If we have reasonable "default" values for
>.gitmodules entries (e.g. missing URL means we won't fetch when
>asked to go recursively fetch), perhaps we can leave everything
>other than "submodule.$name.path" undefined.

My suggestion would be: If we do not have them we do not populate them.
We could even go further and say: If we do not have the set "git
submodule add" would populate then we do not add anything to .gitmodules
and warn the user.

>  - Can't we help those who have gitlinks without .gitmodules entries
>exactly the same way as above, i.e. when we see a gitlink and try
>to treat it as a submodule, we'd first try to look it up from
>.gitmodules (by going from path to name and then to
>submodule.$name.$var); the above "'git add sub/' would add an
>entry for .gitmodules" wish is based on the assumption that there
>are reasonable "default" values for each of these $var--so by
>basing on the same assumption, we can "pretend" as if these
>submodule.$name.$var were in .gitmodules file when we see
>gitlinks without .gitmodules entries.  IOW, if "git add sub/" can
>add .gitmodules to help people without having to type "git
>submodule add sub/", then we can give exactly the same degree of
>help without even modifying .gitmodules when "git add sub/" is
>run.

This "default" value thing got me thinking in a different direction. We
could use a scheme like that to get names (and values) for submodules
that are missing from the .gitmodules file. If we decide that we need to
handle them.

Cheers Heiko

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Heiko Voigt]

On Tue, Oct 10, 2017 at 11:39:21AM -0700, Stefan Beller wrote:
> So you propose to make git-add behave like "git submodule add"
> (i.e. also add the .gitmodules entry for name/path/URL), which I
> like from a submodule perspective.

Well more like: clone and add will behave like "git submodule add" but
basically yes.

> However other users of gitlinks might be confused[1], which is why
> I refrained from "making every gitlink into a submodule". Specifically
> the more powerful a submodule operation is (the more fluff adds),
> the harder it should be for people to mis-use it.
> 
> [1] https://github.com/git-series/git-series/blob/master/INTERNALS.md
>  "git-series uses gitlinks to store pointer to commits in its own repo."

But would those users use

git add

to add a gitlink? From the description in that file I read that it
points to commits in its own repository. Will there also be files
checked out like submodules at that location?

Otherwise I would propose that 'git add' could detect whether a gitlink
is a submodule by trying to read its git configuration. If we do not
find that we simply do not do anything.

> > If everyone agrees that submodules are the default way of handling
> > repositories insided repositories, IMO, 'git add' should also alter
> > .gitmodules by default. We could provide a switch to avoid doing that.
> 
> I wonder if that switch should be default-on (i.e. not treat a gitlink as
> a submodule initially, behavior as-is, and then eventually we will
> die() on unconfigured repos, expecting the user to make the decision)
> 
> > An intermediate solution would be to warn
> 
> That is already implemented by Peff.

Ah ok, thanks I suspected so when I realized that this discussion was
older.

> > but in the long run my goal
> > for submodules is and always was: Make them behave as close to files as
> > possible. And why should a 'git add submodule' not magically do
> > everything it can to make submodules just work? I can look into a patch
> > for that if people agree here...
> 
> I'd love to see this implemented. I cc'd Josh (the author of git-series), who
> may disagree with this, or has some good input how to go forward without
> breaking git-series.

Yeah, lets see if, as described above, that actually would break
git-series.

> > Regarding handling of gitlinks with or without .gitmodules:
> >
> > Currently we are actually in some intermediate state:
> >
> >  * If there is no .gitmodules file: No submodule processing on any
> >gitlinks (AFAIK)
> 
> AFAIK this is true.
> 
> >  * If there is a .gitmodules files with some submodule configured: Do
> >recursive fetch and push as far as possible on gitlinks.
> 
> * If submodule.recurse is set, then we also treat submodules like files
>   for checkout, reset, read-tree.

To clarify: If submodule.recurse is set but there is no .gitmodules file
we do submodule processing for the above commands?

> > So I am not sure whether there are actually many users (knowingly)
> > using a mix of some submodules configured and some not and then relying
> > on the submodule infrastructure.
> >
> > I would rather expect two sorts of users:
> >
> >   1. Those that do use .gitmodules
> 
> Those want to reap all benefits of good submodules.
> 
> >
> >   2. Those that do *not* use .gitmodules
> 
> As said above, we don't know if those users are
> "holding submodules wrong" or are using gitlinks for
> magic tricks (unrelated to submodules).

I did not want to say that they are "holding submodules wrong" but
rather that if they do not use .gitmodules they do that knowingly and
thus consistently not use .gitmodules for any gitlink.

> > Users that do not use any .gitmodules file will currently (AFAIK) not
> > get any submodule handling. So the question is are there really many
> > "mixed users"? My guess would be no.
> 
> I hope that there are few (if any) users of these mixed setups.

That sounds promising.

> > Because without those using this mixed we could switch to saying: "You
> > need to have a .gitmodules file for submodule handling" without much
> > fallout from breaking users use cases.
> 
> That seems reasonable to me, actually.

Nice.

> > Maybe we can test this out somehow? My patch series would be ready in
> > that case, just had to drop the first patch and adjust the commit
> > message of this one.
> 
> I wonder how we would test this, though? Do you have any idea
> (even vague) how we'd accomplish such a measurement?
> I fear we'll have to go this way blindly.

One idea would be to expose this somewhere to a limited amount of users.
I remember Jonathan was suggesting, back when Jens was working on the
recursive checkout, that he could add the series to the debian package
and see what happens. Or we could use Junios next branch? Something like
that. If we get complaints we know the assumption was wrong and we need
a fallback.

Cheers Heiko

No log --no-decorate completion?

[Max Rothman]

I recently noticed that in the git-completion script, there's
completion for --decorate={full,yes,no} for git log and family, but
not for --no-decorate. Is that intentional? If not, I *think* I see
how it could be added.

Thanks,
Max

Re: [PATCH v4 4/4] sha1_name: minimize OID comparisons during disambiguation

[Derrick Stolee]

On 10/10/2017 9:30 AM, Jeff King wrote:

On Tue, Oct 10, 2017 at 09:11:15AM -0400, Derrick Stolee wrote:


On 10/10/2017 8:56 AM, Junio C Hamano wrote:

Jeff King  writes:


OK, I think that makes more sense. But note the p->num_objects thing I
mentioned. If I do:

git pack-objects .git/objects/pack/pack num_objects)
return;

Technically that also covers open_pack_index() failure, too, but that's
a subtlety I don't think we should rely on.

True.  I notice that the early part of the two functions look almost
identical.  Do we need error condition handling for the other one,
too?

I prefer to fix the problem in all code clones when they cause review
friction, so I'll send a fifth commit showing just the diff for these
packfile issues in sha1_name.c. See patch below.

Ah, that answers my earlier question. Junio mean unique_in_pack(). And
yeah, I think it suffers from the same problem.


Should open_pack_index() return a non-zero status if the packfile is empty?
Or, is there a meaningful reason to have empty packfiles?

I can't think of a compelling reason to have an empty packfile. But nor
do I think we should consider them an error when we can handle them
quietly (and returning non-zero status would cause Git to complain on
many operations in a repository that has such a file).

-Peff


Thanks for the comments. I found some typos in my commit messages, too.

I plan to send a (hopefully) final version tomorrow (Thursday). It will 
include:


* Make 'pos' unsigned in get_hex_char_from_oid()

* Check response from open_pack_index()

* Small typos in commit messages

If there are other issues, then please let me know.

Thanks,
-Stolee

Re: Enhancement request: git-push: Allow (configurable) default push-option

[Junio C Hamano]

Marius Paliga  writes:

> @@ -505,6 +509,12 @@ static int git_push_config(const char *k, const
> char *v, void *cb)
>  recurse_submodules = val;
>  }
>
> +default_push_options = git_config_get_value_multi("push.optiondefault");
> +if (default_push_options)
> +for_each_string_list_item(item, default_push_options)
> +if (!string_list_has_string(_options, item->string))
> +string_list_append(_options, item->string);
> +
>  return git_default_config(k, v, NULL);
>  }

Sorry for not catching this earlier, but git_config_get_value* call
inside git_push_config() is just wrong.

There are two styles of configuration parsing.  The original (and
still perfectly valid) way is to call git_config() with a callback
function like git_push_config().  Under this style, the config files
are read from lower-priority to higher-priority ones, and the
callback function is called once for each entry found, with 
pair and the callback specific opaque data.  One way to add the
parsing of a new variable like push.optiondefault is to add

if (!strcmp(k, "push.optiondefault") {
... handle one "[push] optiondefault" entry here ...
return 0;
}

to the function.

An alternate way is to use git_config_get_* functions _outside_
callback of git_config().  This is a newer invention.  Your call to
git_config_get_value_multi() will scan all configuration files and
returns _all_  entries for the given variable at once.

When there is already a callback style parser, in general, it is
cleaner to simply piggy-back on it, instead of reading variables
independently using git_config_get_* functions.  When there isn't a
callback style parser, using either style is OK.  It also is OK to
switch to git_config_get_* altogether, rewriting the callback style
parser, but I do not think it is warranted in this case, which adds
just one variable.

In any case, with the above code, you'll end up calling the
git_config_get_* function and grabbing all the values for
push.optiondefault for each and every configuration variable
definition (count "git config -l | wc -l" to estimate how many times
it will be called).  Which is probably not what you wanted to do.

Also, watch out for how a configuration variable defined like below
is reported to either of the above two styles:

[push]  optiondefault

 - To a git_config() callback function like git_push_config(), such
   an entry is called with k=="push.optiondefault", v==NULL.

 - git_config_get_value_multi() would return a string-list element
   with the string set to NULL to signal that one value is NULL
   (i.e. it is different from "[push] optiondefault = ").

I suspect that with your code, we'd hit

if (strchr(item->string, '\n'))

and end up dereferencing NULL right there.

> @@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const
> char *prefix)
>  int push_cert = -1;
>  int rc;
>  const char *repo = NULL;/* default repository */
> -struct string_list push_options = STRING_LIST_INIT_DUP;
>  const struct string_list_item *item;
>
>  struct option options[] = {

Also, I suspect that this code does not allow the command line
option to override the default set in the configuration file.
OPT_STRING_LIST() appends to the _options string list without
first clearing it, and you are pre-populating the list while reading
the configuration, so the values taken from the command line will
only add to them.

The right way to do this would probably be:

 - Do not muck with push_options in cmd_push().

 - Prepare another string list, push_options_from_config, that is
   file-scope global.

 - In git_push_config(), do not call get_multi; instead react to a
   call with k=="push.optionsdefault" and

   - reject if "v" is NULL, with "return config_error_nonbool(k);"

   - otherwise, append "v" to the "from-config" string list--do not
 attempt to dedup or sort.

   - if "v" is an empty string, clear the "from-config" list.

 - After parse_options() returns to cmd_push(), see if push_options
   is empty.  If it is, you did not get any command line option, so
   override it with what you collected in the "from-config" string
   list.  Otherwise, do not even look at "from-config" string list.

By the way, I really hate "push.optiondefault" as the variable
name.  The "default" part is obvious and there is no need to say it,
as the configuration variables are there to give the default to what
we would normally give from the command line.  Rather, you should
say for which option (there are many options "git push" takes) this
variable gives the default.  Perhaps "push.pushOption" is a much
better name; I am sure people can come up with even better ones,
though ;-)

[PATCH v2 5/5] Add tests around status handling of ignored arguments

[Jameson Miller]

Add tests for status handling of '--ignored=matching` and
`--untracked-files=normal`.

Signed-off-by: Jameson Miller 
---
 t/t7519-ignored-mode.sh | 60 -
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
index 76e91427b0..6be7701d79 100755
--- a/t/t7519-ignored-mode.sh
+++ b/t/t7519-ignored-mode.sh
@@ -116,10 +116,68 @@ test_expect_success 'Verify status behavior on ignored 
folder containing tracked
ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
ignored_dir/tracked &&
git add -f ignored_dir/tracked &&
-   test_tick &&
git commit -m "Force add file in ignored directory" &&
git status --porcelain=v2 --ignored=matching --untracked-files=all 
>output &&
test_i18ncmp expect output
 '
 
+test_expect_success 'Verify matching ignored files with 
--untracked-files=normal' '
+   test_when_finished "git clean -fdx" &&
+   cat >expect <<-\EOF &&
+   ? expect
+   ? output
+   ? untracked_dir/
+   ! ignored_dir/
+   ! ignored_files/ignored_1.ign
+   ! ignored_files/ignored_2.ign
+   EOF
+
+   mkdir ignored_dir ignored_files untracked_dir &&
+   touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+   ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+   untracked_dir/untracked &&
+   git status --porcelain=v2 --ignored=matching --untracked-files=normal 
>output &&
+   test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with 
--untracked-files=normal' '
+   test_when_finished "git clean -fdx" &&
+   cat >expect <<-\EOF &&
+   ? expect
+   ? output
+   ? untracked_dir/
+   ! ignored_dir/
+   ! ignored_files/ignored_1.ign
+   ! ignored_files/ignored_2.ign
+   EOF
+
+   mkdir ignored_dir ignored_files untracked_dir &&
+   touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+   ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+   untracked_dir/untracked &&
+   git status --porcelain=v2 --ignored=matching --untracked-files=normal 
>output &&
+   test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored folder containing 
tracked file' '
+   test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+   cat >expect <<-\EOF &&
+   ? expect
+   ? output
+   ! ignored_dir/ignored_1
+   ! ignored_dir/ignored_1.ign
+   ! ignored_dir/ignored_2
+   ! ignored_dir/ignored_2.ign
+   EOF
+
+   mkdir ignored_dir &&
+   touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+   ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+   ignored_dir/tracked &&
+   git add -f ignored_dir/tracked &&
+   git commit -m "Force add file in ignored directory" &&
+   git status --porcelain=v2 --ignored=matching --untracked-files=normal 
>output &&
+   test_i18ncmp expect output
+'
+
 test_done
-- 
2.13.6

[PATCH v2 3/5] Add tests for git status `--ignored=matching`

[Jameson Miller]

Add tests for new ignored mode (matching) when showing all untracked
files.

Signed-off-by: Jameson Miller 
---
 t/t7519-ignored-mode.sh | 125 
 1 file changed, 125 insertions(+)
 create mode 100755 t/t7519-ignored-mode.sh

diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
new file mode 100755
index 00..76e91427b0
--- /dev/null
+++ b/t/t7519-ignored-mode.sh
@@ -0,0 +1,125 @@
+#!/bin/sh
+
+test_description='git status collapse ignored'
+
+. ./test-lib.sh
+
+# commit initial ignore file
+test_expect_success 'setup initial commit and ignore file' '
+   cat >.gitignore <<-\EOF &&
+   *.ign
+   ignored_dir/
+   !*.unignore
+   EOF
+   git add . &&
+   git commit -m "Initial commit"
+'
+
+test_expect_success 'Verify behavior of status on folders with ignored files' '
+   test_when_finished "git clean -fdx" &&
+   cat >expect <<-\EOF &&
+   ? expect
+   ? output
+   ! dir/ignored/ignored_1.ign
+   ! dir/ignored/ignored_2.ign
+   ! ignored/ignored_1.ign
+   ! ignored/ignored_2.ign
+   EOF
+
+   mkdir -p ignored dir/ignored &&
+   touch ignored/ignored_1.ign ignored/ignored_2.ign \
+   dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+   git status --porcelain=v2 --ignored=matching --untracked-files=all 
>output &&
+   test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on folder with tracked & ignored 
files' '
+   test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+   cat >expect <<-\EOF &&
+   ? expect
+   ? output
+   ! dir/tracked_ignored/ignored_1.ign
+   ! dir/tracked_ignored/ignored_2.ign
+   ! tracked_ignored/ignored_1.ign
+   ! tracked_ignored/ignored_2.ign
+   EOF
+
+   mkdir -p tracked_ignored dir/tracked_ignored &&
+   touch tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+   tracked_ignored/ignored_1.ign tracked_ignored/ignored_2.ign \
+   dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 \
+   dir/tracked_ignored/ignored_1.ign 
dir/tracked_ignored/ignored_2.ign &&
+
+   git add tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+   dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 &&
+   git commit -m "commit tracked files" &&
+
+   git status --porcelain=v2 --ignored=matching --untracked-files=all 
>output &&
+   test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on folder with untracked and 
ignored files' '
+   test_when_finished "git clean -fdx" &&
+   cat >expect <<-\EOF &&
+   ? dir/untracked_ignored/untracked_1
+   ? dir/untracked_ignored/untracked_2
+   ? expect
+   ? output
+   ? untracked_ignored/untracked_1
+   ? untracked_ignored/untracked_2
+   ! dir/untracked_ignored/ignored_1.ign
+   ! dir/untracked_ignored/ignored_2.ign
+   ! untracked_ignored/ignored_1.ign
+   ! untracked_ignored/ignored_2.ign
+   EOF
+
+   mkdir -p untracked_ignored dir/untracked_ignored &&
+   touch untracked_ignored/untracked_1 untracked_ignored/untracked_2 \
+   untracked_ignored/ignored_1.ign untracked_ignored/ignored_2.ign 
\
+   dir/untracked_ignored/untracked_1 
dir/untracked_ignored/untracked_2 \
+   dir/untracked_ignored/ignored_1.ign 
dir/untracked_ignored/ignored_2.ign &&
+
+   git status --porcelain=v2 --ignored=matching --untracked-files=all 
>output &&
+   test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status matching ignored files on ignored folder' '
+   test_when_finished "git clean -fdx" &&
+   cat >expect <<-\EOF &&
+   ? expect
+   ? output
+   ! ignored_dir/
+   EOF
+
+   mkdir ignored_dir &&
+   touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+   ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign &&
+
+   git status --porcelain=v2 --ignored=matching --untracked-files=all 
>output &&
+   test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored folder containing 
tracked file' '
+   test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+   cat >expect <<-\EOF &&
+   ? expect
+   ? output
+   ! ignored_dir/ignored_1
+   ! ignored_dir/ignored_1.ign
+   ! ignored_dir/ignored_2
+   ! ignored_dir/ignored_2.ign
+   EOF
+
+   mkdir ignored_dir &&
+   touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+   ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+   ignored_dir/tracked &&
+   git add -f ignored_dir/tracked &&
+   test_tick &&
+   git commit -m "Force add file in ignored directory" &&
+   git status --porcelain=v2 --ignored=matching --untracked-files=all 
>output &&
+   test_i18ncmp expect output
+'
+

[PATCH v2 2/5] Update documentation for new directory and status logic

[Jameson Miller]

Signed-off-by: Jameson Miller 
---
 Documentation/git-status.txt  | 21 +-
 Documentation/technical/api-directory-listing.txt | 27 +++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f3a78a36c..fc282e0a92 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
(and suppresses the output of submodule summaries when the config option
`status.submoduleSummary` is set).
 
---ignored::
+--ignored[=]::
Show ignored files as well.
++
+The mode parameter is used to specify the handling of ignored files.
+It is optional: it defaults to 'traditional'.
++
+The possible options are:
++
+   - 'traditional' - Shows ignored files and directories, unless
+ --untracked-files=all is specifed, in which case
+ individual files in ignored directories are
+ displayed.
+   - 'no'  - Show no ignored files.
+   - 'matching'- Shows ignored files and directories matching an
+ ignore pattern.
++
+When 'matching' mode is specified, paths that explicity match an
+ignored pattern are shown. If a directory matches an ignore pattern,
+then it is shown, but not paths contained in the ignored directory. If
+a directory does not match an ignore pattern, but all contents are
+ignored, then the directory is not shown, but all contents are shown.
 
 -z::
Terminate entries with NUL, instead of LF.  This implies
diff --git a/Documentation/technical/api-directory-listing.txt 
b/Documentation/technical/api-directory-listing.txt
index 6c77b4920c..7fae00f44f 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -22,16 +22,20 @@ The notable options are:
 
 `flags`::
 
-   A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
+   A bit-field of options:
 
 `DIR_SHOW_IGNORED`:::
 
-   Return just ignored files in `entries[]`, not untracked files.
+   Return just ignored files in `entries[]`, not untracked
+   files. This flag is mutually exclusive with
+   `DIR_SHOW_IGNORED_TOO`.
 
 `DIR_SHOW_IGNORED_TOO`:::
 
-   Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
-   in addition to untracked files in `entries[]`.
+   Similar to `DIR_SHOW_IGNORED`, but return ignored files in
+   `ignored[]` in addition to untracked files in
+   `entries[]`. This flag is mutually exclusive with
+   `DIR_SHOW_IGNORED`.
 
 `DIR_KEEP_UNTRACKED_CONTENTS`:::
 
@@ -39,6 +43,21 @@ The notable options are:
untracked contents of untracked directories are also returned in
`entries[]`.
 
+`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
+
+   Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
+   this is set, returns ignored files and directories that match
+   an exclude pattern. If a directory matches an exclude pattern,
+   then the directory is returned and the contained paths are
+   not. A directory that does not match an exclude pattern will
+   not be returned even if all of its contents are ignored. In
+   this case, the contents are returned as individual entries.
++
+If this is set, files and directories that explicity match an ignore
+pattern are reported. Implicity ignored directories (directories that
+do not match an ignore pattern, but whose contents are all ignored)
+are not reported, instead all of the contents are reported.
+
 `DIR_COLLECT_IGNORED`:::
 
Special mode for git-add. Return ignored files in `ignored[]` and
-- 
2.13.6

[PATCH v2 4/5] Expand support for ignored arguments on status

[Jameson Miller]

Teach status command to handle matching ignored mode when showing
untracked files with the normal option.

Signed-off-by: Jameson Miller 
---
 dir.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index b9af87eca9..8636d080b2 100644
--- a/dir.c
+++ b/dir.c
@@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
 {
int exclude;
int has_path_in_index = !!index_file_exists(istate, path->buf, 
path->len, ignore_case);
+   enum path_treatment path_treatment;
 
if (dtype == DT_UNKNOWN)
dtype = get_dtype(de, istate, path->buf, path->len);
@@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
return path_none;
case DT_DIR:
strbuf_addch(path, '/');
-   return treat_directory(dir, istate, untracked, path->buf, 
path->len,
-  baselen, exclude, pathspec);
+   path_treatment = treat_directory(dir, istate, untracked,
+path->buf, path->len,
+baselen, exclude, pathspec);
+   /*
+* If we are only want to return directories that
+* match an exclude pattern, and this directories does
+* not match an exclude pattern but all contents are
+* excluded, then indicate that we should recurse into
+* this directory (instead of marking the directory
+* itself as an ignored path)
+*/
+   if (!exclude &&
+   path_treatment == path_excluded &&
+   (dir->flags & DIR_SHOW_IGNORED_TOO) &&
+   (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
+   return path_recurse;
+   return path_treatment;
case DT_REG:
case DT_LNK:
return exclude ? path_excluded : path_untracked;
-- 
2.13.6

[PATCH v2 0/5] Teach Status options around showing ignored files

[Jameson Miller]

Changes in v2:

Addressed review comments from the original series:

* Made fixes / simplifications suggested for documentation

* Removed test_tick from test scripts

* Merged 2 commits (as suggested)

Jameson Miller (5):
  Teach status options around showing ignored files
  Update documentation for new directory and status logic
  Add tests for git status `--ignored=matching`
  Expand support for ignored arguments on status
  Add tests around status handling of ignored arguments

 Documentation/git-status.txt  |  21 ++-
 Documentation/technical/api-directory-listing.txt |  27 +++-
 builtin/commit.c  |  31 +++-
 dir.c |  44 +-
 dir.h |   3 +-
 t/t7519-ignored-mode.sh   | 183 ++
 wt-status.c   |  11 +-
 wt-status.h   |   8 +-
 8 files changed, 310 insertions(+), 18 deletions(-)
 create mode 100755 t/t7519-ignored-mode.sh

-- 
2.13.6

[PATCH v2 1/5] Teach status options around showing ignored files

[Jameson Miller]

This change teaches the status command more options to control which
ignored files are reported independently of the which untracked files
are reported by allowing the `--ignored` option to take additional
arguments.  Currently, the shown ignored files are linked to how
untracked files are reported. Both ignored and untracked files are
controlled by arguments to `--untracked-files` option. This makes it
impossible to show all untracked files individually, but show ignored
"files and directories" (that is, ignored directories are shown as 1
entry, instead of having all contents shown).

Our application (Visual Studio) has a specific set of requirements
about how it wants untracked / ignored files reported by git status.
It requires all untracked files listed individually. It would like
ignored files and directories that explicity match an exclude pattern
listed. If an ignored directory matches an exclude pattern, then the
path of the directory should be returned. If a directory does not
match an exclude pattern, but all of its contents are ignored, then we
want the contained files reported instead of the directory.

The reason for controlling these behaviors separately is that there
can be a significant performance impact to scanning the contents of
excluded directories. Additionally, knowing whether a directory
explicitly matches an exclude pattern can help the application make
decisions about how to handle the directory. If an ignored directory
itself matches an exclude pattern, then the application will know that
any files underneath the directory must be ignored as well.

As a more concrete example, on Windows, Visual Studio creates a bin/
and obj/ directory inside of the project where it writes all .obj and
binary build output files. Normal usage is to explicitly ignore these
2 directory names in the .gitignore file (rather than or in addition
to *.obj). We just want to see the "bin/" and "obj/" paths regardless
of which "--untracked-files" flag is passed in. Additionally, if we
know the bin/ and obj/ directories are ignored, then we can ignore any
files changes we notice underneath these paths, as we know they do not
affect the output of stats.

Signed-off-by: Jameson Miller 
---
 builtin/commit.c | 31 +--
 dir.c| 24 
 dir.h|  3 ++-
 wt-status.c  | 11 ---
 wt-status.h  |  8 +++-
 5 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d75b3805ea..98d84d0277 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message;
-static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *untracked_files_arg, *force_date, *ignore_submodule_arg, 
*ignored_arg;
 static char *sign_commit;
 
 /*
@@ -139,7 +139,7 @@ static const char *cleanup_arg;
 static enum commit_whence whence;
 static int sequencer_in_use;
 static int use_editor = 1, include_status = 1;
-static int show_ignored_in_status, have_option_m;
+static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char 
*name)
die(_("--author '%s' is not 'Name ' and matches no existing 
author"), name);
 }
 
+static void handle_ignored_arg(struct wt_status *s)
+{
+   if (!ignored_arg)
+   ; /* default already initialized */
+   else if (!strcmp(ignored_arg, "traditional"))
+   s->show_ignored_mode = SHOW_TRADITIONAL_IGNORED;
+   else if (!strcmp(ignored_arg, "no"))
+   s->show_ignored_mode = SHOW_NO_IGNORED;
+   else if (!strcmp(ignored_arg, "matching"))
+   s->show_ignored_mode = SHOW_MATCHING_IGNORED;
+   else
+   die(_("Invalid ignored mode '%s'"), ignored_arg);
+}
 
 static void handle_untracked_files_arg(struct wt_status *s)
 {
@@ -1363,8 +1376,10 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
  N_("mode"),
  N_("show untracked files, optional modes: all, normal, no. 
(Default: all)"),
  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
-   OPT_BOOL(0, "ignored", _ignored_in_status,
-N_("show ignored files")),
+   { OPTION_STRING, 0, "ignored", _arg,
+ N_("mode"),
+ N_("show ignored files, optional modes: traditional, 
matching, no. (Default: traditional)"),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)"traditional" },
{ OPTION_STRING, 0, "ignore-submodules", _submodule_arg, 
N_("when"),
  N_("ignore changes to 

Re: Enhancement request: git-push: Allow (configurable) default push-option

[Junio C Hamano]

Marius Paliga  writes:

> +default_push_options = git_config_get_value_multi("push.optiondefault");
> +if (default_push_options)
> +for_each_string_list_item(item, default_push_options)
> +if (!string_list_has_string(_options, item->string))
> +string_list_append(_options, item->string);

One thing that is often overlooked is how to allow users to override
a multi-value configuration variable that gets some values from
lower priority configuration files (e.g. ~/.gitconfig) with
repository specific settings in .git/config, and the way we
typically do so is to define "When a variable definition with an
empty string is given, it is a signal to clear everything
accumulated so far."  E.g. if your ~/.gitconfig has

[push]
defaultPushOption = foo
defaultPushOption = bar

and then you write in your .git/config something like

[push]
defaultPushOption =
defaultPushOption = baz

The configuration mechanism reads from lower priority files and then
proceeds to read higher priority files, so the parser would read them
in this order:

push.defaultPushOption = foo
push.defaultPushOption = bar
push.defaultPushOption = 
push.defaultPushOption = baz

and then it would build a list ('foo'), then ('foo', 'bar'), and
clears it upon seeing an empty, and compute the final result as
('baz').

You may want to do something like that in this code.

Re: [PATCH v2 00/24] object_id part 10

[Junio C Hamano]

Michael Haggerty  writes:

> I took a stab at rebasing this patch series on top of current master
> using `git-imerge`. I pushed the results to my GitHub fork [1] as branch
> `object-id-part-10-rebased`. I didn't check the results very carefully,
> nor whether the commit messages need adjusting, but I did verify that
> each of the commits passes the test suite. Junio, it might serve as
> something to compare against your conflict resolution.
>
> Michael
>
> [1] https://github.com/mhagger/git

2f0e14e6 ("Merge branch 'js/rebase-i-final'", 2017-10-09) is where
your -rebased series has forked from the mainline.  I checked that
fork-point out, merged bc/object-id I queued to it, with the help
from rerere I earlier taught.  The resulting tree was identical to
the tip of your rebase.

So hopefully both of us should be good ;-)  I do not know about the
intermediate steps, though.

Thanks.

Re: [PATCH v2 00/24] object_id part 10

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> This is the tenth in a series of patches to convert from unsigned char
> [20] to struct object_id.  This series mostly involves changes to the
> refs code.  After these changes, there are almost no references to
> unsigned char in the main refs code.
> 
> The series has not been rebased on master since the last submission, but
> I can do so if that's more convenient.
> 
> This series is available from the following URL:
> https://github.com/bk2204/git.git object-id-part10

I read through the whole series medium-thoroughly and left a few
comments, but overall it looks very good and clear. Thanks so much for
working on this!

I took a stab at rebasing this patch series on top of current master
using `git-imerge`. I pushed the results to my GitHub fork [1] as branch
`object-id-part-10-rebased`. I didn't check the results very carefully,
nor whether the commit messages need adjusting, but I did verify that
each of the commits passes the test suite. Junio, it might serve as
something to compare against your conflict resolution.

Michael

[1] https://github.com/mhagger/git

$850.000.00 Donation !.

[Mark J. Shapiro]

David & Maureen picked you for $850.000.00 Donation, Kindly reply for details 
and claim.

Re: Enhancement request: git-push: Allow (configurable) default push-option

[Marius Paliga]

Found one possible issue when looking for duplicates, we need to use
  "unsorted_string_list_has_string" instead of "string_list_has_string"

-   if (!string_list_has_string(_options,
item->string))
+   if
(!unsorted_string_list_has_string(_options, item->string)) {

New (fixed) patch follows...


Signed-off-by: Marius Paliga 
---
 Documentation/git-push.txt |  3 +++
 builtin/push.c | 11 ++-
 t/t5545-push-options.sh| 48 ++
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 3e76e99f3..133c42183 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -161,6 +161,9 @@ already exists on the remote side.
 Transmit the given string to the server, which passes them to
 the pre-receive as well as the post-receive hook. The given string
 must not contain a NUL or LF character.
+Default push options can also be specified with configuration
+variable `push.optiondefault`. String(s) specified here will always
+be passed to the server without need to specify it using `--push-option`

 --receive-pack=::
 --exec=::
diff --git a/builtin/push.c b/builtin/push.c
index 2ac810422..ab458419a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -32,6 +32,8 @@ static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;

+static struct string_list push_options = STRING_LIST_INIT_DUP;
+
 static void add_refspec(const char *ref)
 {
 refspec_nr++;
@@ -467,6 +469,8 @@ static int git_push_config(const char *k, const
char *v, void *cb)
 {
 int *flags = cb;
 int status;
+const struct string_list *default_push_options;
+struct string_list_item *item;

 status = git_gpg_config(k, v, NULL);
 if (status)
@@ -505,6 +509,12 @@ static int git_push_config(const char *k, const
char *v, void *cb)
 recurse_submodules = val;
 }

+default_push_options = git_config_get_value_multi("push.optiondefault");
+if (default_push_options)
+for_each_string_list_item(item, default_push_options)
+if (!unsorted_string_list_has_string(_options, item->string))
+string_list_append(_options, item->string);
+
 return git_default_config(k, v, NULL);
 }

@@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const
char *prefix)
 int push_cert = -1;
 int rc;
 const char *repo = NULL;/* default repository */
-struct string_list push_options = STRING_LIST_INIT_DUP;
 const struct string_list_item *item;

 struct option options[] = {
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index 90a4b0d2f..575f3dc38 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -140,6 +140,54 @@ test_expect_success 'push options and submodules' '
 test_cmp expect parent_upstream/.git/hooks/post-receive.push_options
 '

+test_expect_success 'default push option' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.optiondefault=default push up master
+) &&
+test_refs master master &&
+echo "default" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'two default push options' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.optiondefault=default1 -c
push.optiondefault=default2 push up master
+) &&
+test_refs master master &&
+printf "default1\ndefault2\n" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'default and manual push options' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.optiondefault=default push --push-option=manual up master
+) &&
+test_refs master master &&
+printf "default\nmanual\n" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd

-- 
2.14.1

2017-10-11 9:14 GMT+02:00 Marius Paliga :
> Including proposed patch...
>
>
> Signed-off-by: Marius Paliga 
> ---
>  Documentation/git-push.txt |  3 +++
>  builtin/push.c | 11 ++-
>  

slight addition to t.gummerer's proposed "git stash" patch

[Robert P. J. Day]

  was perusing thomas gummerer's proposed "git stash" patch here:

https://www.spinics.net/lists/git/msg313993.html

and i'd make one more change -- i'd separate the OPTIONS entries for
"git stash push" and "git stash save" so they don't end up being
rendered all crushed together when displaying the man page:

OPTIONS
   save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet]
   [], push [-p|--patch] [-k|--[no-]keep-index] 
[-u|--include-untracked] [-a|--all]
   [-q|--quiet] [-m|--message ] [--] [...]
   Save your local modifications to a new stash and roll them back to HEAD 
(in the working
   tree and in the index). The  part is optional and gives the 
description along
   with the stashed state.
   ... snip ...

so rather than:

OPTIONS
---

push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
[-a|--all] [-q|--quiet] [-m|--message ] [--]
[...]::
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
[-a|--all] [-q|--quiet] []::

Save your local modifications to a new 'stash entry' and roll them
back to HEAD (in the working tree and in the index).
The  part is optional and gives
the description along with the stashed state.
...

i'd suggest:

push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
[-a|--all] [-q|--quiet] [-m|--message ] [--]
[...]::

Save your local modifications to a new 'stash entry' and roll them
back to HEAD (in the working tree and in the index).
The  part is optional and gives
the description along with the stashed state.
...

save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]
[-a|--all] [-q|--quiet] []::

This option is deprecated in favour of 'git stash push'.


or something like that.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

[PATCH] doc: emphasize stash "--keep-index" stashes staged content

[Robert P. J. Day]

It's not immediately obvious from the man page that the "--keep-index"
option still adds the staged content to the stash, so make that
abundantly clear.

Signed-off-by: Robert P. J. Day 

---

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 00f95fee1..037144037 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -68,8 +68,8 @@ entries and working tree files are then rolled back to the 
state in
 HEAD only for these files, too, leaving files that do not match the
 pathspec intact.
 +
-If the `--keep-index` option is used, all changes already added to the
-index are left intact.
+If the `--keep-index` option is used, all changes already staged in the
+index are left intact in the index, while still being added to the stash.
 +
 If the `--include-untracked` option is used, all untracked files are also
 stashed and then cleaned up with `git clean`, leaving the working directory

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [PATCH v2 24/24] refs/files-backend: convert static functions to object_id

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> Convert several static functions to take pointers to struct object_id.
> Change the relevant parameters to write_packed_entry to be const, as we
> don't modify them.  Rename lock_ref_sha1_basic to lock_ref_oid_basic to
> reflect its new argument.
> 
> Signed-off-by: brian m. carlson 
> ---
>  refs/files-backend.c | 48 
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 7281f27f62..84d8e3da99 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -811,7 +811,7 @@ static struct ref_iterator *files_ref_iterator_begin(
>   * return a negative value.
>   */

The docstring for this function needs adjusting. It would also be worth
mentioning that `old_oid` is allowed to be NULL (which was true before
but wasn't mentioned).

> [...]

Michael

Re: "git rm" seems to do recursive removal even without "-r"

[Robert P. J. Day]

On Tue, 10 Oct 2017, Heiko Voigt wrote:

> On Sun, Oct 08, 2017 at 07:56:20AM -0400, Robert P. J. Day wrote:
> >   but as i asked in my earlier post, if i wanted to remove *all* files
> > with names of "Makefile*", why can't i use:
> >
> >   $ git rm 'Makefile*'
> >
> > just as i used:
> >
> >   $ git rm '*.c'
> >
> > are those not both acceptable fileglobs? why does the former
> > clearly only match the top-level Makefile, and refuse to cross
> > directory boundaries?
>
> Maybe think about it this way: The only difference between git's
> globbing and the default shell globbing is that the '/' in a path
> has a special meaning. The shells expansion stops at a '/' but git
> does not.
>
> So with *.c the shell matches: blabla.c, blub.c, ...  but not
> subdir/bla.c, subdir/blub.c, ... since it only considers files in
> the current directory. A little different for Makefile* that will
> also match Makefile.bla, Makefile/bla or Makefile_bla/blub in shell
> but not subdir/Makefile or bla.Makefile. Basically anything directly
> in *this* directory that *starts* with 'Makefile'.
>
> Git on the other hand does not consider '/' to be special. So *.c
> matches all of the path above: bla.c, blub.c, subdir/bla.c,
> subdir/blub.c. Basically any file below the current directory with a
> path that ends in '.c'. With Makefile* it is the opposite: Every
> file below the current directory that *starts* with 'Makefile'. So
> Makefile.bla, Makefile/bla, ... but also not subdir/Makefile or
> bla.Makefile.

   ok, i believe i finally appreciate what is happening here, and
perhaps my first contribution will be a minor addition to the "git-rm"
man page to introduce a couple examples explaining these intricacies,
since they're not immediately obvious. i'll put something together and
submit it to the list. thank you all for your patience in explaining
this.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [PATCH v2 23/24] refs: convert read_raw_ref backends to struct object_id

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> Convert the unsigned char * parameter to struct object_id * for
> files_read_raw_ref and packed_read_raw-ref.  Update the documentation.

Nit:

s/packed_read_raw-ref/packed_read_raw_ref/

> Switch from using get_sha1_hex and a hard-coded 40 to using
> parse_oid_hex.
> [...]

Michael

Re: [PATCH v2 21/24] refs: convert resolve_ref_unsafe to struct object_id

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> Convert resolve_ref_unsafe to take a pointer to struct object_id by
> converting one remaining caller to use struct object_id, converting the
> declaration and definition, and applying the following semantic patch:
> 
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_ref_unsafe(E1, E2, E3.hash, E4)
> + resolve_ref_unsafe(E1, E2, , E4)
> 
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_ref_unsafe(E1, E2, E3->hash, E4)
> + resolve_ref_unsafe(E1, E2, E3, E4)
> 
> Signed-off-by: brian m. carlson 
> ---
>  blame.c   |  4 ++--
>  builtin/fsck.c|  2 +-
>  refs.c| 28 ++--
>  refs.h|  4 ++--
>  refs/files-backend.c  |  8 
>  sequencer.c   |  2 +-
>  t/helper/test-ref-store.c |  6 +++---
>  transport-helper.c|  7 +++
>  worktree.c|  2 +-
>  9 files changed, 31 insertions(+), 32 deletions(-)
> 
> [...]
> diff --git a/refs.h b/refs.h
> index bb0dcd97af..4eedc880c6 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -62,10 +62,10 @@ struct worktree;

The docstring above this hunk needs to be adjusted.

>  const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>   const char *refname,
>   int resolve_flags,
> - unsigned char *sha1,
> + struct object_id *oid,
>   int *flags);
>  const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
> -unsigned char *sha1, int *flags);
> +struct object_id *oid, int *flags);
>  
>  char *refs_resolve_refdup(struct ref_store *refs,
> const char *refname, int resolve_flags,
> [...]
Michael

Re: [PATCH v2 14/24] refs: convert peel_ref to struct object_id

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> Convert peel_ref (and its corresponding backend) to struct object_id.
> 
> This transformation was done with an update to the declaration,
> definition, and test helper and the following semantic patch:
> 
> @@
> expression E1, E2;
> @@
> - peel_ref(E1, E2.hash)
> + peel_ref(E1, )
> 
> @@
> expression E1, E2;
> @@
> - peel_ref(E1, E2->hash)
> + peel_ref(E1, E2)
> 
> Signed-off-by: brian m. carlson 
> ---
>  builtin/describe.c| 2 +-
>  builtin/pack-objects.c| 4 ++--
>  builtin/show-ref.c| 2 +-
>  refs.c| 8 
>  refs.h| 4 ++--
>  refs/files-backend.c  | 8 
>  refs/packed-backend.c | 4 ++--
>  refs/refs-internal.h  | 2 +-
>  t/helper/test-ref-store.c | 6 +++---
>  upload-pack.c | 2 +-
>  10 files changed, 21 insertions(+), 21 deletions(-)
> 
> [...]
> diff --git a/refs.h b/refs.h
> index 8159b7b067..832ade2b13 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -120,8 +120,8 @@ extern int refs_init_db(struct strbuf *err);
>   * ultimately resolve to a peelable tag.
>   */

The comment just above needs to be adjusted.

>  int refs_peel_ref(struct ref_store *refs, const char *refname,
> -   unsigned char *sha1);
> -int peel_ref(const char *refname, unsigned char *sha1);
> +   struct object_id *oid);
> +int peel_ref(const char *refname, struct object_id *oid);
>  
>  /**
>   * Resolve refname in the nested "gitlink" repository in the specified
> [...]

Michael

[ANNOUNCE] Git Rev News edition 32

[Christian Couder]

Hi everyone,

The 32st edition of Git Rev News is now published:

  https://git.github.io/rev_news/2017/10/11/edition-32/

Thanks a lot to all the contributors and helpers!

Enjoy,
Christian, Thomas, Jakub and Markus.

Re: Enhancement request: git-push: Allow (configurable) default push-option

[Marius Paliga]

Including proposed patch...


Signed-off-by: Marius Paliga 
---
 Documentation/git-push.txt |  3 +++
 builtin/push.c | 11 ++-
 t/t5545-push-options.sh| 48 ++
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 3e76e99f3..133c42183 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -161,6 +161,9 @@ already exists on the remote side.
 Transmit the given string to the server, which passes them to
 the pre-receive as well as the post-receive hook. The given string
 must not contain a NUL or LF character.
+Default push options can also be specified with configuration
+variable `push.optiondefault`. String(s) specified here will always
+be passed to the server without need to specify it using `--push-option`

 --receive-pack=::
 --exec=::
diff --git a/builtin/push.c b/builtin/push.c
index 2ac810422..4dd5d6f0e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -32,6 +32,8 @@ static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;

+static struct string_list push_options = STRING_LIST_INIT_DUP;
+
 static void add_refspec(const char *ref)
 {
 refspec_nr++;
@@ -467,6 +469,8 @@ static int git_push_config(const char *k, const
char *v, void *cb)
 {
 int *flags = cb;
 int status;
+const struct string_list *default_push_options;
+struct string_list_item *item;

 status = git_gpg_config(k, v, NULL);
 if (status)
@@ -505,6 +509,12 @@ static int git_push_config(const char *k, const
char *v, void *cb)
 recurse_submodules = val;
 }

+default_push_options = git_config_get_value_multi("push.optiondefault");
+if (default_push_options)
+for_each_string_list_item(item, default_push_options)
+if (!string_list_has_string(_options, item->string))
+string_list_append(_options, item->string);
+
 return git_default_config(k, v, NULL);
 }

@@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const
char *prefix)
 int push_cert = -1;
 int rc;
 const char *repo = NULL;/* default repository */
-struct string_list push_options = STRING_LIST_INIT_DUP;
 const struct string_list_item *item;

 struct option options[] = {
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index 90a4b0d2f..575f3dc38 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -140,6 +140,54 @@ test_expect_success 'push options and submodules' '
 test_cmp expect parent_upstream/.git/hooks/post-receive.push_options
 '

+test_expect_success 'default push option' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.optiondefault=default push up master
+) &&
+test_refs master master &&
+echo "default" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'two default push options' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.optiondefault=default1 -c
push.optiondefault=default2 push up master
+) &&
+test_refs master master &&
+printf "default1\ndefault2\n" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'default and manual push options' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.optiondefault=default push --push-option=manual up master
+) &&
+test_refs master master &&
+printf "default\nmanual\n" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd

-- 
2.14.1


2017-10-04 17:20 GMT+02:00 Marius Paliga :
> Hi Stefan,
>
> I will look at it.
>
> Thanks,
> Marius
>
>
> 2017-10-03 18:53 GMT+02:00 Stefan Beller :
>> On Tue, Oct 3, 2017 at 3:15 AM, Marius Paliga  
>> wrote:
>>> There is a need to pass predefined push-option during "git push"
>>> without need to specify it explicitly.
>>>
>>> In another words we need to have a new "git config" variable to
>>> specify string that will be automatically passed as "--push-option"
>>> when pushing to remote.
>>>
>>> 

Re: [PATCH v2 05/24] refs: update ref transactions to use struct object_id

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> Update the ref transaction code to use struct object_id.  Remove one
> NULL pointer check which was previously inserted around a dereference;
> since we now pass a pointer to struct object_id directly through, the
> code we're calling handles this for us.
> 
> Signed-off-by: brian m. carlson 
> ---
>  branch.c   |  2 +-
>  builtin/clone.c|  2 +-
>  builtin/commit.c   |  4 ++--
>  builtin/fetch.c|  4 ++--
>  builtin/receive-pack.c |  4 ++--
>  builtin/replace.c  |  2 +-
>  builtin/tag.c  |  2 +-
>  builtin/update-ref.c   |  8 
>  fast-import.c  |  4 ++--
>  refs.c | 44 +---
>  refs.h | 24 
>  refs/files-backend.c   | 12 ++--
>  refs/refs-internal.h   |  4 ++--
>  sequencer.c|  2 +-
>  walker.c   |  2 +-
>  15 files changed, 59 insertions(+), 61 deletions(-)
> 
> [...]
> diff --git a/refs.h b/refs.h
> index 369614d392..543dcc5956 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -519,15 +519,15 @@ struct ref_transaction *ref_transaction_begin(struct 
> strbuf *err);
>   */
>  int ref_transaction_update(struct ref_transaction *transaction,

The docstring for this function needs to be updated.

>  const char *refname,
> -const unsigned char *new_sha1,
> -const unsigned char *old_sha1,
> +const struct object_id *new_oid,
> +const struct object_id *old_oid,
>  unsigned int flags, const char *msg,
>  struct strbuf *err);
>  
> [...]

Michael

What's cooking in git.git (Oct 2017, #02; Wed, 11)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

2.15-rc1 has been tagged.  Some of the topic marked "Will merge to
'master'" below might still become part of 2.15 final, while others
may be left to be merged after the final, to become part of the
release after that one (2.16?).

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* ar/request-pull-phrasofix (2017-10-03) 1 commit
  (merged to 'next' on 2017-10-03 at 07807bba90)
 + request-pull: capitalise "Git" to make it a proper noun

 Spell the name of our system as "Git" in the output from
 request-pull script.


* ds/avoid-overflow-in-midpoint-computation (2017-10-10) 1 commit
  (merged to 'next' on 2017-10-10 at 6279867a5d)
 + cleanup: fix possible overflow errors in binary search

 Code clean-up.


* er/fast-import-dump-refs-on-checkpoint (2017-09-29) 1 commit
  (merged to 'next' on 2017-10-03 at 4e7b0e7ec1)
 + fast-import: checkpoint: dump branches/tags/marks even if object_count==0

 The checkpoint command "git fast-import" did not flush updates to
 refs and marks unless at least one object was created since the
 last checkpoint, which has been corrected, as these things can
 happen without any new object getting created.


* hn/string-list-doc (2017-10-06) 1 commit
  (merged to 'next' on 2017-10-10 at d214a7c369)
 + api-argv-array.txt: remove broken link to string-list API

 Docfix.


* jk/refs-df-conflict (2017-10-07) 2 commits
  (merged to 'next' on 2017-10-10 at d5953a1ced)
 + refs_resolve_ref_unsafe: handle d/f conflicts for writes
 + t3308: create a real ref directory/file conflict

 An ancient bug that made Git misbehave with creation/renaming of
 refs has been fixed.


* jk/sha1-loose-object-info-fix (2017-10-06) 1 commit
  (merged to 'next' on 2017-10-10 at bcfd9c4e3f)
 + sha1_loose_object_info: handle errors from unpack_sha1_rest

 Leakfix and futureproofing.


* jk/ui-color-always-to-auto (2017-10-04) 3 commits
  (merged to 'next' on 2017-10-05 at 792ae936cf)
 + Merge branch 'jk/ui-color-always-to-auto-maint' into 
jk/ui-color-always-to-auto
 + t7301: use test_terminal to check color
 + t4015: use --color with --color-moved
 (this branch uses jk/ui-color-always-to-auto-maint.)

 Fix regression of "git add -p" for users with "color.ui = always"
 in their configuration, by merging the topic below and adjusting it
 for the 'master' front.


* jk/ui-color-always-to-auto-maint (2017-10-04) 10 commits
 + color: make "always" the same as "auto" in config
 + provide --color option for all ref-filter users
 + t3205: use --color instead of color.branch=always
 + t3203: drop "always" color test
 + t6006: drop "always" color config tests
 + t7502: use diff.noprefix for --verbose test
 + t7508: use test_terminal for color output
 + t3701: use test-terminal to collect color output
 + t4015: prefer --color to -c color.diff=always
 + test-terminal: set TERM=vt100
 (this branch is used by jk/ui-color-always-to-auto.)

 Users with "color.ui = always" in their configuration were broken
 by a recent change that made plumbing commands to pay attention to
 them as the patch created internally by "git add -p" were colored
 (heh) and made unusable.  Fix this regression by redefining
 'always' to mean the same thing as 'auto'.


* jn/strbuf-doc-re-reuse (2017-10-04) 1 commit
  (merged to 'next' on 2017-10-04 at 5940d412d9)
 + strbuf doc: reuse after strbuf_release is fine


* jr/hash-migration-plan-doc (2017-09-28) 1 commit
  (merged to 'next' on 2017-10-04 at b47b3bb656)
 + technical doc: add a design doc for hash function transition

 Lay out plans for weaning us off of SHA-1.


* jt/oidmap (2017-10-01) 1 commit
  (merged to 'next' on 2017-10-05 at e41445fc33)
 + oidmap: map with OID as key

 Introduce a new "oidmap" API and rewrite oidset to use it.


* ks/branch-tweak-error-message-for-extra-args (2017-10-04) 1 commit
  (merged to 'next' on 2017-10-05 at aa0b656bf0)
 + branch: change the error messages to be more meaningful

 Error message tweak.


* ks/verify-filename-non-option-error-message-tweak (2017-10-04) 1 commit
  (merged to 'next' on 2017-10-05 at 2a7030f6fe)
 + setup: update error message to be more meaningful

 Error message tweak.


* ls/filter-process-delayed (2017-10-10) 5 commits
  (merged to 'next' on 2017-10-10 at 7b26d72991)
 + write_entry: untangle symlink and regular-file cases
 + write_entry: avoid reading blobs in CE_RETRY case
 + write_entry: fix leak when retrying delayed filter
 + entry.c: check if file exists after checkout
 + entry.c: update cache entry only for existing files

 Bugfixes to an already graduated series.


* 

[ANNOUNCE] Git v2.15.0-rc1

[Junio C Hamano]

A release candidate Git v2.15.0-rc1 is now available for testing
at the usual places.  It is comprised of 721 non-merge commits
since v2.14.0, contributed by 72 people, 22 of which are new faces.

The tarballs are found at:

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

The following public repositories all have a copy of the
'v2.15.0-rc1' tag and the 'master' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.14.0 are as follows.
Welcome to the Git development community!

  Ann T Ropea, Daniel Watkins, Derrick Stolee, Dimitrios
  Christidis, Eric Rannaud, Evan Zacks, Hielke Christian Braun,
  Ian Campbell, Ilya Kantor, Jameson Miller, Job Snijders, Joel
  Teichroeb, joernchen, Łukasz Gryglicki, Manav Rathi, Martin
  Ågren, Michael Forney, Patryk Obara, Randall S. Becker, Ross
  Kabus, Taylor Blau, and Urs Thuermann.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Adam Dinwoodie, Ævar Arnfjörð Bjarmason, Andreas Heiduk,
  Anthony Sottile, Ben Boeckel, Brandon Casey, Brandon Williams,
  brian m. carlson, Christian Couder, Eric Blake, Han-Wen Nienhuys,
  Heiko Voigt, Jean-Noel Avila, Jeff Hostetler, Jeff King, Johannes
  Schindelin, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
  Junio C Hamano, Kaartic Sivaraam, Kevin Daudt, Kevin Willford,
  Lars Schneider, Martin Koegler, Matthieu Moy, Max Kirillov,
  Michael Haggerty, Michael J Gruber, Nguyễn Thái Ngọc Duy,
  Nicolas Morey-Chaisemartin, Øystein Walle, Paolo Bonzini,
  Pat Thoyts, Philip Oakley, Phillip Wood, Raman Gupta, Ramsay
  Jones, René Scharfe, Sahil Dua, Santiago Torres, Stefan Beller,
  Stephan Beyer, Takashi Iwai, Thomas Braun, Thomas Gummerer,
  Todd Zullinger, Tom G. Christensen, Torsten Bögershausen,
  and William Duclot.



Git 2.15 Release Notes (draft)
==

Backward compatibility notes and other notable changes.

 * Use of an empty string as a pathspec element that is used for
   'everything matches' is still warned and Git asks users to use a
   more explicit '.' for that instead.  The hope is that existing
   users will not mind this change, and eventually the warning can be
   turned into a hard error, upgrading the deprecation into removal of
   this (mis)feature.  That is now scheduled to happen in Git v2.16,
   the next major release after this one.

 * Git now avoids blindly falling back to ".git" when the setup
   sequence said we are _not_ in Git repository.  A corner case that
   happens to work right now may be broken by a call to BUG().
   We've tried hard to locate such cases and fixed them, but there
   might still be cases that need to be addressed--bug reports are
   greatly appreciated.

 * "branch --set-upstream" that has been deprecated in Git 1.8 has
   finally been retired.


Updates since v2.14
---

UI, Workflows & Features

 * An example that is now obsolete has been removed from a sample hook,
   and an old example in it that added a sign-off manually has been
   improved to use the interpret-trailers command.

 * The advice message given when "git rebase" stops for conflicting
   changes has been improved.

 * The "rerere-train" script (in contrib/) learned the "--overwrite"
   option to allow overwriting existing recorded resolutions.

 * "git contacts" (in contrib/) now lists the address on the
   "Reported-by:" trailer to its output, in addition to those on
   S-o-b: and other trailers, to make it easier to notify (and thank)
   the original bug reporter.

 * "git rebase", especially when it is run by mistake and ends up
   trying to replay many changes, spent long time in silence.  The
   command has been taught to show progress report when it spends
   long time preparing these many changes to replay (which would give
   the user a chance to abort with ^C).

 * "git merge" learned a "--signoff" option to add the Signed-off-by:
   trailer with the committer's name.

 * "git diff" learned to optionally paint new lines that are the same
   as deleted lines elsewhere differently from genuinely new lines.

 * "git interpret-trailers" learned to take the trailer specifications
   from the command line that overrides the configured values.

 * "git interpret-trailers" has been taught a "--parse" and a few
   other options to make it easier for scripts to grab existing
   trailer lines from a commit log message.

 * The "--format=%(trailers)" option "git log" and its friends take
   learned to take the 'unfold' and 'only' modifiers to normalize its
   output, e.g. "git log --format=%(trailers:only,unfold)".

 * "gitweb" shows a link to visit the 'raw' contents of blbos in the
   history overview page.

 * "[gc] rerereResolved = 5.days" used to be invalid, as the variable
   is defined 

Re: [PATCH v2 04/24] refs: convert update_ref and refs_update_ref to use struct object_id

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> Convert update_ref, refs_update_ref, and write_pseudoref to use struct
> object_id.  Update the existing callers as well.  Remove update_ref_oid,
> as it is no longer needed.
> 
> Signed-off-by: brian m. carlson 
> ---
>  bisect.c  |  5 +++--
>  builtin/am.c  | 14 +++---
>  builtin/checkout.c|  3 +--
>  builtin/clone.c   | 14 +++---
>  builtin/merge.c   | 13 ++---
>  builtin/notes.c   | 10 +-
>  builtin/pull.c|  2 +-
>  builtin/reset.c   |  4 ++--
>  builtin/update-ref.c  |  2 +-
>  notes-cache.c |  2 +-
>  notes-utils.c |  2 +-
>  refs.c| 40 
>  refs.h|  5 +
>  sequencer.c   |  9 +++--
>  t/helper/test-ref-store.c | 10 +-
>  transport-helper.c|  3 ++-
>  transport.c   |  4 ++--
>  17 files changed, 64 insertions(+), 78 deletions(-)
> 
> [...]
> diff --git a/refs.c b/refs.c
> index 0a5b68d6fb..51942df7b3 100644
> --- a/refs.c
> +++ b/refs.c
> [...]
> @@ -1003,12 +995,12 @@ int refs_update_ref(struct ref_store *refs, const char 
> *msg,
>   int ret = 0;
>  
>   if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
> - assert(refs == get_main_ref_store());

Was the deletion of the line above intentional?

> - ret = write_pseudoref(refname, new_sha1, old_sha1, );
> + ret = write_pseudoref(refname, new_oid, old_oid, );

This is not new to your code, but I just noticed a problem here.
`refs_update_ref()` is documented, via its reference to
`ref_transaction_update()`, to allow `new_sha1` (i.e., now `new_oid`) to
be NULL. (NULL signifies that the value of the reference shouldn't be
changed.)

But `write_pseudoref()` dereferences its `oid` argument unconditionally,
so this call would fail if `new_oid` is NULL.

This has all been the case since `write_pseudoref()` was introduced in
74ec19d4be (pseudorefs: create and use pseudoref update and delete
functions, 2015-07-31).

In my opinion, `write_pseudoref()` is broken. It should accept NULL as
its `oid` argument.

>   } else {
>   t = ref_store_transaction_begin(refs, );
>   if (!t ||
> - ref_transaction_update(t, refname, new_sha1, old_sha1,
> + ref_transaction_update(t, refname, new_oid ? new_oid->hash 
> : NULL,
> +old_oid ? old_oid->hash : NULL,
>  flags, msg, ) ||
>   ref_transaction_commit(t, )) {
>   ret = 1;
> [...]

Michael

Re: [PATCH v3] run-command: add hint when a hook is ignored

[Junio C Hamano]

Junio C Hamano  writes:

> Quite honestly, I do not particulary think this is confusing, and I
> expect that this change will irritate many people by forcing them to
> either set the advise config or move the ones that they deliberately
> left unexecutable by renaming them by adding ".disabled" at the end.
>
> But these remedies are easy enough, so let's see how well it works
> by merging it to 'next' and cooking it there for a while.

Well, it turns out that I am among those who are irritated, as all
the repositories I work with were rather old, dating back to 2005,
back when it was a norm to have these sample files installed without
executable bit, to make it easy for those who choose to use them
as-is to enable them by flipping the executable bit.

And I do not find the advice.ignoredhook is giving particularly a
good piece of advice.  I suspect that it would be a better practice
to rename a disabled foo-hook to foo-hook.disabled if the user wants
to squelch the warning.  It gives them a final chance to review what
they left disabled for all these years, and then choose to either
remove it, or rename it to foo-hook.disabled.  "ls .git/hooks/" will
then make it clear which ones are disabled without the "-F" option,
which is an additional benefit.

Anyway, I am not merging this topic to the upcoming release, so
hopefully we'll hear from others who try 'next'.

Thanks.

Re: [PATCH 1/2] describe: do not use cmd_*() as a subroutine

[Junio C Hamano]

SZEDER Gábor  writes:

> Maybe you already considered all this WRT that cmd_name_rev() call, I
> don't know.  In any case, I think at least the subject line should
> spell out cmd_diff_index().

Perhaps.  The intent was to eradicate cmd_*() used as a subroutine,
so I'd rather opt to explain why name_rev() that does not return is
OK in this case in the log message without changing anything else.

Thanks for looking it over.