[PATCH v3 0/1] doc: Add a note about ~/.zsh/_git file

2019-10-15 Thread Max Belsky via GitGitGadget
Hey,

Today I've spent a few hours to understand why git-completion doesn't work
in my zsh shell. It was because I thought ~/.zsh/_git should be a dictionary
with git-completion.zsh file. 

I think this change may save some hours for someone else.

Maxim Belsky (1):
  doc: Change zsh git completion file name

 contrib/completion/git-completion.zsh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


base-commit: 108b97dc372828f0e72e56bbb40cae8e1e83ece6
Published-As: 
https://github.com/gitgitgadget/git/releases/tag/pr-367%2Fmbelsky%2Fpatch-1-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-367/mbelsky/patch-1-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/367

Range-diff vs v2:

 1:  3f994f3b9a ! 1:  7919addea8 doc: Change zsh git completion file name
 @@ -8,9 +8,10 @@
  
  There is a small update to clarify it.
  
 -Signed-off-by: Maxim Belsky 
  Helped-by: Johannes Schindelin 
  Helped-by: Junio C Hamano 
 +Helped-by: SZEDER Gábor 
 +Signed-off-by: Maxim Belsky 
  
   diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
   --- a/contrib/completion/git-completion.zsh
 @@ -22,8 +23,8 @@
  -# The recommended way to install this script is to copy to 
'~/.zsh/_git', and
  -# then add the following to your ~/.zshrc file:
  +# The recommended way to install this script is to make a copy of it in
 -+# ~/.zsh/ directory as ~/.zsh/git-completion.zsh and then add the 
following
 -+# to your ~/.zshrc file:
 ++# '~/.zsh/' directory as '~/.zsh/_git' and then add the following to your
 ++# ~/.zshrc file:
   #
   #  fpath=(~/.zsh $fpath)
   

-- 
gitgitgadget


[PATCH v2 0/1] doc: Add a note about ~/.zsh/_git file

2019-10-11 Thread Max Belsky via GitGitGadget
Hey,

Today I've spent a few hours to understand why git-completion doesn't work
in my zsh shell. It was because I thought ~/.zsh/_git should be a dictionary
with git-completion.zsh file. 

I think this change may save some hours for someone else.

Maxim Belsky (1):
  doc: Change zsh git completion file name

 contrib/completion/git-completion.zsh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


base-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a
Published-As: 
https://github.com/gitgitgadget/git/releases/tag/pr-367%2Fmbelsky%2Fpatch-1-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-367/mbelsky/patch-1-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/367

Range-diff vs v1:

 1:  ae00e1e393 ! 1:  3f994f3b9a doc: Change zsh git completion file name
 @@ -2,7 +2,15 @@
  
  doc: Change zsh git completion file name
  
 +The original comment does not describe type of ~/.zsh/_git explicitly
 +and zsh does not warn or fail if a user create it as a dictionary.
 +So unexperienced users could be misled by the original comment.
 +
 +There is a small update to clarify it.
 +
  Signed-off-by: Maxim Belsky 
 +Helped-by: Johannes Schindelin 
 +Helped-by: Junio C Hamano 
  
   diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
   --- a/contrib/completion/git-completion.zsh
 @@ -13,9 +21,9 @@
   #
  -# The recommended way to install this script is to copy to 
'~/.zsh/_git', and
  -# then add the following to your ~/.zshrc file:
 -+# The recommended way to install this script is to copy to
 -+# '~/.zsh/.git-completion.zsh', and then add the following to your 
~/.zshrc
 -+# file:
 ++# The recommended way to install this script is to make a copy of it in
 ++# ~/.zsh/ directory as ~/.zsh/git-completion.zsh and then add the 
following
 ++# to your ~/.zshrc file:
   #
   #  fpath=(~/.zsh $fpath)
   

-- 
gitgitgadget


[PATCH 0/1] doc: Add a note about ~/.zsh/_git file

2019-10-10 Thread Max Belsky via GitGitGadget
Hey,

Today I've spent a few hours to understand why git-completion doesn't work
in my zsh shell. It was because I thought ~/.zsh/_git should be a dictionary
with git-completion.zsh file. 

I think this change may save some hours for someone else.

Maxim Belsky (1):
  doc: Change zsh git completion file name

 contrib/completion/git-completion.zsh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


base-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a
Published-As: 
https://github.com/gitgitgadget/git/releases/tag/pr-367%2Fmbelsky%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-367/mbelsky/patch-1-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/367
-- 
gitgitgadget


[PATCH v4 07/11] gc docs: note how --aggressive impacts --window & --depth

2019-04-07 Thread Ævar Arnfjörð Bjarmason
Since 07e7dbf0db (gc: default aggressive depth to 50, 2016-08-11) we
somewhat confusingly use the same depth under --aggressive as we do by
default.

As noted in that commit that makes sense, it was wrong to make more
depth the default for "aggressive", and thus save disk space at the
expense of runtime performance, which is usually the opposite of
someone who'd like "aggressive gc" wants.

But that's left us with a mostly-redundant configuration variable, so
let's clearly note in its documentation that it doesn't change the
default.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config/gc.txt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 56918a5008..f732fe5bfd 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -1,7 +1,8 @@
 gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
-   to 50.
+   to 50, which is the default for the `--depth` option when
+   `--aggressive` isn't in use.
 +
 See the documentation for the `--depth` option in
 linkgit:git-repack[1] for more details.
@@ -9,7 +10,8 @@ linkgit:git-repack[1] for more details.
 gc.aggressiveWindow::
The window size parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
-   to 250.
+   to 250, which is a much more aggressive window size than
+   the default `--window` of 10.
 +
 See the documentation for the `--window` option in
 linkgit:git-repack[1] for more details.
-- 
2.21.0.392.gf8f6787159e



[PATCH v4 09/11] gc docs: note "gc --aggressive" in "fast-import"

2019-04-07 Thread Ævar Arnfjörð Bjarmason
Amend the "PACKFILE OPTIMIZATION" section in "fast-import" to explain
that simply running "git gc --aggressive" after a "fast-import" should
properly optimize the repository. This is simpler and more effective
than the existing "repack" advice (which I'm keeping as it helps
explain things) because it e.g. also packs the newly imported refs.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-fast-import.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index 43ab3b1637..2248755cb7 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -1396,6 +1396,13 @@ deltas are suboptimal (see above) then also adding the 
`-f` option
 to force recomputation of all deltas can significantly reduce the
 final packfile size (30-50% smaller can be quite typical).
 
+Instead of running `git repack` you can also run `git gc
+--aggressive`, which will also optimize other things after an import
+(e.g. pack loose refs). As noted in the "AGGRESSIVE" section in
+linkgit:git-gc[1] the `--aggressive` option will find new deltas with
+the `-f` option to linkgit:git-repack[1]. For the reasons elaborated
+on above using `--aggressive` after a fast-import is one of the few
+cases where it's known to be worthwhile.
 
 MEMORY UTILIZATION
 --
-- 
2.21.0.392.gf8f6787159e



[PATCH v3 07/11] gc docs: note how --aggressive impacts --window & --depth

2019-03-22 Thread Ævar Arnfjörð Bjarmason
Since 07e7dbf0db (gc: default aggressive depth to 50, 2016-08-11) we
somewhat confusingly use the same depth under --aggressive as we do by
default.

As noted in that commit that makes sense, it was wrong to make more
depth the default for "aggressive", and thus save disk space at the
expense of runtime performance, which is usually the opposite of
someone who'd like "aggressive gc" wants.

But that's left us with a mostly-redundant configuration variable, so
let's clearly note in its documentation that it doesn't change the
default.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config/gc.txt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 56918a5008..f732fe5bfd 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -1,7 +1,8 @@
 gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
-   to 50.
+   to 50, which is the default for the `--depth` option when
+   `--aggressive` isn't in use.
 +
 See the documentation for the `--depth` option in
 linkgit:git-repack[1] for more details.
@@ -9,7 +10,8 @@ linkgit:git-repack[1] for more details.
 gc.aggressiveWindow::
The window size parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
-   to 250.
+   to 250, which is a much more aggressive window size than
+   the default `--window` of 10.
 +
 See the documentation for the `--window` option in
 linkgit:git-repack[1] for more details.
-- 
2.21.0.360.g471c308f928



[PATCH v3 09/11] gc docs: note "gc --aggressive" in "fast-import"

2019-03-22 Thread Ævar Arnfjörð Bjarmason
Amend the "PACKFILE OPTIMIZATION" section in "fast-import" to explain
that simply running "git gc --aggressive" after a "fast-import" should
properly optimize the repository. This is simpler and more effective
than the existing "repack" advice (which I'm keeping as it helps
explain things) because it e.g. also packs the newly imported refs.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-fast-import.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index 43ab3b1637..2248755cb7 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -1396,6 +1396,13 @@ deltas are suboptimal (see above) then also adding the 
`-f` option
 to force recomputation of all deltas can significantly reduce the
 final packfile size (30-50% smaller can be quite typical).
 
+Instead of running `git repack` you can also run `git gc
+--aggressive`, which will also optimize other things after an import
+(e.g. pack loose refs). As noted in the "AGGRESSIVE" section in
+linkgit:git-gc[1] the `--aggressive` option will find new deltas with
+the `-f` option to linkgit:git-repack[1]. For the reasons elaborated
+on above using `--aggressive` after a fast-import is one of the few
+cases where it's known to be worthwhile.
 
 MEMORY UTILIZATION
 --
-- 
2.21.0.360.g471c308f928



[PATCH v2 08/10] gc docs: note "gc --aggressive" in "fast-import"

2019-03-21 Thread Ævar Arnfjörð Bjarmason
Amend the "PACKFILE OPTIMIZATION" section in "fast-import" to explain
that simply running "git gc --aggressive" after a "fast-import" should
properly optimize the repository. This is simpler and more effective
than the existing "repack" advice (which I'm keeping as it helps
explain things) because it e.g. also packs the newly imported refs.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-fast-import.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index 43ab3b1637..2248755cb7 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -1396,6 +1396,13 @@ deltas are suboptimal (see above) then also adding the 
`-f` option
 to force recomputation of all deltas can significantly reduce the
 final packfile size (30-50% smaller can be quite typical).
 
+Instead of running `git repack` you can also run `git gc
+--aggressive`, which will also optimize other things after an import
+(e.g. pack loose refs). As noted in the "AGGRESSIVE" section in
+linkgit:git-gc[1] the `--aggressive` option will find new deltas with
+the `-f` option to linkgit:git-repack[1]. For the reasons elaborated
+on above using `--aggressive` after a fast-import is one of the few
+cases where it's known to be worthwhile.
 
 MEMORY UTILIZATION
 --
-- 
2.21.0.360.g471c308f928



[PATCH v2 06/10] gc docs: note how --aggressive impacts --window & --depth

2019-03-21 Thread Ævar Arnfjörð Bjarmason
Since 07e7dbf0db (gc: default aggressive depth to 50, 2016-08-11) we
somewhat confusingly use the same depth under --aggressive as we do by
default.

As noted in that commit that makes sense, it was wrong to make more
depth the default for "aggressive", and thus save disk space at the
expense of runtime performance, which is usually the opposite of
someone who'd like "aggressive gc" wants.

But that's left us with a mostly-redundant configuration variable, so
let's clearly note in its documentation that it doesn't change the
default.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config/gc.txt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 3e7fc052d9..0daa4683f6 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -1,7 +1,8 @@
 gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
-   to 50.
+   to 50, which is the default for the `--depth` option when
+   `--aggressive` isn't in use.
 +
 See the documentation for the `--depth` option in
 linkgit:git-repack[1] for more details.
@@ -9,7 +10,8 @@ linkgit:git-repack[1] for more details.
 gc.aggressiveWindow::
The window size parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
-   to 250.
+   to 250, which is a much more aggressive window size than
+   the default `--window` of 10.
 +
 See the documentation for the `--window` option in
 linkgit:git-repack[1] for more details.
-- 
2.21.0.360.g471c308f928



Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-21 Thread Elijah Newren
On Wed, Mar 20, 2019 at 7:58 AM Duy Nguyen  wrote:
>
> On Wed, Mar 20, 2019 at 8:53 PM Elijah Newren  wrote:
> > So, I think we do need something (eventually at least).  Would you
> > prefer we dropped this patch from Duy and instead made 'checkout -m'
> > abort when the index is dirty?
>
> I have no problem with this. Still scratching my head wondering if
> t7201-co.sh has a slightly incorrect setup, or aborting is actually
> wrong. You're probably a better person to understand that test case
> ;-)

It doesn't surprise me at all that some testcases would fail with this
change; it's a change of behavior from the previous implementation.
However, taking a look at that testcase, it looks like it's not a
simple change to make it do something similar because there's at least
one other bug that we need to fix first.  I'll dig in...though I
really want to get my
directory-rename-detection-defaults-to-reporting-conflict series
updated and sent out first.  Since Junio seems to be okay with just
taking your doc update for now, hopefully that's not a problem.  :-)


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-20 Thread Junio C Hamano
Elijah Newren  writes:

> On Tue, Mar 19, 2019 at 7:50 PM Junio C Hamano  wrote:
>>
>> Duy Nguyen  writes:
>>
>> > Kinda. But "--force --merge" makes no sense. --force discards all
>> > local changes by definition, which means you can't have conflicts and
>> > will not need --merge. I think this is the reason why we die() out
>> > when both are specified. So we need something like
>> > --discard-staged-changes-only...
>>
>> At that point, I would have to say that we do not need anything.
>> The use case is already covered with "git reset && git checkout -m",
>> isn't it?
>
> I guess the problem is just that 'git checkout -m' has not refused to
> run with either a dirty index or a dirty working tree, and if both are
> dirty (making us require more of a four-way merge), then our three-way
> merge has to ...

I didn't actually mean "nothing to do here" relative to the current
code; instead, I meant "nothing more than just stop when the index
has updates" (which is hard to read from the above quoted part, as
"Kinda." is a response in a discussion started with my "checkout -m
should probably refuse to do anything when the index is dirty").

> So, I think we do need something (eventually at least).  Would you
> prefer we dropped this patch from Duy and instead made 'checkout -m'
> abort when the index is dirty?

Let's go with the doc update first, as the patch has already
written.  I think in the longer term, just aborting when the index
is dirty would be a vast improvement over the status quo + a doc
update and is a good place to stop.

Thanks.


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-20 Thread Duy Nguyen
On Wed, Mar 20, 2019 at 8:53 PM Elijah Newren  wrote:
> So, I think we do need something (eventually at least).  Would you
> prefer we dropped this patch from Duy and instead made 'checkout -m'
> abort when the index is dirty?

I have no problem with this. Still scratching my head wondering if
t7201-co.sh has a slightly incorrect setup, or aborting is actually
wrong. You're probably a better person to understand that test case
;-)
-- 
Duy


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-20 Thread Elijah Newren
On Tue, Mar 19, 2019 at 7:50 PM Junio C Hamano  wrote:
>
> Duy Nguyen  writes:
>
> > Kinda. But "--force --merge" makes no sense. --force discards all
> > local changes by definition, which means you can't have conflicts and
> > will not need --merge. I think this is the reason why we die() out
> > when both are specified. So we need something like
> > --discard-staged-changes-only...
>
> At that point, I would have to say that we do not need anything.
> The use case is already covered with "git reset && git checkout -m",
> isn't it?

I guess the problem is just that 'git checkout -m' has not refused to
run with either a dirty index or a dirty working tree, and if both are
dirty (making us require more of a four-way merge), then our three-way
merge has to have some kind of casualty in the implementation for at
least some case.  The current casualty as highlighted by Philip is
that newly staged files before the 'checkout -m' become untracked and
any carefully staged pieces before that command are lost amongst the
unstaged changes again even if there weren't any conflicts.

One solution is to just accept and document or warn about this
shortcoming for now as Duy did in his patch.  Another is to do as you
mentioned earlier in this thread when you stated 'I think "checkout -m
" with a dirty index should refuse to run'.  Duy linked
to a third option that I outlined in his commit message, though it'd
require a bit more capability from merge-recursive than we have today.

So, I think we do need something (eventually at least).  Would you
prefer we dropped this patch from Duy and instead made 'checkout -m'
abort when the index is dirty?


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-19 Thread Junio C Hamano
Duy Nguyen  writes:

> Kinda. But "--force --merge" makes no sense. --force discards all
> local changes by definition, which means you can't have conflicts and
> will not need --merge. I think this is the reason why we die() out
> when both are specified. So we need something like
> --discard-staged-changes-only...

At that point, I would have to say that we do not need anything.
The use case is already covered with "git reset && git checkout -m",
isn't it?

Thanks.



Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-19 Thread Duy Nguyen
On Wed, Mar 20, 2019 at 8:19 AM Junio C Hamano  wrote:
>
> Duy Nguyen  writes:
>
> >> I think "checkout -m " with a dirty index should refuse
> >> to run; there is nothing to "continue" after such a failure, so I am
> >> not sure what you mean by "an option to continue" (iow, I do not see
> >> a need for such an option, and if that option makes the whole notion
> >> strange, we can just decide not to have it, can't we?).
> >
> > We have --force to continue even when we have local changes, which
> > will be overwritten. I was thinking a similar option which gives us
> > permission to destroy staged changes.
>
> Ah, then that is not "checkout --continue", but "checkout --force
> -m"?  That sounds sensible, and should behave as if "checkout -f
> HEAD && checkout -m " was done, with respect to local
> changes, I would think.

Kinda. But "--force --merge" makes no sense. --force discards all
local changes by definition, which means you can't have conflicts and
will not need --merge. I think this is the reason why we die() out
when both are specified. So we need something like
--discard-staged-changes-only...
-- 
Duy


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-19 Thread Junio C Hamano
Duy Nguyen  writes:

>> I think "checkout -m " with a dirty index should refuse
>> to run; there is nothing to "continue" after such a failure, so I am
>> not sure what you mean by "an option to continue" (iow, I do not see
>> a need for such an option, and if that option makes the whole notion
>> strange, we can just decide not to have it, can't we?).
>
> We have --force to continue even when we have local changes, which
> will be overwritten. I was thinking a similar option which gives us
> permission to destroy staged changes.

Ah, then that is not "checkout --continue", but "checkout --force
-m"?  That sounds sensible, and should behave as if "checkout -f
HEAD && checkout -m " was done, with respect to local
changes, I would think.


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-19 Thread Duy Nguyen
On Wed, Mar 20, 2019 at 7:24 AM Junio C Hamano  wrote:
>
> Nguyễn Thái Ngọc Duy   writes:
>
> > If you have staged changes in path A and perform 'checkout
> > --merge' (which could result in conflicts in a totally unrelated path
> > B), changes in A will be gone. Which is unexpected. We are supposed
> > to keep all changes, or kick and scream otherwise.
> >
> > This is the result of how --merge is implemented, from the very first
> > day in 1be0659efc (checkout: merge local modifications while switching
> > branches., 2006-01-12):
> >
> > 1. a merge is done, unmerged entries are collected
> > 2. a hard switch to a new branch is done, then unmerged entries added
> >back
> >
> > There is no trivial fix for this. Going with 3-way merge one file at a
> > time loses rename detection. Going with 3-way merge by trees requires
> > teaching the algorithm to pick up staged changes. And even if we detect
> > staged changes with --merge and abort for safety, an option to continue
> > --merge is very weird. Such an option would keep worktree changes, but
> > drop staged changes.
>
> I think "checkout -m " with a dirty index should refuse
> to run; there is nothing to "continue" after such a failure, so I am
> not sure what you mean by "an option to continue" (iow, I do not see
> a need for such an option, and if that option makes the whole notion
> strange, we can just decide not to have it, can't we?).

We have --force to continue even when we have local changes, which
will be overwritten. I was thinking a similar option which gives us
permission to destroy staged changes.

Refusing to run fails the test suite though (I tried that even before
this patch), in t7201.10, "switch to another branch while carrying a
deletion", because of this line

git rm two

-- 
Duy


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-19 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> If you have staged changes in path A and perform 'checkout
> --merge' (which could result in conflicts in a totally unrelated path
> B), changes in A will be gone. Which is unexpected. We are supposed
> to keep all changes, or kick and scream otherwise.
>
> This is the result of how --merge is implemented, from the very first
> day in 1be0659efc (checkout: merge local modifications while switching
> branches., 2006-01-12):
>
> 1. a merge is done, unmerged entries are collected
> 2. a hard switch to a new branch is done, then unmerged entries added
>back
>
> There is no trivial fix for this. Going with 3-way merge one file at a
> time loses rename detection. Going with 3-way merge by trees requires
> teaching the algorithm to pick up staged changes. And even if we detect
> staged changes with --merge and abort for safety, an option to continue
> --merge is very weird. Such an option would keep worktree changes, but
> drop staged changes.

I think "checkout -m " with a dirty index should refuse
to run; there is nothing to "continue" after such a failure, so I am
not sure what you mean by "an option to continue" (iow, I do not see
a need for such an option, and if that option makes the whole notion
strange, we can just decide not to have it, can't we?).


Re: [PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-19 Thread Phillip Wood

Hi Duy

On 19/03/2019 09:39, Nguyễn Thái Ngọc Duy wrote:

If you have staged changes in path A and perform 'checkout
--merge' (which could result in conflicts in a totally unrelated path
B), changes in A will be gone. Which is unexpected. We are supposed
to keep all changes, or kick and scream otherwise.

This is the result of how --merge is implemented, from the very first
day in 1be0659efc (checkout: merge local modifications while switching
branches., 2006-01-12):

1. a merge is done, unmerged entries are collected
2. a hard switch to a new branch is done, then unmerged entries added
back

There is no trivial fix for this. Going with 3-way merge one file at a
time loses rename detection. Going with 3-way merge by trees requires
teaching the algorithm to pick up staged changes. And even if we detect
staged changes with --merge and abort for safety, an option to continue
--merge is very weird. Such an option would keep worktree changes, but
drop staged changes.

Because the problem has been with us since the introduction of --merge
and everybody has been pretty happy (except Phillip, who found this
problem), I'll just take a note here to acknowledge it and wait for
merge wizards to come in and work their magic. There may be a way
forward [1].

[1] CABPp-BFoL_U=bzon4semaqsku2tkwnognqjt5muaoejtkgu...@mail.gmail.com

Reported-by: Phillip Wood 


I try to use phillip.w...@dunelm.org.uk for git stuff as it shouldn't 
change in the future.



Signed-off-by: Nguyễn Thái Ngọc Duy 
---
  This is my "fix" for Phillip's second problem. I chose to reply here
  because this is where an actual fix was discussed. The test script to
  demonstate it is here

  https://public-inbox.org/git/7d3742d6-73e4-2750-6ecb-9edf761d9...@gmail.com/

  Documentation/git-checkout.txt | 2 ++
  builtin/checkout.c | 9 +
  2 files changed, 11 insertions(+)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index f179b43732..877e5f503a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -242,6 +242,8 @@ should result in deletion of the path).
  +
  When checking out paths from the index, this option lets you recreate
  the conflicted merge in the specified paths.
++
+When switching branches with `--merge`, staged changes may be lost.
  
  --conflict=

[PATCH] checkout.txt: note about losing staged changes with --merge

2019-03-19 Thread Nguyễn Thái Ngọc Duy
If you have staged changes in path A and perform 'checkout
--merge' (which could result in conflicts in a totally unrelated path
B), changes in A will be gone. Which is unexpected. We are supposed
to keep all changes, or kick and scream otherwise.

This is the result of how --merge is implemented, from the very first
day in 1be0659efc (checkout: merge local modifications while switching
branches., 2006-01-12):

1. a merge is done, unmerged entries are collected
2. a hard switch to a new branch is done, then unmerged entries added
   back

There is no trivial fix for this. Going with 3-way merge one file at a
time loses rename detection. Going with 3-way merge by trees requires
teaching the algorithm to pick up staged changes. And even if we detect
staged changes with --merge and abort for safety, an option to continue
--merge is very weird. Such an option would keep worktree changes, but
drop staged changes.

Because the problem has been with us since the introduction of --merge
and everybody has been pretty happy (except Phillip, who found this
problem), I'll just take a note here to acknowledge it and wait for
merge wizards to come in and work their magic. There may be a way
forward [1].

[1] CABPp-BFoL_U=bzon4semaqsku2tkwnognqjt5muaoejtkgu...@mail.gmail.com

Reported-by: Phillip Wood 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 This is my "fix" for Phillip's second problem. I chose to reply here
 because this is where an actual fix was discussed. The test script to
 demonstate it is here

 https://public-inbox.org/git/7d3742d6-73e4-2750-6ecb-9edf761d9...@gmail.com/

 Documentation/git-checkout.txt | 2 ++
 builtin/checkout.c | 9 +
 2 files changed, 11 insertions(+)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index f179b43732..877e5f503a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -242,6 +242,8 @@ should result in deletion of the path).
 +
 When checking out paths from the index, this option lets you recreate
 the conflicted merge in the specified paths.
++
+When switching branches with `--merge`, staged changes may be lost.
 
 --conflict=

Re: [PATCH 1/3] doc: note config file restrictions for completion.commands

2019-03-17 Thread Duy Nguyen
Poking this thread before it goes completely dead...

On Sat, Mar 2, 2019 at 12:34 AM Todd Zullinger  wrote:
>
> The completion.commands config var must be set in either the system-wide
> or global config file.  The completion script does not read a local
> repository config.

After the last discussion, I think the consensus was it's ok to allow
per-repo config so we can just drop this and update the code to read
from any config file, right?

>
> Signed-off-by: Todd Zullinger 
> ---
>  Documentation/config/completion.txt | 3 ++-
>  Documentation/git.txt   | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/completion.txt 
> b/Documentation/config/completion.txt
> index 4d99bf33c9..4859d18e86 100644
> --- a/Documentation/config/completion.txt
> +++ b/Documentation/config/completion.txt
> @@ -4,4 +4,5 @@ completion.commands::
> porcelain commands and a few select others are completed. You
> can add more commands, separated by space, in this
> variable. Prefixing the command with '-' will remove it from
> -   the existing list.
> +   the existing list.  The variable must be set in either the
> +   system-wide or global config.
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 00156d64aa..638f4d6cc9 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -172,7 +172,8 @@ foo.bar= ...`) sets `foo.bar` to the empty string which 
> `git config
> others (all other commands in `$PATH` that have git- prefix),
> list- (see categories in command-list.txt),
> nohelpers (exclude helper commands), alias and config
> -   (retrieve command list from config variable completion.commands)
> +   (retrieve command list from config variable completion.commands,
> +   set in the global or system-wide config file)
>
>  GIT COMMANDS
>  



-- 
Duy


Re: [PATCH v2] Documentation/config: note trailing newline with --type=color

2019-03-06 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Mar 06, 2019 at 03:52:38PM -0800, Taylor Blau wrote:
>
>> In 63e2a0f8e9 (builtin/config: introduce `color` type specifier,
>> 2018-04-09), `--type=color` was introduced and preferred to the
>> old-style `--get-color`.
>> 
>> The two behave the same in almost every way, save for the fact that
>> `--type=color` prints a trailing newline where `--get-color` does not.
>> Instead of introducing ambiguity between `--type=color` and the other
>> `--type` variants, document the difference between `--type=color` and
>> `--get-color` instead.
>
> I just responded to the one Junio posted elsewhere in the thread, but
> for the record this one is fine to me, too (and it gets the literal
> quoting right ;) ).

Yeah, I haven't reached that message when I was doing mine.  Other
than "co-authored-by", which probably is not quite correct (i.e. I
would have added "[tb: wrapped it up with a commit log message]" and
kept you as the author, if I were doing it), I do not have a problem
with Taylor's version, either.



Re: [PATCH v2] Documentation/config: note trailing newline with --type=color

2019-03-06 Thread Jeff King
On Wed, Mar 06, 2019 at 03:52:38PM -0800, Taylor Blau wrote:

> In 63e2a0f8e9 (builtin/config: introduce `color` type specifier,
> 2018-04-09), `--type=color` was introduced and preferred to the
> old-style `--get-color`.
> 
> The two behave the same in almost every way, save for the fact that
> `--type=color` prints a trailing newline where `--get-color` does not.
> Instead of introducing ambiguity between `--type=color` and the other
> `--type` variants, document the difference between `--type=color` and
> `--get-color` instead.

I just responded to the one Junio posted elsewhere in the thread, but
for the record this one is fine to me, too (and it gets the literal
quoting right ;) ).

-Peff


[PATCH v2] Documentation/config: note trailing newline with --type=color

2019-03-06 Thread Taylor Blau
In 63e2a0f8e9 (builtin/config: introduce `color` type specifier,
2018-04-09), `--type=color` was introduced and preferred to the
old-style `--get-color`.

The two behave the same in almost every way, save for the fact that
`--type=color` prints a trailing newline where `--get-color` does not.
Instead of introducing ambiguity between `--type=color` and the other
`--type` variants, document the difference between `--type=color` and
`--get-color` instead.

Co-authored-by: Jeff King 
Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 1bfe9f56a7..d0b9c50d20 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -240,7 +240,9 @@ Valid ``'s include:
output.  The optional `default` parameter is used instead, if
there is no color configured for `name`.
 +
-`--type=color [--default=]` is preferred over `--get-color`.
+`--type=color [--default=]` is preferred over `--get-color`
+(but note that `--get-color` will omit the trailing newline printed by
+`--type=color`).
 
 -e::
 --edit::
-- 
2.20.1


[PATCH 1/3] doc: note config file restrictions for completion.commands

2019-03-01 Thread Todd Zullinger
The completion.commands config var must be set in either the system-wide
or global config file.  The completion script does not read a local
repository config.

Signed-off-by: Todd Zullinger 
---
 Documentation/config/completion.txt | 3 ++-
 Documentation/git.txt   | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/completion.txt 
b/Documentation/config/completion.txt
index 4d99bf33c9..4859d18e86 100644
--- a/Documentation/config/completion.txt
+++ b/Documentation/config/completion.txt
@@ -4,4 +4,5 @@ completion.commands::
porcelain commands and a few select others are completed. You
can add more commands, separated by space, in this
variable. Prefixing the command with '-' will remove it from
-   the existing list.
+   the existing list.  The variable must be set in either the
+   system-wide or global config.
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 00156d64aa..638f4d6cc9 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -172,7 +172,8 @@ foo.bar= ...`) sets `foo.bar` to the empty string which 
`git config
others (all other commands in `$PATH` that have git- prefix),
list- (see categories in command-list.txt),
nohelpers (exclude helper commands), alias and config
-   (retrieve command list from config variable completion.commands)
+   (retrieve command list from config variable completion.commands,
+   set in the global or system-wide config file)
 
 GIT COMMANDS
 


A note from the maintainer

2019-02-26 Thread Junio C Hamano
tster/git/

A few web interfaces are found at:

  http://git.kernel.org/cgit/git/git.git
  https://kernel.googlesource.com/pub/scm/git/git
  http://repo.or.cz/w/alt-git.git

Preformatted documentation from the tip of the "master" branch can be
found in:

  git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
  git://repo.or.cz/git-{htmldocs,manpages}.git/
  https://github.com/gitster/git-{htmldocs,manpages}.git/

The manual pages formatted in HTML for the tip of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.15 done on Oct 30th, 2017.  You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  It is merged into "master"
primarily to propagate the description in the release notes forward.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "ne

Re: [PATCH] git-rebase.txt: update note about directory rename detection and am

2018-12-08 Thread Junio C Hamano
Elijah Newren  writes:

> On Fri, Dec 7, 2018 at 9:51 AM Johannes Sixt  wrote:
>>
>> From: Elijah Newren 
>>
>> In commit 6aba117d5cf7 ("am: avoid directory rename detection when
>> calling recursive merge machinery", 2018-08-29), the git-rebase manpage
>> probably should have also been updated to note the stronger
>> incompatibility between git-am and directory rename detection.  Update
>> it now.
>>
>> Signed-off-by: Elijah Newren 
>> Signed-off-by: Johannes Sixt 
>> ---
>>   Documentation/git-rebase.txt | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>> index 41631df6e4..7bea21e8e3 100644
>> --- a/Documentation/git-rebase.txt
>> +++ b/Documentation/git-rebase.txt
>> @@ -569,8 +569,9 @@ it to keep commits that started empty.
>>   Directory rename detection
>>   ~~
>>
>> -The merge and interactive backends work fine with
>> -directory rename detection.  The am backend sometimes does not.
>> +Directory rename heuristics are enabled in the merge and interactive
>> +backends.  Due to the lack of accurate tree information, directory
>> +rename detection is disabled in the am backend.
>>
>>   include::merge-strategies.txt[]
>
> I was intending to send this out the past couple days, was just kinda
> busy.  Thanks for handling it for me.

Thanks, both.  I can live with format=flowed, but would appreciate
if we can avoid it next time.


Re: [PATCH] git-rebase.txt: update note about directory rename detection and am

2018-12-07 Thread Elijah Newren
On Fri, Dec 7, 2018 at 9:51 AM Johannes Sixt  wrote:
>
> From: Elijah Newren 
>
> In commit 6aba117d5cf7 ("am: avoid directory rename detection when
> calling recursive merge machinery", 2018-08-29), the git-rebase manpage
> probably should have also been updated to note the stronger
> incompatibility between git-am and directory rename detection.  Update
> it now.
>
> Signed-off-by: Elijah Newren 
> Signed-off-by: Johannes Sixt 
> ---
>   Documentation/git-rebase.txt | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 41631df6e4..7bea21e8e3 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -569,8 +569,9 @@ it to keep commits that started empty.
>   Directory rename detection
>   ~~
>
> -The merge and interactive backends work fine with
> -directory rename detection.  The am backend sometimes does not.
> +Directory rename heuristics are enabled in the merge and interactive
> +backends.  Due to the lack of accurate tree information, directory
> +rename detection is disabled in the am backend.
>
>   include::merge-strategies.txt[]

I was intending to send this out the past couple days, was just kinda
busy.  Thanks for handling it for me.


[PATCH] git-rebase.txt: update note about directory rename detection and am

2018-12-07 Thread Johannes Sixt

From: Elijah Newren 

In commit 6aba117d5cf7 ("am: avoid directory rename detection when
calling recursive merge machinery", 2018-08-29), the git-rebase manpage
probably should have also been updated to note the stronger
incompatibility between git-am and directory rename detection.  Update
it now.

Signed-off-by: Elijah Newren 
Signed-off-by: Johannes Sixt 
---
 Documentation/git-rebase.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 41631df6e4..7bea21e8e3 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -569,8 +569,9 @@ it to keep commits that started empty.
 Directory rename detection
 ~~

-The merge and interactive backends work fine with
-directory rename detection.  The am backend sometimes does not.
+Directory rename heuristics are enabled in the merge and interactive
+backends.  Due to the lack of accurate tree information, directory
+rename detection is disabled in the am backend.

 include::merge-strategies.txt[]

--
2.19.1.1133.g2dd3d172d2


Re: [PATCH] config.txt: correct the note about uploadpack.packObjectsHook

2018-09-28 Thread Jeff King
On Sat, Sep 29, 2018 at 08:50:56AM +0200, Nguyễn Thái Ngọc Duy wrote:

> Document for uploadpack.packObjectsHook is added in [1] and consists
> of two paragraphs, the second one is quite important about where this
> variable can stay.
> 
> When the paragraph about uploadpack.allowFilter is added in [2], it's
> added in between the two paragraphs. This makes the "this is non-repo
> level config" note incorrectly apply to allowFilter instead of
> packObjectsHook. Move allowFilter paragraph down to fix this.

Nice catch, and the patch looks obviously correct. Thanks.

-Peff


[PATCH] config.txt: correct the note about uploadpack.packObjectsHook

2018-09-28 Thread Nguyễn Thái Ngọc Duy
Document for uploadpack.packObjectsHook is added in [1] and consists
of two paragraphs, the second one is quite important about where this
variable can stay.

When the paragraph about uploadpack.allowFilter is added in [2], it's
added in between the two paragraphs. This makes the "this is non-repo
level config" note incorrectly apply to allowFilter instead of
packObjectsHook. Move allowFilter paragraph down to fix this.

[1] 20b20a22f8 (upload-pack: provide a hook for running pack-objects -
2016-05-18)

[2] 10ac85c785 (upload-pack: add object filtering for partial clone -
2017-12-08)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/config.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ad0f4510c3..907b1d0cb9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3666,15 +3666,15 @@ uploadpack.packObjectsHook::
was run. I.e., `upload-pack` will feed input intended for
`pack-objects` to the hook, and expects a completed packfile on
stdout.
-
-uploadpack.allowFilter::
-   If this option is set, `upload-pack` will support partial
-   clone and partial fetch object filtering.
 +
 Note that this configuration variable is ignored if it is seen in the
 repository-level config (this is a safety measure against fetching from
 untrusted repositories).
 
+uploadpack.allowFilter::
+   If this option is set, `upload-pack` will support partial
+   clone and partial fetch object filtering.
+
 uploadpack.allowRefInWant::
If this option is set, `upload-pack` will support the `ref-in-want`
feature of the protocol version 2 `fetch` command.  This feature
-- 
2.19.0.341.g3acb95d729



Re: [PATCH 2/2] git-config.txt: fix 'see: above' note

2018-09-19 Thread Martin Ågren
Hi Taylor,

On Wed, 19 Sep 2018 at 19:21, Taylor Blau  wrote:
> I could take or leave 2/2, since I usually write ", (see: above)", but
> I'm not sure if that's grammatically correct or not.

Well, I sure ain't no grammar expert too... This is not a patch I feel
strongly about, so I'll be happy to defer to others.

Thanks for reviewing,
Martin


Re: [PATCH 2/2] git-config.txt: fix 'see: above' note

2018-09-19 Thread Taylor Blau
Hi Martin,

On Wed, Sep 19, 2018 at 06:38:19PM +0200, Martin Ågren wrote:
> Rather than saying "(see: above)", drop the colon. Also drop the comma
> before this note.
>
> Signed-off-by: Martin Ågren 

Thanks for both of these. I should have at least taken care of 1/2
myself, but I am appreciative of you doing it, too :-).

I could take or leave 2/2, since I usually write ", (see: above)", but
I'm not sure if that's grammatically correct or not.

But, either approach is fine with me, so both of these have my:

  Reviewed-by: Taylor Blau 

Thanks,
Taylor


[PATCH 2/2] git-config.txt: fix 'see: above' note

2018-09-19 Thread Martin Ågren
Rather than saying "(see: above)", drop the colon. Also drop the comma
before this note.

Signed-off-by: Martin Ågren 
---
 Documentation/git-config.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 9d8cea72dd..5e87d82933 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -188,8 +188,8 @@ Valid ``'s include:
 --bool-or-int::
 --path::
 --expiry-date::
-  Historical options for selecting a type specifier. Prefer instead `--type`,
-  (see: above).
+  Historical options for selecting a type specifier. Prefer instead `--type`
+  (see above).
 
 --no-type::
   Un-sets the previously set type specifier (if one was previously set). This
-- 
2.19.0.216.g2d3b1c576c



[PATCH v2 2/2] rerere: add note about files with existing conflict markers

2018-08-28 Thread Thomas Gummerer
When a file contains lines that look like conflict markers, 'git
rerere' may fail not be able to record a conflict resolution.
Emphasize that in the man page, and mention a possible workaround for
the issue.

Suggested-by: Junio C Hamano 
Signed-off-by: Thomas Gummerer 
---

Compared to v1, this now mentions the workaround of setting the
'conflict-marker-size', as mentioned in


 Documentation/git-rerere.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index 031f31fa47..df310d2a58 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt
@@ -211,6 +211,12 @@ would conflict the same way as the test merge you resolved 
earlier.
 'git rerere' will be run by 'git rebase' to help you resolve this
 conflict.
 
+[NOTE] 'git rerere' relies on the conflict markers in the file to
+detect the conflict.  If the file already contains lines that look the
+same as lines with conflict markers, 'git rerere' may fail to record a
+conflict resolution.  To work around this, the `conflict-marker-size`
+setting in linkgit:gitattributes[5] can be used.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
-- 
2.18.0.1088.ge017bf2cd1



[PATCH 09/24] unpack-trees: add a note about path invalidation

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 unpack-trees.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index f9efee0836..c07a6cd646 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1552,6 +1552,17 @@ static int verify_uptodate_sparse(const struct 
cache_entry *ce,
return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);
 }
 
+/*
+ * TODO: We should actually invalidate o->result, not src_index [1].
+ * But since cache tree and untracked cache both are not copied to
+ * o->result until unpacking is complete, we invalidate them on
+ * src_index instead with the assumption that they will be copied to
+ * dst_index at the end.
+ *
+ * [1] src_index->cache_tree is also used in unpack_callback() so if
+ * we invalidate o->result, we need to update it to use
+ * o->result.cache_tree as well.
+ */
 static void invalidate_ce_path(const struct cache_entry *ce,
   struct unpack_trees_options *o)
 {
-- 
2.18.0.1004.g6639190530



[PATCH 1/2] doc hash-function-transition: note the lack of a changelog

2018-07-25 Thread Ævar Arnfjörð Bjarmason
The changelog embedded in the document pre-dates the addition of the
document to git.git (it used to be a Google Doc), so it only goes up
to 752414ae43 ("technical doc: add a design doc for hash function
transition", 2017-09-27).

Since then I made some small edits to it, which would have been worthy
of including in this changelog (but weren't). Instead of amending it
to include these, just note that future changes will be noted in the
log.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/technical/hash-function-transition.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/technical/hash-function-transition.txt 
b/Documentation/technical/hash-function-transition.txt
index 4ab6cd1012..5ee4754adb 100644
--- a/Documentation/technical/hash-function-transition.txt
+++ b/Documentation/technical/hash-function-transition.txt
@@ -814,6 +814,12 @@ Incorporated suggestions from jonathantanmy and sbeller:
 * avoid loose object overhead by packing more aggressively in
   "git gc --auto"
 
+Later history:
+
+ See the history of this file in git.git for the history of subsequent
+ edits. This document history is no longer being maintained as it
+ would now be superfluous to the commit log
+
 [1] 
http://public-inbox.org/git/ca+55afzjtejicjv0e43+9or3qujk2pifilqemytolpyjwe6...@mail.gmail.com/
 [2] 
http://public-inbox.org/git/ca+55afz+gkasdz24zmepques1xps9bp_s8o7q4wq7lv7x5-...@mail.gmail.com/
 [3] 
http://public-inbox.org/git/20170306084353.nrns455dvkdsf...@sigill.intra.peff.net/
-- 
2.17.0.290.gded63e768a



Re: [PATCH 03/20] blame doc: explicitly note how --abbrev=40 gives 39 chars

2018-06-12 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> In a later change I'm adding stress testing of the commit abbreviation
> as it relates to git-blame and others, and initially thought that the
> inability to extract full SHA-1s from the non-"--porcelain" output was
> a bug.

... meaning that it is not actually a bug, as the output format
other than porcelain is for human consumption?

> --- a/Documentation/git-blame.txt
> +++ b/Documentation/git-blame.txt
> @@ -88,6 +88,11 @@ include::blame-options.txt[]
>   Instead of using the default 7+1 hexadecimal digits as the
>   abbreviated object name, use +1 digits. Note that 1 column
>   is used for a caret to mark the boundary commit.

This is outside the scope of this patch, but is the above 7+1 still
current or do we need updating it for the (not so) recent change to
auto-scale the default abbreviation width?

> ++
> +Because of this UI design, the only way to get the full SHA-1 of the
> +boundary commit is to use the `--porcelain` format. With `--abbrev=40`
> +only 39 characters of the boundary SHA-1 will be emitted, since one
> +will be used for the caret to mark the boundary.

OK.


[PATCH 03/20] blame doc: explicitly note how --abbrev=40 gives 39 chars

2018-06-08 Thread Ævar Arnfjörð Bjarmason
In a later change I'm adding stress testing of the commit abbreviation
as it relates to git-blame and others, and initially thought that the
inability to extract full SHA-1s from the non-"--porcelain" output was
a bug.

In hindsight I could have read the existing paragraph more carefully,
but let's make this clearer by explicitly stating this limitation of
--abbrev as it relates to git-blame, it is not shared by any other
command that supports core.abbrev or --abbrev.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-blame.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 16323eb80e..7b562494ac 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -88,6 +88,11 @@ include::blame-options.txt[]
Instead of using the default 7+1 hexadecimal digits as the
abbreviated object name, use +1 digits. Note that 1 column
is used for a caret to mark the boundary commit.
++
+Because of this UI design, the only way to get the full SHA-1 of the
+boundary commit is to use the `--porcelain` format. With `--abbrev=40`
+only 39 characters of the boundary SHA-1 will be emitted, since one
+will be used for the caret to mark the boundary.
 
 
 THE PORCELAIN FORMAT
-- 
2.17.0.290.gded63e768a



[PATCH v4 02/23] unpack-trees: add a note about path invalidation

2018-06-06 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 unpack-trees.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index 3a85a02a77..5d06aa9c98 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1545,6 +1545,17 @@ static int verify_uptodate_sparse(const struct 
cache_entry *ce,
return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);
 }
 
+/*
+ * TODO: We should actually invalidate o->result, not src_index [1].
+ * But since cache tree and untracked cache both are not copied to
+ * o->result until unpacking is complete, we invalidate them on
+ * src_index instead with the assumption that they will be copied to
+ * dst_index at the end.
+ *
+ * [1] src_index->cache_tree is also used in unpack_callback() so if
+ * we invalidate o->result, we need to update it to use
+ * o->result.cache_tree as well.
+ */
 static void invalidate_ce_path(const struct cache_entry *ce,
   struct unpack_trees_options *o)
 {
-- 
2.18.0.rc0.333.g22e6ee6cdf



[PATCH v3 02/20] unpack-trees: add a note about path invalidation

2018-06-06 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 unpack-trees.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index 3a85a02a77..5d06aa9c98 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1545,6 +1545,17 @@ static int verify_uptodate_sparse(const struct 
cache_entry *ce,
return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);
 }
 
+/*
+ * TODO: We should actually invalidate o->result, not src_index [1].
+ * But since cache tree and untracked cache both are not copied to
+ * o->result until unpacking is complete, we invalidate them on
+ * src_index instead with the assumption that they will be copied to
+ * dst_index at the end.
+ *
+ * [1] src_index->cache_tree is also used in unpack_callback() so if
+ * we invalidate o->result, we need to update it to use
+ * o->result.cache_tree as well.
+ */
 static void invalidate_ce_path(const struct cache_entry *ce,
   struct unpack_trees_options *o)
 {
-- 
2.18.0.rc0.333.g22e6ee6cdf



[PATCH v2 2/5] unpack-trees: add a note about path invalidation

2018-06-05 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 unpack-trees.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index 3a85a02a77..5d06aa9c98 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1545,6 +1545,17 @@ static int verify_uptodate_sparse(const struct 
cache_entry *ce,
return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);
 }
 
+/*
+ * TODO: We should actually invalidate o->result, not src_index [1].
+ * But since cache tree and untracked cache both are not copied to
+ * o->result until unpacking is complete, we invalidate them on
+ * src_index instead with the assumption that they will be copied to
+ * dst_index at the end.
+ *
+ * [1] src_index->cache_tree is also used in unpack_callback() so if
+ * we invalidate o->result, we need to update it to use
+ * o->result.cache_tree as well.
+ */
 static void invalidate_ce_path(const struct cache_entry *ce,
   struct unpack_trees_options *o)
 {
-- 
2.18.0.rc0.333.g22e6ee6cdf



[PATCH 2/6] unpack-trees: add a note about path invalidation

2018-06-05 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 unpack-trees.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index 3a85a02a77..5d06aa9c98 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1545,6 +1545,17 @@ static int verify_uptodate_sparse(const struct 
cache_entry *ce,
return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);
 }
 
+/*
+ * TODO: We should actually invalidate o->result, not src_index [1].
+ * But since cache tree and untracked cache both are not copied to
+ * o->result until unpacking is complete, we invalidate them on
+ * src_index instead with the assumption that they will be copied to
+ * dst_index at the end.
+ *
+ * [1] src_index->cache_tree is also used in unpack_callback() so if
+ * we invalidate o->result, we need to update it to use
+ * o->result.cache_tree as well.
+ */
 static void invalidate_ce_path(const struct cache_entry *ce,
   struct unpack_trees_options *o)
 {
-- 
2.18.0.rc0.333.g22e6ee6cdf



Re: [PATCH] RelNotes: remove duplicate release note

2018-05-31 Thread Junio C Hamano
Elijah Newren  writes:

> In the 2.18 cycle, directory rename detection was merged, then reverted,
> then reworked in such a way to fix another prominent bug in addition to
> the original problem causing it to be reverted.  When the reworked series
> was merged, we ended up with two nearly duplicate release notes.  Remove
> the second copy, but preserve the information about the extra bug fix.

Thanks.


Re: [PATCH v2 2/2] note git-secur...@googlegroups.com in more places

2018-05-31 Thread Thomas Gummerer
On 05/30, brian m. carlson wrote:
> On Wed, May 30, 2018 at 09:52:55PM +0100, Thomas Gummerer wrote:
> > Add a mention of the security mailing list to the README, and to
> > Documentation/SubmittingPatches..  2caa7b8d27 ("git manpage: note
> > git-secur...@googlegroups.com", 2018-03-08) already added it to the
> > man page, but for developers either the README, or the documentation
> > on how to contribute (SubmittingPatches) may be the first place to
> > look.
> > 
> > Use the same wording as we already have on the git-scm.com website and
> > in the man page for the README, while the wording is adjusted in
> > SubmittingPatches to match the surrounding document better.
> > 
> > Signed-off-by: Thomas Gummerer 
> > ---
> >  Documentation/SubmittingPatches | 13 +
> >  README.md   |  3 +++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/Documentation/SubmittingPatches 
> > b/Documentation/SubmittingPatches
> > index 27553128f5..c8f9deb391 100644
> > --- a/Documentation/SubmittingPatches
> > +++ b/Documentation/SubmittingPatches
> > @@ -176,6 +176,12 @@ that is fine, but please mark it as such.
> >  [[send-patches]]
> >  === Sending your patches.
> >  
> > +:security-ml: footnoteref:[security-ml,The Git Security mailing list: 
> > git-secur...@googlegroups.com]
> > +
> > +Before sending any patches, please note that patches that may be
> > +security relevant should be submitted privately to the Git Security
> > +mailing list{security-ml}, instead of the public mailing list.
> > +
> >  Learn to use format-patch and send-email if possible.  These commands
> >  are optimized for the workflow of sending patches, avoiding many ways
> >  your existing e-mail client that is optimized for "multipart/*" mime
> > @@ -259,6 +265,13 @@ patch, format it as "multipart/signed", not a 
> > text/plain message
> >  that starts with `-BEGIN PGP SIGNED MESSAGE-`.  That is
> >  not a text/plain, it's something else.
> >  
> > +:security-ml-ref: footnoteref:[security-ml]
> 
> My only feedback here is that using the footnoteref syntax to refer to
> the previous footnote potentially makes this a little less readable for
> plain text users, although it also reduces duplication.  I'm not sure I
> feel strongly one way or the other on this.

Yeah, using the plain footnote syntax we end up with two footnotes
that are exactly the same, which felt a little awkward.  But I don't
feel strongly either, so if the consensus is to duplicate the footnote
for better readability in plain text I'm happy to change that.

To really improve the readability we'd probably have to duplicate the
attribute as well, which I wanted to avoid (altough it's not
completely possible with the footnoteref syntax either).

> Otherwise, this looked fine to me.
> -- 
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204


[PATCH] RelNotes: remove duplicate release note

2018-05-30 Thread Elijah Newren
In the 2.18 cycle, directory rename detection was merged, then reverted,
then reworked in such a way to fix another prominent bug in addition to
the original problem causing it to be reverted.  When the reworked series
was merged, we ended up with two nearly duplicate release notes.  Remove
the second copy, but preserve the information about the extra bug fix.

Signed-off-by: Elijah Newren 
---
 Documentation/RelNotes/2.18.0.txt | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/Documentation/RelNotes/2.18.0.txt 
b/Documentation/RelNotes/2.18.0.txt
index c9e2e19721..fd5aecf8e9 100644
--- a/Documentation/RelNotes/2.18.0.txt
+++ b/Documentation/RelNotes/2.18.0.txt
@@ -12,7 +12,9 @@ UI, Workflows & Features
want to move to z/d by taking the hint that the entire directory
'x' moved to 'z'.  A bug causing dirty files involved in a rename
to be overwritten during merge has also been fixed as part of this
-   work.
+   work.  Incidentally, this also avoids updating a file in the
+   working tree after a (non-trivial) merge whose result matches what
+   our side originally had.
 
  * "git filter-branch" learned to use a different exit code to allow
the callers to tell the case where there was no new commits to
@@ -256,16 +258,6 @@ Performance, Internal Implementation, Development Support 
etc.
repository object (which in turn tells the API which object store
the objects are to be located).
 
- * Rename detection logic in "diff" family that is used in "merge" has
-   learned to guess when all of x/a, x/b and x/c have moved to z/a,
-   z/b and z/c, it is likely that x/d added in the meantime would also
-   want to move to z/d by taking the hint that the entire directory
-   'x' moved to 'z'.  A bug causing dirty files involved in a rename
-   to be overwritten during merge has also been fixed as part of this
-   work.  Incidentally, this also avoids updating a file in the
-   working tree after a (non-trivial) merge whose result matches what
-   our side originally had.
-
  * "git pack-objects" needs to allocate tons of "struct object_entry"
while doing its work, and shrinking its size helps the performance
quite a bit.
-- 
2.18.0.rc0



Re: [PATCH v2 2/2] note git-secur...@googlegroups.com in more places

2018-05-30 Thread brian m. carlson
On Wed, May 30, 2018 at 09:52:55PM +0100, Thomas Gummerer wrote:
> Add a mention of the security mailing list to the README, and to
> Documentation/SubmittingPatches..  2caa7b8d27 ("git manpage: note
> git-secur...@googlegroups.com", 2018-03-08) already added it to the
> man page, but for developers either the README, or the documentation
> on how to contribute (SubmittingPatches) may be the first place to
> look.
> 
> Use the same wording as we already have on the git-scm.com website and
> in the man page for the README, while the wording is adjusted in
> SubmittingPatches to match the surrounding document better.
> 
> Signed-off-by: Thomas Gummerer 
> ---
>  Documentation/SubmittingPatches | 13 +
>  README.md   |  3 +++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 27553128f5..c8f9deb391 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -176,6 +176,12 @@ that is fine, but please mark it as such.
>  [[send-patches]]
>  === Sending your patches.
>  
> +:security-ml: footnoteref:[security-ml,The Git Security mailing list: 
> git-secur...@googlegroups.com]
> +
> +Before sending any patches, please note that patches that may be
> +security relevant should be submitted privately to the Git Security
> +mailing list{security-ml}, instead of the public mailing list.
> +
>  Learn to use format-patch and send-email if possible.  These commands
>  are optimized for the workflow of sending patches, avoiding many ways
>  your existing e-mail client that is optimized for "multipart/*" mime
> @@ -259,6 +265,13 @@ patch, format it as "multipart/signed", not a text/plain 
> message
>  that starts with `-BEGIN PGP SIGNED MESSAGE-`.  That is
>  not a text/plain, it's something else.
>  
> +:security-ml-ref: footnoteref:[security-ml]

My only feedback here is that using the footnoteref syntax to refer to
the previous footnote potentially makes this a little less readable for
plain text users, although it also reduces duplication.  I'm not sure I
feel strongly one way or the other on this.

Otherwise, this looked fine to me.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


[PATCH v2 2/2] note git-secur...@googlegroups.com in more places

2018-05-30 Thread Thomas Gummerer
Add a mention of the security mailing list to the README, and to
Documentation/SubmittingPatches..  2caa7b8d27 ("git manpage: note
git-secur...@googlegroups.com", 2018-03-08) already added it to the
man page, but for developers either the README, or the documentation
on how to contribute (SubmittingPatches) may be the first place to
look.

Use the same wording as we already have on the git-scm.com website and
in the man page for the README, while the wording is adjusted in
SubmittingPatches to match the surrounding document better.

Signed-off-by: Thomas Gummerer 
---
 Documentation/SubmittingPatches | 13 +
 README.md   |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 27553128f5..c8f9deb391 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -176,6 +176,12 @@ that is fine, but please mark it as such.
 [[send-patches]]
 === Sending your patches.
 
+:security-ml: footnoteref:[security-ml,The Git Security mailing list: 
git-secur...@googlegroups.com]
+
+Before sending any patches, please note that patches that may be
+security relevant should be submitted privately to the Git Security
+mailing list{security-ml}, instead of the public mailing list.
+
 Learn to use format-patch and send-email if possible.  These commands
 are optimized for the workflow of sending patches, avoiding many ways
 your existing e-mail client that is optimized for "multipart/*" mime
@@ -259,6 +265,13 @@ patch, format it as "multipart/signed", not a text/plain 
message
 that starts with `-BEGIN PGP SIGNED MESSAGE-`.  That is
 not a text/plain, it's something else.
 
+:security-ml-ref: footnoteref:[security-ml]
+
+As mentioned at the beginning of the section, patches that may be
+security relevant should not be submitted to the public mailing list
+mentioned below, but should instead be sent privately to the Git
+Security mailing list{security-ml-ref}.
+
 Send your patch with "To:" set to the mailing list, with "cc:" listing
 people who are involved in the area you are touching (the `git
 contacts` command in `contrib/contacts/` can help to
diff --git a/README.md b/README.md
index f17af66a97..f920a42fad 100644
--- a/README.md
+++ b/README.md
@@ -36,6 +36,9 @@ the body to majord...@vger.kernel.org. The mailing list 
archives are
 available at <https://public-inbox.org/git/>,
 <http://marc.info/?l=git> and other archival sites.
 
+Issues which are security relevant should be disclosed privately to
+the Git Security mailing list .
+
 The maintainer frequently sends the "What's cooking" reports that
 list the current status of various development topics to the mailing
 list.  The discussion following them give a good reference for
-- 
2.17.0.1181.g093e983b0



Re: [PATCH] README: note git-secur...@googlegroups.com

2018-05-27 Thread Jonathan Nieder
Thomas Gummerer wrote:

> Add a mention of the security mailing list to the README.
> 2caa7b8d27 ("git manpage: note git-secur...@googlegroups.com",
> 2018-03-08) already added it to the man page, but I suspect that for
> many developers, such as myself, the README would be the first place
> to go looking for it.
>
> Use the same wording as we already have on the git-scm.com website and
> in the man page.
>
> Signed-off-by: Thomas Gummerer 
> ---
>  README.md | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Jonathan Nieder 

> 2caa7b8d27 ("git manpage: note git-secur...@googlegroups.com",
> 2018-03-08) also mentions SubmittingPatches, but I think people are
> much more likely to submit a report of a security issue first, rather
> than sending a patch, for which I think the README is more useful.

I don't see a mention of SubmittingPatches in "git show 2caa7b8d27"
output.  git help git tells me:

Report bugs to the Git mailing list 
where the development and maintenance is primarily done. You
do not have to be subscribed to the list to send a message
there.

Issues which are security relevant should be disclosed
privately to the Git Security mailing list
.

Do you mean that the discussion around that change suggested updating
SubmittingPatches too?  The "Sending your patches" section indeed
mentions git@vger.kernel.org, so a mention of the security list would
indeed be welcome there, even though typically the discussion has
already started there before a patch is written.

Thanks,
Jonathan


[PATCH] README: note git-secur...@googlegroups.com

2018-05-27 Thread Thomas Gummerer
Add a mention of the security mailing list to the README.
2caa7b8d27 ("git manpage: note git-secur...@googlegroups.com",
2018-03-08) already added it to the man page, but I suspect that for
many developers, such as myself, the README would be the first place
to go looking for it.

Use the same wording as we already have on the git-scm.com website and
in the man page.

Signed-off-by: Thomas Gummerer 
---

2caa7b8d27 ("git manpage: note git-secur...@googlegroups.com",
2018-03-08) also mentions SubmittingPatches, but I think people are
much more likely to submit a report of a security issue first, rather
than sending a patch, for which I think the README is more useful.

 README.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/README.md b/README.md
index f17af66a97..f920a42fad 100644
--- a/README.md
+++ b/README.md
@@ -36,6 +36,9 @@ the body to majord...@vger.kernel.org. The mailing list 
archives are
 available at <https://public-inbox.org/git/>,
 <http://marc.info/?l=git> and other archival sites.
 
+Issues which are security relevant should be disclosed privately to
+the Git Security mailing list .
+
 The maintainer frequently sends the "What's cooking" reports that
 list the current status of various development topics to the mailing
 list.  The discussion following them give a good reference for
-- 
2.17.0.921.gf22659ad46



[PATCH v3 12/15] show-branch: note about its object flags usage

2018-05-18 Thread Nguyễn Thái Ngọc Duy
This is another candidate for commit-slab. Keep Junio's observation in
code so we can search it later on when somebody wants to improve the
code.
---
 builtin/show-branch.c | 5 +
 object.h  | 1 +
 2 files changed, 6 insertions(+)

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 29d15d16d2..f2e985c00a 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -22,6 +22,11 @@ static int showbranch_use_color = -1;
 
 static struct argv_array default_args = ARGV_ARRAY_INIT;
 
+/*
+ * TODO: convert this use of commit->object.flags to commit-slab
+ * instead to store a pointer to ref name directly. Then use the same
+ * UNINTERESTING definition from revision.h here.
+ */
 #define UNINTERESTING  01
 
 #define REV_SHIFT   2
diff --git a/object.h b/object.h
index b8e70e5519..caf36529f3 100644
--- a/object.h
+++ b/object.h
@@ -43,6 +43,7 @@ struct object_array {
  * builtin/index-pack.c: 2021
  * builtin/pack-objects.c:   20
  * builtin/reflog.c:   10--12
+ * builtin/show-branch.c:0---26
  * builtin/unpack-objects.c: 2021
  */
 #define FLAG_BITS  27
-- 
2.17.0.705.g3525833791



[PATCH v3 6/7] doc: add note about shell quoting to revision.txt

2018-05-03 Thread Andreas Heiduk
Signed-off-by: Andreas Heiduk 
Reviewed-by: Junio C Hamano 
---
 Documentation/revisions.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index dfcc49c72c..e760416d07 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -7,6 +7,10 @@ syntax.  Here are various ways to spell object names.  The
 ones listed near the end of this list name trees and
 blobs contained in a commit.
 
+NOTE: This document shows the "raw" syntax as seen by git. The shell
+and other UIs might require additional quoting to protect special
+characters and to avoid word splitting.
+
 '', e.g. 'dae86e1950b1277e545cee180551750029cfe735', 'dae86e'::
   The full SHA-1 object name (40-byte hexadecimal string), or
   a leading substring that is unique within the repository.
@@ -186,6 +190,8 @@ existing tag object.
   is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a
   literal '!' character, followed by 'foo'. Any other sequence beginning with
   ':/!' is reserved for now.
+  Depending on the given text, the shell's word splitting rules might
+  require additional quoting.
 
 ':', e.g. 'HEAD:README', ':README', 'master:./README'::
   A suffix ':' followed by a path names the blob or tree
-- 
2.16.2



Re: [PATCH v2 6/6] doc: add note about shell quoting to revision.txt

2018-04-27 Thread Andreas Heiduk
Am 27.04.2018 um 19:36 schrieb Eric Sunshine:
> On Fri, Apr 27, 2018 at 1:04 PM, Andreas Heiduk  wrote:
>> Signed-off-by: Andreas Heiduk 
>> Reviewed-by: Junio C Hamano 
>> ---
>> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
>> @@ -186,6 +190,8 @@ existing tag object.
>> +  Depending on the given text the shell's word splitting rules might
>> +  require additional quoting.
> 
> s/text/&,/
> 

Fixed, Thanks


Re: [PATCH v2 6/6] doc: add note about shell quoting to revision.txt

2018-04-27 Thread Eric Sunshine
On Fri, Apr 27, 2018 at 1:04 PM, Andreas Heiduk  wrote:
> Signed-off-by: Andreas Heiduk 
> Reviewed-by: Junio C Hamano 
> ---
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> @@ -186,6 +190,8 @@ existing tag object.
> +  Depending on the given text the shell's word splitting rules might
> +  require additional quoting.

s/text/&,/


[PATCH v2 6/6] doc: add note about shell quoting to revision.txt

2018-04-27 Thread Andreas Heiduk
Signed-off-by: Andreas Heiduk 
Reviewed-by: Junio C Hamano 
---
 Documentation/revisions.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index dfcc49c72c..c1d3a40a90 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -7,6 +7,10 @@ syntax.  Here are various ways to spell object names.  The
 ones listed near the end of this list name trees and
 blobs contained in a commit.
 
+NOTE: This document shows the "raw" syntax as seen by git. The shell
+and other UIs might require additional quoting to protect special
+characters and to avoid word splitting.
+
 '', e.g. 'dae86e1950b1277e545cee180551750029cfe735', 'dae86e'::
   The full SHA-1 object name (40-byte hexadecimal string), or
   a leading substring that is unique within the repository.
@@ -186,6 +190,8 @@ existing tag object.
   is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a
   literal '!' character, followed by 'foo'. Any other sequence beginning with
   ':/!' is reserved for now.
+  Depending on the given text the shell's word splitting rules might
+  require additional quoting.
 
 ':', e.g. 'HEAD:README', ':README', 'master:./README'::
   A suffix ':' followed by a path names the blob or tree
-- 
2.16.2



Re: [PATCH 6/6] doc: add note about shell quoting to revision.txt

2018-04-10 Thread Junio C Hamano
Andreas Heiduk  writes:

> Signed-off-by: Andreas Heiduk 
> ---
>  Documentation/revisions.txt | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index dfcc49c72c..c1d3a40a90 100644
> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -7,6 +7,10 @@ syntax.  Here are various ways to spell object names.  The
>  ones listed near the end of this list name trees and
>  blobs contained in a commit.
>  
> +NOTE: This document shows the "raw" syntax as seen by git. The shell
> +and other UIs might require additional quoting to protect special
> +characters and to avoid word splitting.
> +
>  '', e.g. 'dae86e1950b1277e545cee180551750029cfe735', 'dae86e'::
>The full SHA-1 object name (40-byte hexadecimal string), or
>a leading substring that is unique within the repository.
> @@ -186,6 +190,8 @@ existing tag object.
>is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a
>literal '!' character, followed by 'foo'. Any other sequence beginning with
>':/!' is reserved for now.
> +  Depending on the given text the shell's word splitting rules might
> +  require additional quoting.
>  
>  ':', e.g. 'HEAD:README', ':README', 'master:./README'::
>A suffix ':' followed by a path names the blob or tree

I've seen this suggested before and thought it is a good idea.  GOod
to see it is finally happening ;-)  Thanks.


[PATCH 6/6] doc: add note about shell quoting to revision.txt

2018-04-10 Thread Andreas Heiduk
Signed-off-by: Andreas Heiduk 
---
 Documentation/revisions.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index dfcc49c72c..c1d3a40a90 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -7,6 +7,10 @@ syntax.  Here are various ways to spell object names.  The
 ones listed near the end of this list name trees and
 blobs contained in a commit.
 
+NOTE: This document shows the "raw" syntax as seen by git. The shell
+and other UIs might require additional quoting to protect special
+characters and to avoid word splitting.
+
 '', e.g. 'dae86e1950b1277e545cee180551750029cfe735', 'dae86e'::
   The full SHA-1 object name (40-byte hexadecimal string), or
   a leading substring that is unique within the repository.
@@ -186,6 +190,8 @@ existing tag object.
   is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a
   literal '!' character, followed by 'foo'. Any other sequence beginning with
   ':/!' is reserved for now.
+  Depending on the given text the shell's word splitting rules might
+  require additional quoting.
 
 ':', e.g. 'HEAD:README', ':README', 'master:./README'::
   A suffix ':' followed by a path names the blob or tree
-- 
2.16.2



Re: [PATCH] git manpage: note git-secur...@googlegroups.com

2018-03-08 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> Add a mention of the security mailing list to the "Reporting Bugs"
> section. There's a mention of this list at
> https://git-scm.com/community but none in git.git itself.

This is quite a sensible thing to do.

>
> The copy is pasted from the git-scm.com website. Let's use the same
> wording in both places.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> Someone at Git Merge mentioned that our own docs have no mention of
> how to report security issues. Perhaps this should be in
> SubmittingPatches too, but I couldn't figure out how that magical
> footnote format works.

The "Notes from the maintainer" posted periodically here for
developers does mention it, and I do agree with you that
SubmittingPatches is a good place to add it, as it is a document
that is targetted more towards developers.  But this is a good first
step.

Will queue.

>
>  Documentation/git.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 8163b5796b..4767860e72 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -849,6 +849,9 @@ Report bugs to the Git mailing list  
> where the
>  development and maintenance is primarily done.  You do not have to be
>  subscribed to the list to send a message there.
>  
> +Issues which are security relevant should be disclosed privately to
> +the Git Security mailing list .
> +
>  SEE ALSO
>  
>  linkgit:gittutorial[7], linkgit:gittutorial-2[7],


[PATCH] git manpage: note git-secur...@googlegroups.com

2018-03-08 Thread Ævar Arnfjörð Bjarmason
Add a mention of the security mailing list to the "Reporting Bugs"
section. There's a mention of this list at
https://git-scm.com/community but none in git.git itself.

The copy is pasted from the git-scm.com website. Let's use the same
wording in both places.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
Someone at Git Merge mentioned that our own docs have no mention of
how to report security issues. Perhaps this should be in
SubmittingPatches too, but I couldn't figure out how that magical
footnote format works.

 Documentation/git.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 8163b5796b..4767860e72 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -849,6 +849,9 @@ Report bugs to the Git mailing list  
where the
 development and maintenance is primarily done.  You do not have to be
 subscribed to the list to send a message there.
 
+Issues which are security relevant should be disclosed privately to
+the Git Security mailing list .
+
 SEE ALSO
 
 linkgit:gittutorial[7], linkgit:gittutorial-2[7],
-- 
2.15.1.424.g9478a66081



[PATCH/RFC v3 05/12] pack-objects: note about in_pack_header_size

2018-03-08 Thread Nguyễn Thái Ngọc Duy
Object header in a pack is packed really tight (see
pack-format.txt). Even with 8 bytes length, we need 9-10 bytes most,
plus a hash (20 bytes). Which means this field only needs to store a
number as big as 32 (5 bits).

This is trickier to pack tight though since a new hash algorithm is
coming, the number of bits needed may quickly increase. So leave it
for now.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 pack-objects.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pack-objects.h b/pack-objects.h
index 4b17402953..2ccd6359d2 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -78,7 +78,7 @@ struct object_entry {
unsigned long z_delta_size; /* delta data size (compressed) */
uint32_t hash;  /* name hint hash */
unsigned int in_pack_pos;
-   unsigned char in_pack_header_size;
+   unsigned char in_pack_header_size; /* note: spare bits available! */
unsigned type:TYPE_BITS;
unsigned in_pack_type:TYPE_BITS; /* could be delta */
unsigned preferred_base:1; /*
-- 
2.16.2.873.g32ff258c87



[PATCH/RFC v2 5/9] pack-objects: note about in_pack_header_size

2018-03-02 Thread Nguyễn Thái Ngọc Duy
Object header in a pack is packed really tight (see
pack-format.txt). Even with 8 bytes length, we need 9-10 bytes most,
plus a hash (20 bytes). Which means this field only needs to store a
number as big as 32 (5 bits).

This is trickier to pack tight though since a new hash algorithm is
coming, the number of bits needed may quickly increase. So leave it
for now.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 pack-objects.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pack-objects.h b/pack-objects.h
index 2050a05a0b..fb2a3c8f48 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -32,7 +32,7 @@ struct object_entry {
unsigned long z_delta_size; /* delta data size (compressed) */
uint32_t hash;  /* name hint hash */
unsigned int in_pack_pos;
-   unsigned char in_pack_header_size;
+   unsigned char in_pack_header_size; /* note: spare bits available! */
unsigned type:TYPE_BITS;
unsigned in_pack_type:TYPE_BITS; /* could be delta */
unsigned preferred_base:1; /*
-- 
2.16.1.435.g8f24da2e1a



[PATCH 05/11] pack-objects: note about in_pack_header_size

2018-03-01 Thread Nguyễn Thái Ngọc Duy
Object header in a pack is packed really tight (see
pack-format.txt). Even with 8 bytes length, we need 9-10 bytes most,
plus a hash (20 bytes). Which means this field only needs to store a
number as big as 32 (5 bits).

This is trickier to pack tight though since a new hash algorithm is
coming, the number of bits needed may quickly increase. So leave it
for now.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 pack-objects.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pack-objects.h b/pack-objects.h
index 3941e6c9a6..017cc3425f 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -32,7 +32,7 @@ struct object_entry {
unsigned long z_delta_size; /* delta data size (compressed) */
uint32_t hash;  /* name hint hash */
unsigned int in_pack_pos;
-   unsigned char in_pack_header_size;
+   unsigned char in_pack_header_size; /* note: spare bits available! */
unsigned type:TYPE_BITS;
unsigned in_pack_type:TYPE_BITS; /* could be delta */
unsigned preferred_base:1; /*
-- 
2.16.1.435.g8f24da2e1a



[PATCH 10/11] t/README: add a note about don't saving stderr of compound commands

2018-02-23 Thread SZEDER Gábor
Explain in 't/README' why it is a bad idea to redirect and verify the
stderr of compound commands, in the hope that future contributions
will follow this advice and the test suite will keep working with '-x'
tracing and /bin/sh.

While at it, since we can now run the test suite with '-x' without
needing a Bash version supporting BASH_XTRACEFD, remove the now
outdated caution note about non-Bash shells from the description of
the '-x' option.

Signed-off-by: SZEDER Gábor 
---
 t/README | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/t/README b/t/README
index c430e9c52c..615682263e 100644
--- a/t/README
+++ b/t/README
@@ -84,9 +84,7 @@ appropriately before running "make".
 
 -x::
Turn on shell tracing (i.e., `set -x`) during the tests
-   themselves. Implies `--verbose`. Note that in non-bash shells,
-   this can cause failures in some tests which redirect and test
-   the output of shell functions. Use with caution.
+   themselves. Implies `--verbose`.
Ignored in test scripts that set the variable 'test_untraceable'
to a non-empty value, unless it's run with a Bash version
supporting BASH_XTRACEFD, i.e. v4.1 or later.
@@ -455,6 +453,22 @@ Don't:
causing the next test to start in an unexpected directory.  Do so
inside a subshell if necessary.
 
+ - save and verify the standard error of compound commands, i.e. group
+   commands, subshells, and shell functions (except test helper
+   functions like 'test_must_fail') like this:
+
+ ( cd dir && git cmd ) 2>error &&
+ test_cmp expect error
+
+   When running the test with '-x' tracing, then the trace of commands
+   executed in the compound command will be included in standard error
+   as well, quite possibly throwing off the subsequent checks examining
+   the output.  Instead, save only the relevant git command's standard
+   error:
+
+ ( cd dir && git cmd 2>../error ) &&
+ test_cmp expect error
+
  - Break the TAP output
 
The raw output from your test may be interpreted by a TAP harness. TAP
-- 
2.16.2.400.g911b7cc0da



Re: [PATCH 1/2] update-index doc: note a fixed bug in the untracked cache

2018-02-09 Thread Ævar Arnfjörð Bjarmason

On Fri, Feb 09 2018, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  writes:
>
>>> Will queue with the above typofix, together with the other one.  I
>>> am not sure if we want to say "Before 2.17", though.
>>
>> I'm just keeping in mind the user who later on upgrades git from say
>> 2.14 to 2.18 or something, and is able to find in the docs when/why this
>> new warning got added, which helps diagnose it.
>
> Ah, no, that is not what I meant.  I just didn't think '2.17' in
> that sentence may not be understood as "Git version 2.17" by most
> readers.

Ah, I see. Yes I agree, sorry. Do you mind fixing it up to just say
"Before Git version 2.17, ..." ?


Re: [PATCH 1/2] update-index doc: note a fixed bug in the untracked cache

2018-02-09 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason  writes:

>> Will queue with the above typofix, together with the other one.  I
>> am not sure if we want to say "Before 2.17", though.
>
> I'm just keeping in mind the user who later on upgrades git from say
> 2.14 to 2.18 or something, and is able to find in the docs when/why this
> new warning got added, which helps diagnose it.

Ah, no, that is not what I meant.  I just didn't think '2.17' in
that sentence may not be understood as "Git version 2.17" by most
readers.


Re: [PATCH 1/2] update-index doc: note a fixed bug in the untracked cache

2018-02-09 Thread Ævar Arnfjörð Bjarmason

On Fri, Feb 09 2018, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason   writes:
>
>> +Before 2.17, the untracked cache had a bug where replacing a directory
>> +with a symlink to another directory could cause it to incorrectly show
>> +files tracked by git as untracked. See the "status: add a failing test
>> +showing a core.untrackedCache bug" commit to git.git. A workaround for
>> +that was (and this might work for other undiscoverd bugs in the
>> +future):
>
> s/undiscoverd/undiscovered/
>
> But more importantly, would it help _us_ to encourage people to
> squelch the diagnoses without telling us about potential breakage, I
> wonder, by telling them to do this for other undiscovered cases,
> too?

You mean including something like "if you see this the git ML would like
to hear about it"?

> Will queue with the above typofix, together with the other one.  I
> am not sure if we want to say "Before 2.17", though.

I'm just keeping in mind the user who later on upgrades git from say
2.14 to 2.18 or something, and is able to find in the docs when/why this
new warning got added, which helps diagnose it.

>> +
>> +
>> +$ git -c core.untrackedCache=false status
>> +
>> +
>> +This bug has also been shown to affect non-symlink cases of replacing
>> +a directory with a file when it comes to the internal structures of
>> +the untracked cache, but no case has been found where this resulted in
>> +wrong "git status" output.
>> +
>>  File System Monitor
>>  ---


Re: [PATCH 0/2] update-index doc: note new caveats in 2.17

2018-02-09 Thread Ævar Arnfjörð Bjarmason

On Fri, Feb 09 2018, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason   writes:
>
>> When users upgrade to 2.17 they're going to have git yelling at them
>> (as my users did) on existing checkouts, with no indication whatsoever
>> that it's due to the UC or how to fix it.
>
> Wait.  Are you saying that the new warning is "warning" against a
> condition that is not an error?

No I mean the warning itself gives you no indication what the solution
is, or why it might be happening, and because it's printed on every
occurrence we had "git status" invocations on some big repos where there
would be hundreds of lines of the same warning for different
now-nonexisting directories.

Deferring it and just printing "we had N cases of..." would be better,
but would make the logic a lot more complex, I'm not sure how common
this would be in the wild, but as noted happened on a large proportion
of our checkouts, so having something mentioning this in the docs is
good.

I only had that git version out for about an hour, but had some users rm
-rfing their checkouts and re-cloning because they couldn't figure out
how to fix it.

Is noting it in the docs going to help all those users? No, but at least
we'll have something Google-able they're likely to find.

>> ... doesn't it only warn under that mode? I.e.:
>>
>> -"could not open directory '%s'")
>> +"core.untrackedCache: could not open directory '%s'")
>
> For example, if it attempts to open a directory which does *not*
> have to exist, and sees an error from opendir() due to ENOENT, then
> I do not think it should be giving a warning.  If we positively know
> that a directory should exist there and we cannot open it, of course
> we should warn.  Also, if we know a directory should be readable
> when it exists, then we should be warning when we see an error other
> than ENOENT.

*nod*, so not UC-specific, even though I've only seen it in relation to
that.


Re: [PATCH 1/2] update-index doc: note a fixed bug in the untracked cache

2018-02-09 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> +Before 2.17, the untracked cache had a bug where replacing a directory
> +with a symlink to another directory could cause it to incorrectly show
> +files tracked by git as untracked. See the "status: add a failing test
> +showing a core.untrackedCache bug" commit to git.git. A workaround for
> +that was (and this might work for other undiscoverd bugs in the
> +future):

s/undiscoverd/undiscovered/

But more importantly, would it help _us_ to encourage people to
squelch the diagnoses without telling us about potential breakage, I
wonder, by telling them to do this for other undiscovered cases,
too?

Will queue with the above typofix, together with the other one.  I
am not sure if we want to say "Before 2.17", though.

> +
> +
> +$ git -c core.untrackedCache=false status
> +
> +
> +This bug has also been shown to affect non-symlink cases of replacing
> +a directory with a file when it comes to the internal structures of
> +the untracked cache, but no case has been found where this resulted in
> +wrong "git status" output.
> +
>  File System Monitor
>  ---


Re: [PATCH 0/2] update-index doc: note new caveats in 2.17

2018-02-09 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> When users upgrade to 2.17 they're going to have git yelling at them
> (as my users did) on existing checkouts, with no indication whatsoever
> that it's due to the UC or how to fix it.

Wait.  Are you saying that the new warning is "warning" against a
condition that is not an error?

> ... doesn't it only warn under that mode? I.e.:
>
> -"could not open directory '%s'")
> +"core.untrackedCache: could not open directory '%s'")

For example, if it attempts to open a directory which does *not*
have to exist, and sees an error from opendir() due to ENOENT, then
I do not think it should be giving a warning.  If we positively know
that a directory should exist there and we cannot open it, of course
we should warn.  Also, if we know a directory should be readable
when it exists, then we should be warning when we see an error other
than ENOENT.

So...


[PATCH 1/2] update-index doc: note a fixed bug in the untracked cache

2018-02-09 Thread Ævar Arnfjörð Bjarmason
Document the bug tested for in my "status: add a failing test showing
a core.untrackedCache bug" and fixed in Duy's "dir.c: fix missing dir
invalidation in untracked code".

Since this is very likely something others will encounter in the
future on older versions, and it's not obvious how to fix it let's
document both that it exists, and how to "fix" it with a one-off
command.

As noted in that commit, even though this bug gets the untracked cache
into a bad state, we have not yet found a case where this is user
visible, and thus it makes sense for these docs to focus on the
symlink case only.

Signed-off-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-update-index.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index bdb0342593..e30b185918 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -464,6 +464,22 @@ command reads the index; while when 
`--[no-|force-]untracked-cache`
 are used, the untracked cache is immediately added to or removed from
 the index.
 
+Before 2.17, the untracked cache had a bug where replacing a directory
+with a symlink to another directory could cause it to incorrectly show
+files tracked by git as untracked. See the "status: add a failing test
+showing a core.untrackedCache bug" commit to git.git. A workaround for
+that was (and this might work for other undiscoverd bugs in the
+future):
+
+
+$ git -c core.untrackedCache=false status
+
+
+This bug has also been shown to affect non-symlink cases of replacing
+a directory with a file when it comes to the internal structures of
+the untracked cache, but no case has been found where this resulted in
+wrong "git status" output.
+
 File System Monitor
 ---
 
-- 
2.15.1.424.g9478a66081



[PATCH 0/2] update-index doc: note new caveats in 2.17

2018-02-09 Thread Ævar Arnfjörð Bjarmason
On Fri, Feb 09 2018, Junio C. Hamano jotted:
> Ævar Arnfjörð Bjarmason  writes:
>> I think you / Nguyễn may not have seen my
>> <87d11omi2o@evledraar.gmail.com>
>> (https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)
>>
>> As noted there I think it's best to proceed without the "dir.c: stop
>> ignoring opendir[...]" patch.
>>
>> It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
>> of warnings in previously working repos if the UC is on.
>
> Well, I am not sure if it is a regression to diagnose problematic
> untracked cache information left by earlier version of the software
> with bugs.  After all, this is still an experimental feature, and we
> do want to see the warning to serve its purpose to diagnose possible
> remaining bugs, and new similar ones when a newer iteration of the
> feature introduces, no?

Fair enough. I just wanted to make sure it had been seen. It wasn't
obvious whether it had just been missed since there was no follow-up
there or note in WC.

We were disagreeing to the extent that some of this should be
documented in 878tcbmbqb@evledraar.gmail.com and related, and I
see you've ejected the doc patch I had in that series.

I really think we should be saying something like what this new doc
series says, it's conceptually on top of Duy's series but applies on
top of master.

I think there's room to quibble about whether to include 1/2 at all
since it's a relatively obscure edge case.

2/2 however is something I think a *lot* of users are going to hit. I
just skimmed the relevant bits of git-config and git-update-index now,
and nothing describes the UC as an experimental feature, and it's been
well-known to improve performance.

When users upgrade to 2.17 they're going to have git yelling at them
(as my users did) on existing checkouts, with no indication whatsoever
that it's due to the UC or how to fix it.

Let's at least documente it. I also wonder if the "dir.c: stop
ignoring opendir() error in open_cached_dir()" change shouldn't have
something in the warning itself about it being caused by the UC,
doesn't it only warn under that mode? I.e.:

-"could not open directory '%s'")
+"core.untrackedCache: could not open directory '%s'")

Ævar Arnfjörð Bjarmason (2):
  update-index doc: note a fixed bug in the untracked cache
  update-index doc: note the caveat with "could not open..."

 Documentation/git-update-index.txt | 26 ++
 1 file changed, 26 insertions(+)

-- 
2.15.1.424.g9478a66081



[PATCH 2/2] update-index doc: note the caveat with "could not open..."

2018-02-09 Thread Ævar Arnfjörð Bjarmason
Note the caveat where 2.17 is stricter about index validation
potentially causing "could not open directory" warnings when git is
upgraded. See the preceding "dir.c: stop ignoring opendir() error in
open_cached_dir()" change.

This caused some mayhem when I upgraded git to a version with this
series at Booking.com, and other users have doubtless enabled the UC
extension and are in for a surprise when they upgrade. Let's give them
a headsup in the docs.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-update-index.txt | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index e30b185918..0c81600d8c 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -480,6 +480,16 @@ a directory with a file when it comes to the internal 
structures of
 the untracked cache, but no case has been found where this resulted in
 wrong "git status" output.
 
+There are also cases where existing indexes written by git versions
+before 2.17 will reference directories that don't exist anymore,
+potentially causing many "could not open directory" warnings to be
+printed on "git status". These are new warnings for existing issues
+that were previously silently discarded.
+
+As with the bug described above the solution is to one-off do a "git
+status" run with `core.untrackedCache=false` to flush out the leftover
+bad data.
+
 File System Monitor
 ---
 
-- 
2.15.1.424.g9478a66081



[PATCH 4/5] update-index doc: note a fixed bug in the untracked cache

2018-01-24 Thread Nguyễn Thái Ngọc Duy
From: Ævar Arnfjörð Bjarmason 

Document the bug tested for in my "status: add a failing test showing
a core.untrackedCache bug" and fixed in Duy's "dir.c: fix missing dir
invalidation in untracked code".

Since this is very likely something others will encounter in the
future on older versions, and it's not obvious how to fix it let's
document both that it exists, and how to "fix" it with a one-off
command.

As noted in that commit, even though this bug gets the untracked cache
into a bad state, we have not yet found a case where this is user
visible, and thus it makes sense for these docs to focus on the
symlink case only.

Signed-off-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-update-index.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index bdb0342593..128e0c671f 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -464,6 +464,22 @@ command reads the index; while when 
`--[no-|force-]untracked-cache`
 are used, the untracked cache is immediately added to or removed from
 the index.
 
+Before 2.16, the untracked cache had a bug where replacing a directory
+with a symlink to another directory could cause it to incorrectly show
+files tracked by git as untracked. See the "status: add a failing test
+showing a core.untrackedCache bug" commit to git.git. A workaround for
+that was (and this might work for other undiscoverd bugs in the
+future):
+
+
+$ git -c core.untrackedCache=false status
+
+
+This bug has also been shown to affect non-symlink cases of replacing
+a directory with a file when it comes to the internal structures of
+the untracked cache, but no case has been found where this resulted in
+wrong "git status" output.
+
 File System Monitor
 ---
 
-- 
2.16.0.47.g3d9b0fac3a



NOTE

2018-01-08 Thread Ahmed Zama
-- 
Greetings,

Can you handle a transaction that involves the transfer of fund valued
15 million Euros into a foreign account. I will give you the full
detailed information as soon as I hear from you. Send me the
followings if you are willing and ready to work with me.
1)Full Names (2)Occupation (3)Age (4)Nationality (5)Direct Mobile Line

Ahmed Zama
UBA BANK
OUAGADOUGOU BURKINA FASO


[PATCH v2 4/5] update-index doc: note a fixed bug in the untracked cache

2018-01-03 Thread Ævar Arnfjörð Bjarmason
Document the bug tested for in my "status: add a failing test showing
a core.untrackedCache bug" and fixed in Duy's "dir.c: fix missing dir
invalidation in untracked code".

Since this is very likely something others will encounter in the
future on older versions, and it's not obvious how to fix it let's
document both that it exists, and how to "fix" it with a one-off
command.

As noted in that commit, even though this bug gets the untracked cache
into a bad state, we have not yet found a case where this is user
visible, and thus it makes sense for these docs to focus on the
symlink case only.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-update-index.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index bdb0342593..128e0c671f 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -464,6 +464,22 @@ command reads the index; while when 
`--[no-|force-]untracked-cache`
 are used, the untracked cache is immediately added to or removed from
 the index.
 
+Before 2.16, the untracked cache had a bug where replacing a directory
+with a symlink to another directory could cause it to incorrectly show
+files tracked by git as untracked. See the "status: add a failing test
+showing a core.untrackedCache bug" commit to git.git. A workaround for
+that was (and this might work for other undiscoverd bugs in the
+future):
+
+
+$ git -c core.untrackedCache=false status
+
+
+This bug has also been shown to affect non-symlink cases of replacing
+a directory with a file when it comes to the internal structures of
+the untracked cache, but no case has been found where this resulted in
+wrong "git status" output.
+
 File System Monitor
 ---
 
-- 
2.15.1.424.g9478a66081



Re: [PATCH 3/1] update-index doc: note a fixed bug in the untracked cache

2017-12-27 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> Document the bug tested for in my "status: add a failing test showing
> a core.untrackedCache bug" and fixed in Duy's "dir.c: fix missing dir
> invalidation in untracked code".
>
> Since this is very likely something others will encounter in the
> future on older versions, and it's not obvious how to fix it let's
> document both that it exists, and how to "fix" it with a one-off
> command.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>
>> On Tue, Dec 26, 2017 at 05:47:19PM +0700, Duy Nguyen wrote:
>>> Strangely, root dir is not cached (no valid flag). I don't know why
>>> but that's ok we'll roll with it.
>>
>> I figured this out. Which is good because now I know how/why the bug
>> happens.
>
> Thanks a lot. I think it's probably good to apply something like this
> on top of this now 3-patch series.

Thanks both for working well together.  Now that the problem turns
out not about symbolic links, can we update the test in 1/1 not to
depend on symbolic links?

>
>  Documentation/git-update-index.txt | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/git-update-index.txt 
> b/Documentation/git-update-index.txt
> index bdb0342593..bc6c32002f 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -464,6 +464,17 @@ command reads the index; while when 
> `--[no-|force-]untracked-cache`
>  are used, the untracked cache is immediately added to or removed from
>  the index.
>  
> +Before 2.16, the untracked cache had a bug where replacing a directory
> +with a symlink to another directory could cause it to incorrectly show
> +files tracked by git as untracked. See the "status: add a failing test
> +showing a core.untrackedCache bug" commit to git.git. A workaround for
> +that was (and this might work for other undiscoverd bugs in the
> +future):
> +
> +
> +$ git -c core.untrackedCache=false status
> +
> +
>  File System Monitor
>  ---


[PATCH 3/1] update-index doc: note a fixed bug in the untracked cache

2017-12-27 Thread Ævar Arnfjörð Bjarmason
Document the bug tested for in my "status: add a failing test showing
a core.untrackedCache bug" and fixed in Duy's "dir.c: fix missing dir
invalidation in untracked code".

Since this is very likely something others will encounter in the
future on older versions, and it's not obvious how to fix it let's
document both that it exists, and how to "fix" it with a one-off
command.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

> On Tue, Dec 26, 2017 at 05:47:19PM +0700, Duy Nguyen wrote:
>> Strangely, root dir is not cached (no valid flag). I don't know why
>> but that's ok we'll roll with it.
>
> I figured this out. Which is good because now I know how/why the bug
> happens.

Thanks a lot. I think it's probably good to apply something like this
on top of this now 3-patch series.

 Documentation/git-update-index.txt | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index bdb0342593..bc6c32002f 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -464,6 +464,17 @@ command reads the index; while when 
`--[no-|force-]untracked-cache`
 are used, the untracked cache is immediately added to or removed from
 the index.
 
+Before 2.16, the untracked cache had a bug where replacing a directory
+with a symlink to another directory could cause it to incorrectly show
+files tracked by git as untracked. See the "status: add a failing test
+showing a core.untrackedCache bug" commit to git.git. A workaround for
+that was (and this might work for other undiscoverd bugs in the
+future):
+
+
+$ git -c core.untrackedCache=false status
+
+
 File System Monitor
 ---
 
-- 
2.15.1.424.g9478a66081



A note from the maintainer

2017-11-28 Thread Junio C Hamano
  git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
  git://repo.or.cz/git-{htmldocs,manpages}.git/
  https://github.com/gitster/git-{htmldocs,manpages}.git/

The manual pages formatted in HTML for the tip of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.15 done on Oct 30th, 2017.  You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  This branch is also
merged into "master" to propagate the fixes forward as needed.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "next" will be rebuilt from the tip of "master"
using the topics that didn't make the cut in the feature release.

A natural consequence of how "next" and "pu" bundles topics together
is that until a topic is merged to "next&

Re: [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications

2017-11-05 Thread Junio C Hamano
Ann T Ropea  writes:

> Also, fix typo: "three dot" ---> "three-dot" (align with "two-dot").
>
> Signed-off-by: Ann T Ropea 
> ---
>  Documentation/revisions.txt | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index 61277469c874..d1b126427177 100644
> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
>   for commits that are reachable from r2 excluding those that are reachable
>   from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
>  
> -The '...' (three dot) Symmetric Difference Notation::
> +The '...' (three-dot) Symmetric Difference Notation::
>   A similar notation 'r1\...r2' is called symmetric difference
>   of 'r1' and 'r2' and is defined as
>   'r1 r2 --not $(git merge-base --all r1 r2)'.
> @@ -285,6 +285,15 @@ is a shorthand for 'HEAD..origin' and asks "What did the 
> origin do since
>  I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
>  empty range that is both reachable and unreachable from HEAD.
>  
> +However, there are instances where '...' is *not*
> +equivalent to '...HEAD'.  See the "RAW OUTPUT FORMAT"
> +section of linkgit:git-diff[1]: the three-dot notations used
> +there are simply continuation indications for the abbreviated
> +SHA-1 values.  The ones encountered there are usually
> +associated with file/index/tree contents rather than with commit
> +objects, and the range operators described above are only
> +applicable to commit objects (i.e., 'r1' and 'r2').
> +
>  Other {caret} Parent Shorthand Notations
>  ~
>  Three other shorthands exist, particularly useful for merge commits,

I actually have a mild suspicion that this is going in a wrong
direction.  In very early days of Git, we wanted to make sure that
people can tell if a long hex string is a truncated object name or a
full one (mostly because some lower-level commands insisted to be
fed only full object name).

These days, everybody knows when they see 79ec0be62a that it is
*not* a full object name and will no longer be confused unlike early
days and there is no strong reason to waste six output columns of
"git diff --raw" output by using these three dots.  

I wonder if we can come up with a solution in line with the patch
1/3 in this series, which got rid of the "..." that indicated that
the hexstring was not a full object name.




[PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications

2017-11-05 Thread Ann T Ropea
Also, fix typo: "three dot" ---> "three-dot" (align with "two-dot").

Signed-off-by: Ann T Ropea 
---
 Documentation/revisions.txt | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 61277469c874..d1b126427177 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
  for commits that are reachable from r2 excluding those that are reachable
  from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
 
-The '...' (three dot) Symmetric Difference Notation::
+The '...' (three-dot) Symmetric Difference Notation::
  A similar notation 'r1\...r2' is called symmetric difference
  of 'r1' and 'r2' and is defined as
  'r1 r2 --not $(git merge-base --all r1 r2)'.
@@ -285,6 +285,15 @@ is a shorthand for 'HEAD..origin' and asks "What did the 
origin do since
 I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
 empty range that is both reachable and unreachable from HEAD.
 
+However, there are instances where '...' is *not*
+equivalent to '...HEAD'.  See the "RAW OUTPUT FORMAT"
+section of linkgit:git-diff[1]: the three-dot notations used
+there are simply continuation indications for the abbreviated
+SHA-1 values.  The ones encountered there are usually
+associated with file/index/tree contents rather than with commit
+objects, and the range operators described above are only
+applicable to commit objects (i.e., 'r1' and 'r2').
+
 Other {caret} Parent Shorthand Notations
 ~
 Three other shorthands exist, particularly useful for merge commits,
-- 
2.13.6



Re: A note from the maintainer

2017-10-30 Thread Johannes Schindelin
Hi Junio,

On Mon, 30 Oct 2017, Junio C Hamano wrote:

> The development is primarily done on the Git mailing list. Help
> requests, feature proposals, bug reports and patches should be sent to
> the list address .  You don't have to be
> subscribed to send messages.  The convention on the list is to keep
> everybody involved on Cc:, so it is unnecessary to say "Please Cc: me,
> I am not subscribed".

I have heard about a dozen complaints that mails were simply eaten by the
mailing list. At least some of those cases were due to HTML (or
HTML/plain) mails being quietly dropped, and it caused more than just
minor frustration.

Maybe mention this in your maintainer's note, to help stave off such
problems?

Ciao,
Dscho


A note from the maintainer

2017-10-29 Thread Junio C Hamano
p of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.15 done on Oct 30th, 2017.  You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  This branch is also
merged into "master" to propagate the fixes forward as needed.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "next" will be rebuilt from the tip of "master"
using the topics that didn't make the cut in the feature release.

A natural consequence of how "next" and "pu" bundles topics together
is that until a topic is merged to "next", updates to it is expected
by replacing the patch(es) in the topic with an improved version,
and once a topic is merged to "next", updates to it needs to come as
incremental patches, pointing out what was wron

A note from the maintainer

2017-08-04 Thread Junio C Hamano
p of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.14 done on Aug 4th, 2017.  You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  This branch is also
merged into "master" to propagate the fixes forward as needed.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "next" will be rebuilt from the tip of "master"
using the topics that didn't make the cut in the feature release.

A natural consequence of how "next" and "pu" bundles topics together
is that until a topic is merged to "next", updates to it is expected
by replacing the patch(es) in the topic with an improved version,
and once a topic is merged to "next", updates to it needs to come as
incremental patches, pointing out what was wron

NOTE

2017-07-20 Thread Bong Phang
BCEAO BANK TOGO has agreed to wire USD$ 7,500.000.00,get in touch with
me by my private email immediately: (myemailcham...@gmail.com)for more
details


A note from the maintainer

2017-07-13 Thread Junio C Hamano
p of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.12 done on Feb 24th, 2017. You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  This branch is also
merged into "master" to propagate the fixes forward as needed.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "next" will be rebuilt from the tip of "master"
using the topics that didn't make the cut in the feature release.

A natural consequence of how "next" and "pu" bundles topics together
is that until a topic is merged to "next", updates to it is expected
by replacing the patch(es) in the topic with an improved version,
and once a topic is merged to "next", updates to it needs to come as
incremental patches, pointing out what was wron

A note from the maintainer

2017-06-24 Thread Junio C Hamano
p of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.12 done on Feb 24th, 2017. You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  This branch is also
merged into "master" to propagate the fixes forward as needed.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "next" will be rebuilt from the tip of "master"
using the topics that didn't make the cut in the feature release.

A natural consequence of how "next" and "pu" bundles topics together
is that until a topic is merged to "next", updates to it is expected
by replacing the patch(es) in the topic with an improved version,
and once a topic is merged to "next", updates to it needs to come as
incremental patches, pointing out what was wron

[PATCH v6 10/11] run-command: add note about forking and threading

2017-04-19 Thread Brandon Williams
All non-Async-Signal-Safe functions (e.g. malloc and die) were removed
between 'fork' and 'exec' in start_command in order to avoid potential
deadlocking when forking while multiple threads are running.  This
deadlocking is possible when a thread (other than the one forking) has
acquired a lock and didn't get around to releasing it before the fork.
This leaves the lock in a locked state in the resulting process with no
hope of it ever being released.

Add a note describing this potential pitfall before the call to 'fork()'
so people working in this section of the code know to only use
Async-Signal-Safe functions in the child process.

Signed-off-by: Brandon Williams 
---
 run-command.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/run-command.c b/run-command.c
index 615b6e9c9..df1edd963 100644
--- a/run-command.c
+++ b/run-command.c
@@ -537,6 +537,15 @@ int start_command(struct child_process *cmd)
prepare_cmd(&argv, cmd);
childenv = prep_childenv(cmd->env);
 
+   /*
+* NOTE: In order to prevent deadlocking when using threads special
+* care should be taken with the function calls made in between the
+* fork() and exec() calls.  No calls should be made to functions which
+* require acquiring a lock (e.g. malloc) as the lock could have been
+* held by another thread at the time of forking, causing the lock to
+* never be released in the child process.  This means only
+* Async-Signal-Safe functions are permitted in the child.
+*/
cmd->pid = fork();
failed_errno = errno;
if (!cmd->pid) {
-- 
2.12.2.816.g281164-goog



[PATCH v5 10/11] run-command: add note about forking and threading

2017-04-18 Thread Brandon Williams
All non-Async-Signal-Safe functions (e.g. malloc and die) were removed
between 'fork' and 'exec' in start_command in order to avoid potential
deadlocking when forking while multiple threads are running.  This
deadlocking is possible when a thread (other than the one forking) has
acquired a lock and didn't get around to releasing it before the fork.
This leaves the lock in a locked state in the resulting process with no
hope of it ever being released.

Add a note describing this potential pitfall before the call to 'fork()'
so people working in this section of the code know to only use
Async-Signal-Safe functions in the child process.

Signed-off-by: Brandon Williams 
---
 run-command.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/run-command.c b/run-command.c
index 615b6e9c9..df1edd963 100644
--- a/run-command.c
+++ b/run-command.c
@@ -537,6 +537,15 @@ int start_command(struct child_process *cmd)
prepare_cmd(&argv, cmd);
childenv = prep_childenv(cmd->env);
 
+   /*
+* NOTE: In order to prevent deadlocking when using threads special
+* care should be taken with the function calls made in between the
+* fork() and exec() calls.  No calls should be made to functions which
+* require acquiring a lock (e.g. malloc) as the lock could have been
+* held by another thread at the time of forking, causing the lock to
+* never be released in the child process.  This means only
+* Async-Signal-Safe functions are permitted in the child.
+*/
cmd->pid = fork();
failed_errno = errno;
if (!cmd->pid) {
-- 
2.12.2.816.g281164-goog



[PATCH v4 09/10] run-command: add note about forking and threading

2017-04-17 Thread Brandon Williams
All non-Async-Signal-Safe functions (e.g. malloc and die) were removed
between 'fork' and 'exec' in start_command in order to avoid potential
deadlocking when forking while multiple threads are running.  This
deadlocking is possible when a thread (other than the one forking) has
acquired a lock and didn't get around to releasing it before the fork.
This leaves the lock in a locked state in the resulting process with no
hope of it ever being released.

Add a note describing this potential pitfall before the call to 'fork()'
so people working in this section of the code know to only use
Async-Signal-Safe functions in the child process.

Signed-off-by: Brandon Williams 
---
 run-command.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/run-command.c b/run-command.c
index bd6414283..c27c53bc5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -558,6 +558,15 @@ int start_command(struct child_process *cmd)
prepare_cmd(&argv, cmd);
childenv = prep_childenv(cmd->env);
 
+   /*
+* NOTE: In order to prevent deadlocking when using threads special
+* care should be taken with the function calls made in between the
+* fork() and exec() calls.  No calls should be made to functions which
+* require acquiring a lock (e.g. malloc) as the lock could have been
+* held by another thread at the time of forking, causing the lock to
+* never be released in the child process.  This means only
+* Async-Signal-Safe functions are permitted in the child.
+*/
cmd->pid = fork();
failed_errno = errno;
if (!cmd->pid) {
-- 
2.12.2.762.g0e3151a226-goog



[PATCH v3 09/10] run-command: add note about forking and threading

2017-04-14 Thread Brandon Williams
All non-Async-Signal-Safe functions (e.g. malloc and die) were removed
between 'fork' and 'exec' in start_command in order to avoid potential
deadlocking when forking while multiple threads are running.  This
deadlocking is possible when a thread (other than the one forking) has
acquired a lock and didn't get around to releasing it before the fork.
This leaves the lock in a locked state in the resulting process with no
hope of it ever being released.

Add a note describing this potential pitfall before the call to 'fork()'
so people working in this section of the code know to only use
Async-Signal-Safe functions in the child process.

Signed-off-by: Brandon Williams 
---
 run-command.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/run-command.c b/run-command.c
index f36eafa8d..d3a32eab6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -557,6 +557,15 @@ int start_command(struct child_process *cmd)
prepare_cmd(&argv, cmd);
childenv = prep_childenv(cmd->env);
 
+   /*
+* NOTE: In order to prevent deadlocking when using threads special
+* care should be taken with the function calls made in between the
+* fork() and exec() calls.  No calls should be made to functions which
+* require acquiring a lock (e.g. malloc) as the lock could have been
+* held by another thread at the time of forking, causing the lock to
+* never be released in the child process.  This means only
+* Async-Signal-Safe functions are permitted in the child.
+*/
cmd->pid = fork();
failed_errno = errno;
if (!cmd->pid) {
-- 
2.12.2.762.g0e3151a226-goog



[PATCH v2 6/6] run-command: add note about forking and threading

2017-04-13 Thread Brandon Williams
All non-Async-Signal-Safe functions (e.g. malloc and die) were removed
between 'fork' and 'exec' in start_command in order to avoid potential
deadlocking when forking while multiple threads are running.  This
deadlocking is possible when a thread (other than the one forking) has
acquired a lock and didn't get around to releasing it before the fork.
This leaves the lock in a locked state in the resulting process with no
hope of it ever being released.

Add a note describing this potential pitfall before the call to 'fork()'
so people working in this section of the code know to only use
Async-Signal-Safe functions in the child process.

Signed-off-by: Brandon Williams 
---
 run-command.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/run-command.c b/run-command.c
index 4230c4933..1c36e692d 100644
--- a/run-command.c
+++ b/run-command.c
@@ -525,6 +525,15 @@ int start_command(struct child_process *cmd)
prepare_cmd(&argv, cmd);
childenv = prep_childenv(cmd->env);
 
+   /*
+* NOTE: In order to prevent deadlocking when using threads special
+* care should be taken with the function calls made in between the
+* fork() and exec() calls.  No calls should be made to functions which
+* require acquiring a lock (e.g. malloc) as the lock could have been
+* held by another thread at the time of forking, causing the lock to
+* never be released in the child process.  This means only
+* Async-Signal-Safe functions are permitted in the child.
+*/
cmd->pid = fork();
failed_errno = errno;
if (!cmd->pid) {
-- 
2.12.2.762.g0e3151a226-goog



Re: [PATCH 5/5] run-command: add note about forking and threading

2017-04-11 Thread Eric Wong
Brandon Williams  wrote:
> On 04/11, Eric Wong wrote:
> > On the other hand, I believe we should make run-command
> > vfork-compatible (and Brandon's series is a big (but incomplete)
> > step in the (IMHO) right direction); as anything which is
> > vfork-safe would also be safe in the presence of threads+(plain) fork.
> > With vfork; the two processes share heap until execve.
> 
> I haven't looked to much into vfork, one of the benefits of vfork is
> that it is slightly more preferment than vanilla fork correct?  What are
> some of the other benefits of using vfork over fork?

Yes, mainly performance and perhaps portability...  Last I
checked (over a decade ago); uCLinux without MMU could not
fork processes; only vfork.


Re: [PATCH 5/5] run-command: add note about forking and threading

2017-04-11 Thread Brandon Williams
On 04/11, Eric Wong wrote:
> Jonathan Nieder  wrote:
> > Why can't git use e.g. posix_spawn to avoid this?
> 
> posix_spawn does not support chdir, and it seems we run non-git
> commands so no using "git -C" for those.

This is actually the biggest reason why I didn't go down that route from
the start.  I didn't want to dig through each and every user of
run-command and verify that removing chdir wouldn't break them (or add
in some other way to do it).

> 
> > fork()-ing in a threaded context is very painful for maintainability.
> > Any library function you are using could start taking a lock, and then
> > you have a deadlock.  So you have to make use of a very small
> > whitelisted list of library functions for this to work.
> 
> Completely agreed.

Yes it is difficult to get right, but it seems very doable to just do
all of the heavy-lifting prior to fork/exec making it easier to just use
async-safe between the fork/exec in the child.

> 
> On the other hand, I believe we should make run-command
> vfork-compatible (and Brandon's series is a big (but incomplete)
> step in the (IMHO) right direction); as anything which is
> vfork-safe would also be safe in the presence of threads+(plain) fork.
> With vfork; the two processes share heap until execve.

I haven't looked to much into vfork, one of the benefits of vfork is
that it is slightly more preferment than vanilla fork correct?  What are
some of the other benefits of using vfork over fork?

> 
> I posted some notes about it last year:
> 
>   https://public-inbox.org/git/20160629200142.ga17...@dcvr.yhbt.net/
> 
> > The function calls you have to audit are not only between fork() and
> > exec() in the normal control flow.  You have to worry about signal
> > handlers, too.
> 
> Yes, all that auditing is necessary for vfork; too, but totally
> doable.  The mainline Ruby implementation has been using vfork
> for spawning subprocesses for several years, now; and I think the
> ruby-core developers (myself included) have fixed all the
> problems with it; even in multi-threaded code which calls malloc.

-- 
Brandon Williams


Re: [PATCH 5/5] run-command: add note about forking and threading

2017-04-11 Thread Jonathan Nieder
Eric Wong wrote:
> Jonathan Nieder  wrote:

>> Why can't git use e.g. posix_spawn to avoid this?
>
> posix_spawn does not support chdir, and it seems we run non-git
> commands so no using "git -C" for those.

On the other hand, a number of the non-git commands we run are in a
shell.  At the cost of a wasted shell process, other commands can
be spawned using posix_spawn by passing the chosen directory and
command to

sh -c 'cd "$1" && shift && exec "$@"' -

[...]
> I posted some notes about it last year:
>
>   https://public-inbox.org/git/20160629200142.ga17...@dcvr.yhbt.net/

Thanks for these notes.

Sincerely,
Jonathan


Re: [PATCH 5/5] run-command: add note about forking and threading

2017-04-10 Thread Eric Wong
Jonathan Nieder  wrote:
> Hi,
> 
> Brandon Williams wrote:
> 
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -458,6 +458,14 @@ int start_command(struct child_process *cmd)
> > argv_array_pushv(&argv, cmd->argv);
> > }
> >  
> > +   /*
> > +* NOTE: In order to prevent deadlocking when using threads special
> > +* care should be taken with the function calls made in between the
> > +* fork() and exec() calls.  No calls should be made to functions which
> > +* require acquiring a lock (e.g. malloc) as the lock could have been
> > +* held by another thread at the time of forking, causing the lock to
> > +* never be released in the child process.
> > +*/
> > cmd->pid = fork();
> 
> Why can't git use e.g. posix_spawn to avoid this?

posix_spawn does not support chdir, and it seems we run non-git
commands so no using "git -C" for those.

> fork()-ing in a threaded context is very painful for maintainability.
> Any library function you are using could start taking a lock, and then
> you have a deadlock.  So you have to make use of a very small
> whitelisted list of library functions for this to work.

Completely agreed.

On the other hand, I believe we should make run-command
vfork-compatible (and Brandon's series is a big (but incomplete)
step in the (IMHO) right direction); as anything which is
vfork-safe would also be safe in the presence of threads+(plain) fork.
With vfork; the two processes share heap until execve.

I posted some notes about it last year:

  https://public-inbox.org/git/20160629200142.ga17...@dcvr.yhbt.net/

> The function calls you have to audit are not only between fork() and
> exec() in the normal control flow.  You have to worry about signal
> handlers, too.

Yes, all that auditing is necessary for vfork; too, but totally
doable.  The mainline Ruby implementation has been using vfork
for spawning subprocesses for several years, now; and I think the
ruby-core developers (myself included) have fixed all the
problems with it; even in multi-threaded code which calls malloc.


Re: [PATCH 5/5] run-command: add note about forking and threading

2017-04-10 Thread Jonathan Nieder
Hi,

Brandon Williams wrote:

> --- a/run-command.c
> +++ b/run-command.c
> @@ -458,6 +458,14 @@ int start_command(struct child_process *cmd)
>   argv_array_pushv(&argv, cmd->argv);
>   }
>  
> + /*
> +  * NOTE: In order to prevent deadlocking when using threads special
> +  * care should be taken with the function calls made in between the
> +  * fork() and exec() calls.  No calls should be made to functions which
> +  * require acquiring a lock (e.g. malloc) as the lock could have been
> +  * held by another thread at the time of forking, causing the lock to
> +  * never be released in the child process.
> +  */
>   cmd->pid = fork();

Why can't git use e.g. posix_spawn to avoid this?

fork()-ing in a threaded context is very painful for maintainability.
Any library function you are using could start taking a lock, and then
you have a deadlock.  So you have to make use of a very small
whitelisted list of library functions for this to work.

The function calls you have to audit are not only between fork() and
exec() in the normal control flow.  You have to worry about signal
handlers, too.

By comparison, posix_spawn() takes care of this all.  I really
strongly do not want to move in the direction of more fork() in a
multi-threaded context.  I'd rather turn off threads in the submodule
case of grep altogether, but that shouldn't be needed:

* it is possible to launch a command without fork+exec
* if that's not suitable, then using fork introduces parallelism that
  interacts much better with fork+exec than threads do

I really don't want to go down this path.  I've said that a few times.
I'm saying it again now.

My two cents,
Jonathan


[PATCH 5/5] run-command: add note about forking and threading

2017-04-10 Thread Brandon Williams
Allocation was pushed before forking in order to avoid potential
deadlocking when forking while multiple threads are running.  This
deadlocking is possible when a thread (other than the one forking) has
acquired a lock and didn't get around to releasing it before the fork.
This leaves the lock in a locked state in the resulting process with no
hope of it ever being released.

Signed-off-by: Brandon Williams 
---
 run-command.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/run-command.c b/run-command.c
index 84c63b209..2b3249de4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -458,6 +458,14 @@ int start_command(struct child_process *cmd)
argv_array_pushv(&argv, cmd->argv);
}
 
+   /*
+    * NOTE: In order to prevent deadlocking when using threads special
+* care should be taken with the function calls made in between the
+* fork() and exec() calls.  No calls should be made to functions which
+* require acquiring a lock (e.g. malloc) as the lock could have been
+* held by another thread at the time of forking, causing the lock to
+* never be released in the child process.
+*/
cmd->pid = fork();
failed_errno = errno;
if (!cmd->pid) {
-- 
2.12.2.715.g7642488e1d-goog



[PATCH v7 28/28] refs.h: add a note about sorting order of for_each_ref_*

2017-03-25 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 refs.h | 4 ++--
 t/t1405-main-ref-store.sh  | 6 ++
 t/t1406-submodule-ref-store.sh | 6 ++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/refs.h b/refs.h
index 1a07f9d86f..49e97d7d5f 100644
--- a/refs.h
+++ b/refs.h
@@ -230,7 +230,7 @@ typedef int each_ref_fn(const char *refname,
  * it is not safe to modify references while an iteration is in
  * progress, unless the same callback function invocation that
  * modifies the reference also returns a nonzero value to immediately
- * stop the iteration.
+ * stop the iteration. Returned references are sorted.
  */
 int refs_for_each_ref(struct ref_store *refs,
  each_ref_fn fn, void *cb_data);
@@ -370,7 +370,7 @@ int for_each_reflog_ent_reverse(const char *refname, 
each_reflog_ent_fn fn, void
 
 /*
  * Calls the specified function for each reflog file until it returns nonzero,
- * and returns the value
+ * and returns the value. Reflog file order is unspecified.
  */
 int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void 
*cb_data);
 int for_each_reflog(each_ref_fn fn, void *cb_data);
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 77e1c130c2..490521f8cb 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -53,6 +53,12 @@ test_expect_success 'for_each_ref(refs/heads/)' '
test_cmp expected actual
 '
 
+test_expect_success 'for_each_ref() is sorted' '
+   $RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+   sort actual > expected &&
+   test_cmp expected actual
+'
+
 test_expect_success 'resolve_ref(new-master)' '
SHA1=`git rev-parse new-master` &&
echo "$SHA1 refs/heads/new-master 0x0" >expected &&
diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index 22214ebd32..13b5454c56 100755
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -47,6 +47,12 @@ test_expect_success 'for_each_ref(refs/heads/)' '
test_cmp expected actual
 '
 
+test_expect_success 'for_each_ref() is sorted' '
+   $RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+   sort actual > expected &&
+   test_cmp expected actual
+'
+
 test_expect_success 'resolve_ref(master)' '
SHA1=`git -C sub rev-parse master` &&
echo "$SHA1 refs/heads/master 0x0" >expected &&
-- 
2.11.0.157.gd943d85



A note from the maintainer

2017-03-24 Thread Junio C Hamano
p of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.12 done on Feb 24th, 2017. You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  This branch is also
merged into "master" to propagate the fixes forward as needed.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "next" will be rebuilt from the tip of "master"
using the topics that didn't make the cut in the feature release.

A natural consequence of how "next" and "pu" bundles topics together
is that until a topic is merged to "next", updates to it is expected
by replacing the patch(es) in the topic with an improved version,
and once a topic is merged to "next", updates to it needs to come as
incremental patches, pointing out what was wron

A note from the maintainer

2017-03-20 Thread Junio C Hamano
p of 'master' can be
viewed online at:

  https://git.github.io/htmldocs/git.html


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting.  Every now and then, a
"feature release" is cut from the tip of this branch.  They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").

The last such release was 2.12 done on Feb 24th, 2017. You can expect
that the tip of the "master" branch is always more stable than any of
the released versions.

Whenever a feature release is made, "maint" branch is forked off from
"master" at that point.  Obvious and safe fixes after a feature
release are applied to this branch and maintenance releases are cut
from it.  Usually the fixes are merged to the "master" branch first,
several days before merged to the "maint" branch, to reduce the chance
of last-minute issues.  The maintenance releases used to be named with
four dotted decimal, named after the feature release they are updates
to (e.g. "1.8.5.1" was the first maintenance release for "1.8.5"
feature release).  These days, maintenance releases are named by
incrementing the last digit of three-dotted decimal name (e.g. "2.12.1"
was the first maintenance release for the "2.12" series).

New features never go to the 'maint' branch.  This branch is also
merged into "master" to propagate the fixes forward as needed.

A new development does not usually happen on "master". When you send a
series of patches, after review on the mailing list, a separate topic
branch is forked from the tip of "master" (or somewhere older, especially
when the topic is about fixing an earlier bug) and your patches are queued
there, and kept out of "master" while people test it out. The quality of
topic branches are judged primarily by the mailing list discussions.

Topic branches that are in good shape are merged to the "next" branch. In
general, the "next" branch always contains the tip of "master".  It might
not be quite rock-solid, but is expected to work more or less without major
breakage. The "next" branch is where new and exciting things take place. A
topic that is in "next" is expected to be polished to perfection before it
is merged to "master".  Please help this process by building & using the
"next" branch for your daily work, and reporting any new bugs you find to
the mailing list, before the breakage is merged down to the "master".

The "pu" (proposed updates) branch bundles all the remaining topic
branches the maintainer happens to have seen.  There is no guarantee that
the maintainer has enough bandwidth to pick up any and all topics that
are remotely promising from the list traffic, so please do not read
too much into a topic being on (or not on) the "pu" branch.  This
branch is mainly to remind the maintainer that the topics in them may
turn out to be interesting when they are polished, nothing more.  The
topics on this branch aren't usually complete, well tested, or well
documented and they often need further work.  When a topic that was
in "pu" proves to be in a testable shape, it is merged to "next".

You can run "git log --first-parent master..pu" to see what topics are
currently in flight.  Sometimes, an idea that looked promising turns out
to be not so good and the topic can be dropped from "pu" in such a case.
The output of the above "git log" talks about a "jch" branch, which is an
early part of the "pu" branch; that branch contains all topics that
are in "next" and a bit more (but not all of "pu") and is used by the
maintainer for his daily work.

The two branches "master" and "maint" are never rewound, and "next"
usually will not be either.  After a feature release is made from
"master", however, "next" will be rebuilt from the tip of "master"
using the topics that didn't make the cut in the feature release.

A natural consequence of how "next" and "pu" bundles topics together
is that until a topic is merged to "next", updates to it is expected
by replacing the patch(es) in the topic with an improved version,
and once a topic is merged to "next", updates to it needs to come as
incremental patches, pointing out what was wron

  1   2   3   4   >