[PATCH v3 0/1] doc: Add a note about ~/.zsh/_git file
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
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
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
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"
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
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"
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"
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Æ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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Æ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
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
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
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
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
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
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
Æ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
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
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
Æ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
Æ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
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
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..."
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
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
-- 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
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
Æ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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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_*
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
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
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