Documentation: update "man git-add" to include "[]"

2018-12-08 Thread Robert P. J. Day


Current "man git-add" emphasizes single letter interactive
shortcut commands with "[]".

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

---

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 45652fe4a6..ad9bd7c7a6 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -239,8 +239,8 @@ and type return, like this:

 
 *** Commands ***
-  1: status   2: update   3: revert   4: add untracked
-  5: patch6: diff 7: quit 8: help
+  1: [s]tatus 2: [u]pdate 3: [r]evert 4: [a]dd untracked
+  5: [p]atch  6: [d]iff   7: [q]uit   8: [h]elp
 What now> 1
 


-- 


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

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



Re: [PATCH] mailmap: update brandon williams's email address

2018-12-08 Thread Ævar Arnfjörð Bjarmason


On Sat, Dec 08 2018, Junio C Hamano wrote:

> Stefan Beller  writes:
>
>> On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder  wrote:
>>>
>>> Brandon Williams wrote:
>>>
>>> > Signed-off-by: Brandon Williams 
>>> > ---
>>> >  .mailmap | 1 +
>>> >  1 file changed, 1 insertion(+)
>>>
>>> I can confirm that this is indeed the same person.
>>
>> What would be more of interest is why we'd be interested in this patch
>> as there is no commit/patch sent by Brandon with this email in gits history.
>
> Once I "git am" the message that began this thread, there will be a
> commit under this new ident, so that would be somewhat a moot point.

"Get to the top of 'git shortlog -sn' with this one easy trick" :)

(The patch makes sense, good to see you back on-list Brandon)


Re: [PATCH] mailmap: update brandon williams's email address

2018-12-08 Thread Duy Nguyen
On Sat, Dec 8, 2018 at 7:09 AM Junio C Hamano  wrote:
> If this were "Jonathan asked Brandon if we want to record an address
> we can reach him in our .mailmap file and sent a patch to add one",

Not sure about Jonathan, but I did.

> then the story is different, and I tend to agree with you that such
> a patch is more or less pointless.  That's not the purpose of the
> mailmap file.

Not directly, but when multiple commands use mailmap to show the
canonical mail addresses, then it kinda is. When I look back at an old
commit message, I may look up the author name and address, which is
mapped.

> Not until git-send-email learns to use that file to rewrite
> To/cc/etc to the "canonical" addresses, anyway ;-)

git-send-email does not have to when I copy/paste the address from
git-log anyway.

> I am not sure if there are people whose "canonical" address to be
> used as the author is not necessarily the best address they want to
> get their e-mails at, though.  If we can be reasonably sure that the
> set of such people is empty, then people can take the above mention
> about send-email as a hint about a low-hanging fruit ;-)
>
> Thanks.
>
>


-- 
Duy


Re: [PATCH] mailmap: update brandon williams's email address

2018-12-07 Thread Brandon Williams
On Fri, Dec 7, 2018 at 10:08 PM Junio C Hamano  wrote:
>
> Stefan Beller  writes:
>
> > On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder  wrote:
> >>
> >> Brandon Williams wrote:
> >>
> >> > Signed-off-by: Brandon Williams 
> >> > ---
> >> >  .mailmap | 1 +
> >> >  1 file changed, 1 insertion(+)
> >>
> >> I can confirm that this is indeed the same person.
> >
> > What would be more of interest is why we'd be interested in this patch
> > as there is no commit/patch sent by Brandon with this email in gits history.
>
> Once I "git am" the message that began this thread, there will be a
> commit under this new ident, so that would be somewhat a moot point.
>
> If this were "Jonathan asked Brandon if we want to record an address
> we can reach him in our .mailmap file and sent a patch to add one",
> then the story is different, and I tend to agree with you that such
> a patch is more or less pointless.  That's not the purpose of the
> mailmap file.
>

Turns out this is exactly the reason :) I've had a couple of people
reach out to me asking me to do this because CCing my old email
bounces and they've wanted my input/comments on something related to
work I've done.  If that's not the intended purpose then please ignore
this patch

> Not until git-send-email learns to use that file to rewrite
> To/cc/etc to the "canonical" addresses, anyway ;-)
>
> I am not sure if there are people whose "canonical" address to be
> used as the author is not necessarily the best address they want to
> get their e-mails at, though.  If we can be reasonably sure that the
> set of such people is empty, then people can take the above mention
> about send-email as a hint about a low-hanging fruit ;-)
>
> Thanks.
>
>


Re: [PATCH] mailmap: update brandon williams's email address

2018-12-07 Thread Junio C Hamano
Stefan Beller  writes:

> On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder  wrote:
>>
>> Brandon Williams wrote:
>>
>> > Signed-off-by: Brandon Williams 
>> > ---
>> >  .mailmap | 1 +
>> >  1 file changed, 1 insertion(+)
>>
>> I can confirm that this is indeed the same person.
>
> What would be more of interest is why we'd be interested in this patch
> as there is no commit/patch sent by Brandon with this email in gits history.

Once I "git am" the message that began this thread, there will be a
commit under this new ident, so that would be somewhat a moot point.

If this were "Jonathan asked Brandon if we want to record an address
we can reach him in our .mailmap file and sent a patch to add one",
then the story is different, and I tend to agree with you that such
a patch is more or less pointless.  That's not the purpose of the
mailmap file.

Not until git-send-email learns to use that file to rewrite
To/cc/etc to the "canonical" addresses, anyway ;-)

I am not sure if there are people whose "canonical" address to be
used as the author is not necessarily the best address they want to
get their e-mails at, though.  If we can be reasonably sure that the
set of such people is empty, then people can take the above mention
about send-email as a hint about a low-hanging fruit ;-)

Thanks.




Re: [wishlist] git submodule update --reset-hard

2018-12-07 Thread Yaroslav Halchenko

On Fri, 07 Dec 2018, Yaroslav Halchenko wrote:


> On Fri, 07 Dec 2018, Stefan Beller wrote:
> > > the initial "git submodule update --reset-hard" is pretty much a
> > > crude workaround for some of those cases, so I would just go earlier in
> > > the history, and redo some things, whenever I could just drop or revert
> > > some selected set of commits.

> > That makes sense.
> > Do you want to give the implementation a try for the --reset-hard switch?

> ok, will do, thanks for the blessing ;-)

The patch is attached (please advise if should be done
differently) and also submitted as PR
https://github.com/git/git/pull/563

I guess it would need more tests.  Took me some time to figure out
why I was getting

fatal: bad value for update parameter

after all my changes to the git-submodule.sh script after looking at an
example commit 42b491786260eb17d97ea9fb1c4b70075bca9523 which introduced
--merge to the update ;-)

-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
From 170296dc661b4bc3d942917ce27288df52ff650d Mon Sep 17 00:00:00 2001
From: Yaroslav Halchenko 
Date: Fri, 7 Dec 2018 21:28:29 -0500
Subject: [PATCH] submodule: Add --reset-hard option for git submodule update

This patch adds a --reset-hard option for the update command to hard
reset submodule(s) to the gitlink for the corresponding submodule in
the superproject.  This feature is desired e.g. to be able to discard
recent changes in the entire hierarchy of the submodules after running

   git reset --hard PREVIOUS_STATE

in the superproject which leaves submodules in their original state,
and

   git reset --hard --recurse-submodules PREVIOUS_STATE

would result in the submodules being checked into detached HEADs.

As in the original  git reset --hard  no checks or any kind of
safe-guards against jumping into some state which was never on the
current branch is done.

must_die_on_failure is not set to  yes to mimic behavior of a update
--checkout strategy, which would leave user with a non-clean state
immediately apparent via  git status  so an informed decision/actions
could be done manually.

Signed-off-by: Yaroslav Halchenko 
---
 Documentation/git-submodule.txt | 12 +++-
 Documentation/gitmodules.txt|  4 ++--
 builtin/submodule--helper.c |  3 ++-
 git-submodule.sh| 10 +-
 submodule.c |  4 
 submodule.h |  1 +
 t/t7406-submodule-update.sh | 17 -
 7 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..f90a42d265 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -124,7 +124,7 @@ If you really want to remove a submodule from the repository and commit
 that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal
 options.
 
-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::
+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge|--reset-hard] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::
 +
 --
 Update the registered submodules to match what the superproject
@@ -358,6 +358,16 @@ the submodule itself.
 	If the key `submodule.$name.update` is set to `rebase`, this option is
 	implicit.
 
+--reset-hard::
+	This option is only valid for the update command.
+	Hard reset current state to the commit recorded in the	superproject.
+If this option is given, the submodule's HEAD will not get detached
+if it was not detached before. Note that, like with a regular
+git reset --hard  no safe-guards are in place to prevent jumping
+to a commit which was never part of the current branch.
+	If the key `submodule.$name.update` is set to `reset-hard`, this
+	option is implicit.
+
 --init::
 	This option is only valid for the update command.
 	Initialize all submodules for which "git submodule init" has not been
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 312b6f9259..e085dbc01f 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -43,8 +43,8 @@ submodule..update::
 	command in the superproject. This is only used by `git
 	submodule init` to initialize the configuration variable of
 	the same name. Allowed values here are 'checkout', 'rebase',
-	'merge' or 'none'. See description of 'update' command in
-	linkgit:git-submodule[1] for their meaning. Note that the
+	'merge', 'reset-hard' or 'none'. See description of 'update' command
+	in l

Re: [wishlist] git submodule update --reset-hard

2018-12-07 Thread Yaroslav Halchenko


On Fri, 07 Dec 2018, Stefan Beller wrote:
> > the initial "git submodule update --reset-hard" is pretty much a
> > crude workaround for some of those cases, so I would just go earlier in
> > the history, and redo some things, whenever I could just drop or revert
> > some selected set of commits.

> That makes sense.
> Do you want to give the implementation a try for the --reset-hard switch?

ok, will do, thanks for the blessing ;-)

-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik


[PATCH 1/4] submodule update: add regression test with old style setups

2018-12-07 Thread Stefan Beller
As f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'",
2018-09-07) was produced shortly before a release, nobody asked for
a regression test to be included. Add a regression test that makes sure
that the invocation of `git submodule update` on old setups doesn't
produce errors as pointed out in f178c13fda.

The place to add such a regression test may look odd in t7412, but
that is the best place as there we setup old style submodule setups
explicitly.

Signed-off-by: Stefan Beller 
---
 t/t7412-submodule-absorbgitdirs.sh | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/t7412-submodule-absorbgitdirs.sh 
b/t/t7412-submodule-absorbgitdirs.sh
index ce74c12da2..1cfa150768 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' '
GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
core.worktree "../../../nested" &&
# make sure this re-setup is correct
-   git status --ignore-submodules=none
+   git status --ignore-submodules=none &&
+
+   # also make sure this old setup does not regress
+   git submodule update --init --recursive >out 2>err &&
+   test_must_be_empty out &&
+   test_must_be_empty err
 '
 
 test_expect_success 'absorb the git dir in a nested submodule' '
-- 
2.20.0.rc2.403.gdbc3b29805-goog



Re: [PATCH] mailmap: update brandon williams's email address

2018-12-07 Thread Jonathan Nieder
Hi,

Stefan Beller wrote:

> What would be more of interest is why we'd be interested in this patch
> as there is no commit/patch sent by Brandon with this email in gits history.

I think there's an implicit assumption in this question that isn't
spelled out.  Do I understand correctly that you're saying the main
purpose of .mailmap is to figure out whether two commits are by the
same author?

My own uses of .mailmap primarily have a different purpose: to find
out the preferred contact address for the author of a given commit.

Thanks,
Jonathan


Re: [PATCH] mailmap: update brandon williams's email address

2018-12-07 Thread Stefan Beller
On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder  wrote:
>
> Brandon Williams wrote:
>
> > Signed-off-by: Brandon Williams 
> > ---
> >  .mailmap | 1 +
> >  1 file changed, 1 insertion(+)
>
> I can confirm that this is indeed the same person.

What would be more of interest is why we'd be interested in this patch
as there is no commit/patch sent by Brandon with this email in gits history.

Is that so you get cc'd on your private address and can follow
things you worked on without being subscribed to the mailing list?
(I'd be interested to see the use case in the commit message;)

Thanks,
Stefan


Re: [wishlist] git submodule update --reset-hard

2018-12-07 Thread Stefan Beller
On Thu, Dec 6, 2018 at 5:23 PM Yaroslav Halchenko  wrote:

> > There was a proposal to "re-attach HEAD" in the submodule, i.e.
> > if the branch branch points at the same commit, we don't need
> > a detached HEAD, but could go with the branch instead.
>
> if I got the idea right, if we are talking about any branch, it
> would also non-deterministic since who knows what left over branch(es)
> point to that commit.  Not sure if I would have used that ;)

I would think we'd rather want to have it deterministic, i.e. something like
1) record branch name of the submodule
2) update submodules HEAD to to superprojects gitlink
3) if recorded branch (1) matches the sha1 of detached HEAD,
  have HEAD point to the branch instead.

You notice a small inefficiency here as we write HEAD twice, so it
could be reworded as:
1) compare superprojects gitlink with the submodules branch
2a) if equal, set submodules HEAD to branch
2b) if unequal set HEAD to gitlink value, resulting in detached HEAD

Note that this idea of reattaching reflects the idea (a) below.


> > a) "stay on submodule branch (i.e. HEAD still points at $branch), and
> > reset --hard" such that the submodule has a clean index and at that $branch
> > or
> > b) "stay on submodule branch (i.e. HEAD still points at $branch), but 
> > $branch is
> >set to the gitlink from the superproject, and then a reset --hard
> >will have the worktree set to it as well.


> NB "gitlink" -- just now discovered the thing for me.  Thought it would be
> called a  subproject  echoing what git diff/log -p shows for submodule 
> commits.

The terminology is messy:
The internal representation in Gits object model is a "gitlink" entry in a tree
object. Once we have a .gitmodules entry, we call it submodule.

The term 'subproject' is a historic artifact and will likely not be changed
in the diff output (or format-patch), because these diffs can be applied using
git-am for example. That makes the diff output effectively a transport
protocol, and changing protocols is hard if you have no versioning in them.

More in https://git-scm.com/docs/gitsubmodules (a rather recent new write
of a man page, going into concepts).

> > > right -- I meant the local changes and indeed reset --recurse-submodules
> > > indeed seems to recurse nicely.  Then the undesired effect remaining only
> > > the detached HEAD
>
> > For that we may want to revive discussions in
> > https://public-inbox.org/git/20170501180058.8063-5-sbel...@google.com/
>
> well, isn't that one requires a branch to be specified in .gitmodules?

Ah good point.

> >   git reset --hard --recursive=hard,keep-branch PREVIOUSPOINT
>
> 'keep-branch' (given aforementioned keeping the specified in .gitmodules
> branch) might be confusing.  Also what if a submodule already in a
> detached HEAD?  IMHO --recursive=hard, and just saying that it would do
> "reset --hard", is imho sufficient.  (that is why I like pure
> --reset hard   since it doesn't care and neither does anything to the
> branch)

For that we might want to first do the

  git submodule update --reset-hard

which runs reset --hard inside the submodule, no matter which
branch the submodule is on (if any) and resets to the given
superproject sha1.

See git-submodule.sh in git.git[1] in cmd_update.
We'd need to add a command line flag (`--reset-hard`
would be the obvious choice?) which would set the `update`
variable, which then is evaluated to what needs to be done in
the submodule, which in that case would be the hard reset.

https://github.com/git/git/blob/master/git-submodule.sh#L606

Once that is done we'd want to add a test case, presumably
in t/t7406-submodule-update.sh

> > > I would have asked for
>
> > >git revert --recursive ...
> > >git rebase --recursive [-i] ...
>
> > > which I also frequently desire (could elaborate on the use cases etc).
>
> > These would be nice to have. It would be nice if you'd elaborate on the
> > use cases for future reference in the mailing list archive. :-)
>
> ok, will try to do so ;-) In summary: they are just a logical extension
> of git support for submodules for anyone actively working with
> submodules to keep entire tree in sync.  Then quite often the need for
> reverting a specific commit (which also has changes reflected in
> submodules) arises.  The same with rebase, especially to trim away some
> no longer desired changes reflected in submodules.
>
> the initial "git submodule update --reset-hard" is pretty much a
> crude workaround for some of those cases, so I would just go earlier in
> the history, and redo some things, whenever I could just drop or revert
> some selected set of commits.

That makes sens

Re: [PATCH] mailmap: update brandon williams's email address

2018-12-07 Thread Jonathan Nieder
Brandon Williams wrote:

> Signed-off-by: Brandon Williams 
> ---
>  .mailmap | 1 +
>  1 file changed, 1 insertion(+)

I can confirm that this is indeed the same person.

Reviewed-by: Jonathan Nieder 

Welcome back!


[PATCH] mailmap: update brandon williams's email address

2018-12-07 Thread Brandon Williams
Signed-off-by: Brandon Williams 
---
 .mailmap | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index eb7b5fc7b..247a3deb7 100644
--- a/.mailmap
+++ b/.mailmap
@@ -27,6 +27,7 @@ Ben Walton  
 Benoit Sigoure  
 Bernt Hansen  
 Brandon Casey  
+Brandon Williams  
 brian m. carlson 
 brian m. carlson  

 Bryan Larsen  
-- 
2.19.1



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

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

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


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

2018-12-07 Thread Johannes Sixt

From: Elijah Newren 

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

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

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

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

 include::merge-strategies.txt[]

--
2.19.1.1133.g2dd3d172d2


Re: [wishlist] git submodule update --reset-hard

2018-12-06 Thread Yaroslav Halchenko
Hi Stefan,

Thanks for the dialogue!  Replies are embedded below.

On Thu, 06 Dec 2018, Stefan Beller wrote:
> ...
> > > What if the branch differs from the sha1 recorded in the superproject?

> > git reset --hard  itself is an operation which should be done with some
> > level of competence in doing "the right thing" by calling it.  You
> > can hop branches even in current (without any submodules in question)
> > repository with it and cause as much chaos as you desire.

> Right.

> git reset --hard would the branch (as well as the working tree) to the
> given sha1, which is confusing as submodules get involved.

> The Right Thing as of now is the sha1 as found in the
> superprojects gitlink. But as that can be different from any branch
> in the submodule, we'd rather detach the HEAD to make it
> deterministic.

yeap, makes total sense to be the thing do that by default ;-)

> There was a proposal to "re-attach HEAD" in the submodule, i.e.
> if the branch branch points at the same commit, we don't need
> a detached HEAD, but could go with the branch instead.

if I got the idea right, if we are talking about any branch, it
would also non-deterministic since who knows what left over branch(es)
point to that commit.  Not sure if I would have used that ;)

> > If desired though, a number of prevention mechanisms could be in place (but
> > would require option(s) to overcome) to allow submodule to be reset 
> > --hard'ed
> > only when some conditions met (e.g. only to the commit which is among parent
> > commits path of the current branch).  This way wild hops would be prevented,
> > although you might still end up in some feature branch.  But since "reset
> > --hard" itself doesn't have any safe-guards, I do not really think they 
> > should
> > be implemented here either.

> So are you looking for
> a) "stay on submodule branch (i.e. HEAD still points at $branch), and
> reset --hard" such that the submodule has a clean index and at that $branch 
> or
> b) "stay on submodule branch (i.e. HEAD still points at $branch), but $branch 
> is
>set to the gitlink from the superproject, and then a reset --hard
>will have the worktree set to it as well.

yes!

NB "gitlink" -- just now discovered the thing for me.  Thought it would be
called a  subproject  echoing what git diff/log -p shows for submodule commits.

> (a) is what the referenced submodule.repoLike option implements.

sounds like it indeed, thanks for spelling out

> I'd understand the desire for (b) as well, as it is a "real" hard reset on
> the superproject level, without detaching branches.

yeap

> > >   git reset --hard --recurse-submodules HEAD

> > it does indeed some trick(s) but not all seems to be the ones I desire:

> > 1. Seems to migrate submodule's .git directories into the top level
> > .git/modules

> Ah yes, that happens too. This will help once you want to git-rm
> a submodule and checkout states before and after.

yeap ;-) 

> > > undesirable in the sense of still having local changes (that is what
> > > the above reset with `--recurse` would fix) or changed the branch
> > > state? (i.e. is detached but was on a branch before?)

> > right -- I meant the local changes and indeed reset --recurse-submodules
> > indeed seems to recurse nicely.  Then the undesired effect remaining only
> > the detached HEAD

> For that we may want to revive discussions in
> https://public-inbox.org/git/20170501180058.8063-5-sbel...@google.com/

well, isn't that one requires a branch to be specified in .gitmodules?

> > > >   git submodule update --recursive

> > > > I would end up in the detached HEADs within submodules.

> > > > What I want is to retain current branch they are at (or may be possible
> > > > "were in"? reflog records might have that information)

> > > So something like

> > >   git submodule foreach --recursive git reset --hard

> > > ?

> > not quite  -- this would just kill all local changes within each submodule, 
> > not
> > to reset it to the desired state, which wouldn't be specified in such
> > invocation, and is only known to the repo containing it

> With this answer it sounds like you'd want (b) from above.

yeap

> > > You may be interested in
> > > https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/
> > > which introduces a switch `submodule.repoLike [ = true]`, which
> > > when set would not detach HEAD in submodules.

> > Thanks! looks interesting -- was there more discussion/activity beyond 
> > those 5
> > posts in the thread?

> 

Re: [wishlist] git submodule update --reset-hard

2018-12-06 Thread Stefan Beller
On Thu, Dec 6, 2018 at 1:25 PM Yaroslav Halchenko  wrote:
>
>
> On Thu, 06 Dec 2018, Stefan Beller wrote:
>
> > On Thu, Dec 6, 2018 at 10:02 AM Yaroslav Halchenko  
> > wrote:
>
> > > Dear Git Gurus,
>
> > > I wondered what would be your take on my wishlist request to add
> > > --reset-hard option, which would be very similar to regular "update" which
> > > checks out necessary commit, but I want it to remain in the branch.
>
> > What if the branch differs from the sha1 recorded in the superproject?
>
> git reset --hard  itself is an operation which should be done with some
> level of competence in doing "the right thing" by calling it.  You
> can hop branches even in current (without any submodules in question)
> repository with it and cause as much chaos as you desire.

Right.

git reset --hard would the branch (as well as the working tree) to the
given sha1, which is confusing as submodules get involved.

The Right Thing as of now is the sha1 as found in the
superprojects gitlink. But as that can be different from any branch
in the submodule, we'd rather detach the HEAD to make it
deterministic.

There was a proposal to "re-attach HEAD" in the submodule, i.e.
if the branch branch points at the same commit, we don't need
a detached HEAD, but could go with the branch instead.

> If desired though, a number of prevention mechanisms could be in place (but
> would require option(s) to overcome) to allow submodule to be reset --hard'ed
> only when some conditions met (e.g. only to the commit which is among parent
> commits path of the current branch).  This way wild hops would be prevented,
> although you might still end up in some feature branch.  But since "reset
> --hard" itself doesn't have any safe-guards, I do not really think they should
> be implemented here either.

So are you looking for
a) "stay on submodule branch (i.e. HEAD still points at $branch), and
reset --hard"
such that the submodule has a clean index and at that $branch or
b) "stay on submodule branch (i.e. HEAD still points at $branch), but $branch is
   set to the gitlink from the superproject, and then a reset --hard
will have the worktree
   set to it as well.

(a) is what the referenced submodule.repoLike option implements.

I'd understand the desire for (b) as well, as it is a "real" hard reset on
the superproject level, without detaching branches.

> >   git reset --hard --recurse-submodules HEAD

> it does indeed some trick(s) but not all seems to be the ones I desire:
>
> 1. Seems to migrate submodule's .git directories into the top level
> .git/modules

Ah yes, that happens too. This will help once you want to git-rm
a submodule and checkout states before and after.

> > undesirable in the sense of still having local changes (that is what
> > the above reset with `--recurse` would fix) or changed the branch
> > state? (i.e. is detached but was on a branch before?)
>
> right -- I meant the local changes and indeed reset --recurse-submodules
> indeed seems to recurse nicely.  Then the undesired effect remaining only
> the detached HEAD

For that we may want to revive discussions in
https://public-inbox.org/git/20170501180058.8063-5-sbel...@google.com/


> > >   git submodule update --recursive
>
> > > I would end up in the detached HEADs within submodules.
>
> > > What I want is to retain current branch they are at (or may be possible
> > > "were in"? reflog records might have that information)
>
> > So something like
>
> >   git submodule foreach --recursive git reset --hard
>
> > ?
>
> not quite  -- this would just kill all local changes within each submodule, 
> not
> to reset it to the desired state, which wouldn't be specified in such
> invocation, and is only known to the repo containing it

With this answer it sounds like you'd want (b) from above.

> > You may be interested in
> > https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/
> > which introduces a switch `submodule.repoLike [ = true]`, which
> > when set would not detach HEAD in submodules.
>
> Thanks! looks interesting -- was there more discussion/activity beyond those 5
> posts in the thread?

Unfortunately there was not.

> This feature might indeed come handy but if I got it right, it is somewhat
> complimentary to just having submodule update --reset-hard .  E.g.  submodules
> might be in different branches (if I am not tracking based on branch names), 
> so
> I would not want a recursive checkout with -b|-B.  But we would indeed benefit
> from such functionality, since this difficulty of managing branches of
> submodules I think would be elevated with it! (e.g. in one

Re: [wishlist] git submodule update --reset-hard

2018-12-06 Thread Yaroslav Halchenko


On Thu, 06 Dec 2018, Stefan Beller wrote:

> On Thu, Dec 6, 2018 at 10:02 AM Yaroslav Halchenko  
> wrote:

> > Dear Git Gurus,

> > I wondered what would be your take on my wishlist request to add
> > --reset-hard option, which would be very similar to regular "update" which
> > checks out necessary commit, but I want it to remain in the branch.

> What if the branch differs from the sha1 recorded in the superproject?

git reset --hard  itself is an operation which should be done with some
level of competence in doing "the right thing" by calling it.  You
can hop branches even in current (without any submodules in question)
repository with it and cause as much chaos as you desire.

If desired though, a number of prevention mechanisms could be in place (but
would require option(s) to overcome) to allow submodule to be reset --hard'ed
only when some conditions met (e.g. only to the commit which is among parent
commits path of the current branch).  This way wild hops would be prevented,
although you might still end up in some feature branch.  But since "reset
--hard" itself doesn't have any safe-guards, I do not really think they should
be implemented here either.

> > Rationale: In DataLad we heavily rely on submodules, and we have established
> > easy ways to do some manipulations across full hierarchies of them. E.g. a
> > single command could introduce a good number of commits across deep 
> > hierarchy
> > of submodules, e.g. while committing changes within deep submodule, while 
> > also
> > doing all necessary commits in the repositories leading to that submodule so
> > the entire tree of them stays in a "clean" state. The difficulty comes when
> > there is a need to just "forget" some changes.  The obvious way is to e.g.

> >git reset --hard PREVIOUS_STATE

>   git reset --hard --recurse-submodules HEAD

> would do the trick

it does indeed some trick(s) but not all seems to be the ones I desire:

1. Seems to migrate submodule's .git directories into the top level
.git/modules

$>  git reset --hard --recurse-submodules HEAD^^^
Migrating git directory of 'famface' from
'/tmp/gobbini/famface/.git' to
'/tmp/gobbini/.git/modules/famface'
Migrating git directory of 'famface/data' from
'/tmp/gobbini/famface/data/.git' to
'/tmp/gobbini/.git/modules/famface/modules/data'
Migrating git directory of 'famface/data/scripts/mridefacer' from
'/tmp/gobbini/famface/data/scripts/mridefacer/.git' to

'/tmp/gobbini/.git/modules/famface/modules/data/modules/scripts/mridefacer'
HEAD is now at 9b4296d [DATALAD] aggregated meta data

we might eventually adopt this default already for years model (git annex seems
to be ok, in that it then replaces .git symlink file with the actual
symlink .git -> ../../.git/modules/...  So things seems to keep working
for annex)

2. It still does the detached HEAD for me

$> git submodule status --recursive  
 2569ab436501a832d35afbbe9cc20ffeb6077eb1 famface (2569ab4)
 f1e8c9b8b025c311424283b9711efc6bc906ba2b famface/data (BIDS-v1.0.1)
 49b0fe42696724c2a8492f999736056e51b77358 
famface/data/scripts/mridefacer (49b0fe4)


> > in the top level repository.  But that leaves all the submodules now in
> > the undesired state.  If I do

> undesirable in the sense of still having local changes (that is what
> the above reset with `--recurse` would fix) or changed the branch
> state? (i.e. is detached but was on a branch before?)

right -- I meant the local changes and indeed reset --recurse-submodules
indeed seems to recurse nicely.  Then the undesired effect remaining only
the detached HEAD

> >   git submodule update --recursive

> > I would end up in the detached HEADs within submodules.

> > What I want is to retain current branch they are at (or may be possible
> > "were in"? reflog records might have that information)

> So something like

>   git submodule foreach --recursive git reset --hard

> ?

not quite  -- this would just kill all local changes within each submodule, not
to reset it to the desired state, which wouldn't be specified in such
invocation, and is only known to the repo containing it

> You may be interested in
> https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/
> which introduces a switch `submodule.repoLike [ = true]`, which
> when set would not detach HEAD in submodules.

Thanks! looks interesting -- was there more discussion/activity beyond those 5
posts in the thread?
https://public-inbox.org/git/87h8i9ift4@evledraar.gmail.com/#r 

This feature might indeed come handy but if I got it right, it is somewhat
complimentary to just having submodule update --reset-hard .  

Re: [wishlist] git submodule update --reset-hard

2018-12-06 Thread Stefan Beller
On Thu, Dec 6, 2018 at 10:02 AM Yaroslav Halchenko  wrote:
>
> Dear Git Gurus,
>
> I wondered what would be your take on my wishlist request to add
> --reset-hard option, which would be very similar to regular "update" which
> checks out necessary commit, but I want it to remain in the branch.

What if the branch differs from the sha1 recorded in the superproject?

> Rationale: In DataLad we heavily rely on submodules, and we have established
> easy ways to do some manipulations across full hierarchies of them. E.g. a
> single command could introduce a good number of commits across deep hierarchy
> of submodules, e.g. while committing changes within deep submodule, while also
> doing all necessary commits in the repositories leading to that submodule so
> the entire tree of them stays in a "clean" state. The difficulty comes when
> there is a need to just "forget" some changes.  The obvious way is to e.g.
>
>git reset --hard PREVIOUS_STATE

  git reset --hard --recurse-submodules HEAD

would do the trick

> in the top level repository.  But that leaves all the submodules now in
> the undesired state.  If I do

undesirable in the sense of still having local changes (that is what
the above reset with `--recurse` would fix) or changed the branch
state? (i.e. is detached but was on a branch before?)

>   git submodule update --recursive
>
> I would end up in the detached HEADs within submodules.
>
> What I want is to retain current branch they are at (or may be possible
> "were in"? reflog records might have that information)

So something like

  git submodule foreach --recursive git reset --hard

?

You may be interested in
https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/
which introduces a switch `submodule.repoLike [ = true]`, which
when set would not detach HEAD in submodules.

Can you say more about the first question above:
Would you typically have situations where the
submodule branch is out of sync with the superproject
and how do you deal with that?

Adding another mode to `git submodule update` sounds
reasonable to me, too.

Stefan


[wishlist] git submodule update --reset-hard

2018-12-06 Thread Yaroslav Halchenko
Dear Git Gurus,

I wondered what would be your take on my wishlist request to add
--reset-hard option, which would be very similar to regular "update" which
checks out necessary commit, but I want it to remain in the branch.

Rationale: In DataLad we heavily rely on submodules, and we have established
easy ways to do some manipulations across full hierarchies of them. E.g. a
single command could introduce a good number of commits across deep hierarchy
of submodules, e.g. while committing changes within deep submodule, while also
doing all necessary commits in the repositories leading to that submodule so
the entire tree of them stays in a "clean" state. The difficulty comes when
there is a need to just "forget" some changes.  The obvious way is to e.g. 

   git reset --hard PREVIOUS_STATE

in the top level repository.  But that leaves all the submodules now in
the undesired state.  If I do

  git submodule update --recursive

I would end up in the detached HEADs within submodules.  

What I want is to retain current branch they are at (or may be possible
"were in"? reflog records might have that information)

Example:

# Have to use datalad install  since  git clone --recurse-submodules
# seems to not consider alternative locations for submodules' .git/
# with url being just a relative path, and where submodules aren't 
# all residing up under toplevel URL .git/

$> datalad install -r http://datasets.datalad.org/labs/gobbini/.git
[INFO   ] Cloning http://datasets.datalad.org/labs/gobbini/.git into 
'/tmp/gobbini' 
install(ok): /tmp/gobbini (dataset) 

[INFO   ] Installing  recursively 
[INFO   ] Cloning http://datasets.datalad.org/labs/gobbini/famface/.git 
into '/tmp/gobbini/famface' 
[INFO   ] Cloning 
http://datasets.datalad.org/labs/gobbini/famface/data/.git into 
'/tmp/gobbini/famface/data'   
[INFO   ] access to dataset sibling "datasets.datalad.org" not 
auto-enabled, enable with:   
|   datalad siblings -d "/tmp/gobbini/famface/data" enable 
-s datasets.datalad.org 
[INFO   ] Cloning 
http://datasets.datalad.org/labs/gobbini/famface/data/scripts/mridefacer/.git 
[2 other candidates] into '/tmp/gobbini/famface/data/scripts/mridefacer' 
action summary: 

  install (ok: 4)

so I have a hierarchy in a good state and all checked out in master
branch

$> cd gobbini

$> git submodule status --recursive   
 b9071a6bc9f7665f7c75549c63d29f16d40e8af7 famface (heads/master)
 e59ba76b42f219bdf14b6b547dd6d9cc0ed5227f famface/data 
(BIDS-v1.0.1-3-ge59ba76b)
 5d8036c0aaeebb448a00df6296ddc9f799efdd1f 
famface/data/scripts/mridefacer (heads/master)

$> git submodule foreach --recursive cat .git/HEAD 
Entering 'famface'
ref: refs/heads/master
Entering 'famface/data'
ref: refs/heads/master
Entering 'famface/data/scripts/mridefacer'
ref: refs/heads/master


and if I do roll back

$> git reset --hard HEAD^^^
HEAD is now at 9b4296d [DATALAD] aggregated meta data
changes on filesystem:  
        
     famface | 2 +-

and default update --recursive

$> git submodule update --recursive
Submodule path 'famface': checked out 
'2569ab436501a832d35afbbe9cc20ffeb6077eb1'
Submodule path 'famface/data': checked out 
'f1e8c9b8b025c311424283b9711efc6bc906ba2b'
Submodule path 'famface/data/scripts/mridefacer': checked out 
'49b0fe42696724c2a8492f999736056e51b77358'

I end up in detached HEADs

$> git submodule status --recursive 
 2569ab436501a832d35afbbe9cc20ffeb6077eb1 famface (2569ab4)
 f1e8c9b8b025c311424283b9711efc6bc906ba2b famface/data (BIDS-v1.0.1)
 49b0fe42696724c2a8492f999736056e51b77358 
famface/data/scripts/mridefacer (49b0fe4)


I do see that there is a "custom command" way to do it via
"submodule..update" config setting, but that is not easy to use for my
case since all the `` would be different to specify !git reset --hard for
all of them via config option and I could not find any way to "glob" config
(like submodule.*.update).  But in effect that is probably what I need:

    # restarting from a clean state here
$> git -c submodule.famface.update='!git reset --hard' submodule update 
--recursive
HEAD is now at 2569ab4 [DATALAD] aggregated meta data
Submodule path 'famface': 'git reset --hard 
2569ab436501a832d35afbbe9cc20ffeb6077eb1'
Submodule path 'famface/data': checked out

Re: [PATCH v2] l10n: update German translation

2018-12-06 Thread phillip

Hi,

thanks for your great work! Just two remarks:


  #: midx.c:407
-#, fuzzy, c-format
+#, c-format
  msgid "failed to add packfile '%s'"
-msgstr "Fehler beim Lesen der Reihenfolgedatei '%s'."
+msgstr "Fehler beim Hinzufügen von Packdatei'%s'."


A Space is missing: "Fehler beim Hinzufügen von Packdatei '%s'."


  #: run-command.c:1229
-#, fuzzy, c-format
+#, c-format
  msgid "cannot create async thread: %s"
-msgstr "kann Thread nicht erzeugen: %s"
+msgstr "Kann Thread für async nicht erzeugen: %s"


I think we should use "Konnte" here.


Best regards,

Phillip



[PATCH v2] l10n: update German translation

2018-12-03 Thread Ralf Thielow
Signed-off-by: Ralf Thielow 
---
v2 updates the translation up to the latest update of git.pot.

range-diff:
1:  f0a6c76bf ! 1:  f8313495e l10n: update German translation
@@ -205,13 +205,13 @@
 -msgstr ""
 +msgstr "Falsche Reihenfolge bei multi-pack-index Pack-Namen: '%s' vor 
'%s'"
  
  #: midx.c:205
  #, c-format
- msgid "bad pack-int-id: %u (%u total packs"
+ msgid "bad pack-int-id: %u (%u total packs)"
 -msgstr ""
-+msgstr "Fehlerhafte pack-int-id: %u (%u Pakete insgesamt)"
++msgstr "Ungültige pack-int-id: %u (%u Pakete insgesamt)"
  
  #: midx.c:246
  msgid "multi-pack-index stores a 64-bit offset, but off_t is too small"
 -msgstr ""
 +msgstr "multi-pack-index speichert einen 64-Bit Offset, aber off_t ist zu 
klein."
@@ -364,31 +364,31 @@
 +#, c-format
  msgid "unable to join load_cache_entries thread: %s"
 -msgstr "kann Thread nicht erzeugen: %s"
 +msgstr "Kann Thread für load_cache_entries nicht erzeugen: %s"
  
- #: read-cache.c:2200
+ #: read-cache.c:2201
 -#, fuzzy, c-format
 +#, c-format
  msgid "unable to create load_index_extensions thread: %s"
 -msgstr "kann Thread nicht erzeugen: %s"
 +msgstr "Kann Thread für load_index_extensions nicht erzeugen: %s"
  
- #: read-cache.c:2227
+ #: read-cache.c:2228
 -#, fuzzy, c-format
 +#, c-format
  msgid "unable to join load_index_extensions thread: %s"
 -msgstr "kann Thread nicht erzeugen: %s"
-+msgstr "Kann Thread für load_index_extensions nicht erzeugen: %s"
++msgstr "Kann Thread für load_index_extensions nicht beitreten: %s"
  
- #: read-cache.c:2953 sequencer.c:4727 wrapper.c:658 builtin/merge.c:1086
+ #: read-cache.c:2982 sequencer.c:4727 wrapper.c:658 builtin/merge.c:1086
  #, c-format
  msgid "could not close '%s'"
 -msgstr "Konnte '%s' nicht schließen"
 +msgstr "Konnte '%s' nicht schließen."
  
- #: read-cache.c:3026 sequencer.c:2203 sequencer.c:3592
+ #: read-cache.c:3055 sequencer.c:2203 sequencer.c:3592
  #, c-format
 @@
  msgstr "Konnte '%s' nicht entfernen."
  
  #: rebase-interactive.c:10
@@ -802,14 +802,19 @@
  
  #: builtin/grep.c:1051
 -#, fuzzy
  msgid "invalid option combination, ignoring --threads"
 -msgstr "keine Unterstützung von Threads, --threads wird ignoriert"
-+msgstr "ungültige Kombination von Optionen, --threads wird ignoriert"
++msgstr "Ungültige Kombination von Optionen, --threads wird ignoriert."
  
- #: builtin/grep.c:1054 builtin/pack-objects.c:3395
+ #: builtin/grep.c:1054 builtin/pack-objects.c:3397
  msgid "no threads support, ignoring --threads"
+-msgstr "keine Unterstützung von Threads, --threads wird ignoriert"
++msgstr "Keine Unterstützung für Threads, --threads wird ignoriert."
+ 
+ #: builtin/grep.c:1057 builtin/index-pack.c:1503 
builtin/pack-objects.c:2716
+ #, c-format
 @@
  msgstr "Für '%s' wurde der Alias '%s' angelegt."
  
  #: builtin/help.c:444
 -#, fuzzy, c-format
@@ -944,17 +949,17 @@
  #: builtin/pack-objects.c:2123
  msgid "suboptimal pack - out of memory"
 @@
  "packen"
  
- #: builtin/pack-objects.c:3316
+ #: builtin/pack-objects.c:3318
 -#, fuzzy
  msgid "respect islands during delta compression"
 -msgstr "Größe des Fensters für die Delta-Kompression"
 +msgstr "Delta-Islands bei Delta-Kompression beachten"
  
- #: builtin/pack-objects.c:3340
+ #: builtin/pack-objects.c:3342
  #, c-format
 @@
  "wurde nicht angefordert."
  
  #: builtin/pull.c:565
@@ -964,10 +969,28 @@
 -msgstr "Konnte Commit '%s' nicht parsen."
 +msgstr "Konnte nicht auf Commit '%s' zugreifen."
  
  #: builtin/pull.c:843
  msgid "ignoring --verify-signatures for rebase"
+@@
+ "config'."
+ 
+ #: builtin/push.c:168
+-#, fuzzy, c-format
++#, c-format
+ msgid ""
+ "The upstream branch of your current branch does not match\n"
+ "the name of your current branch.  To push to the upstream branch\n"
+@@
+ "Um auf den Branch mit demselben Namen im Remote-Repository zu 
versenden,\n"
+ "benutzen Sie:\n"
+ "\n"
+-"git push %s %s\n"
++"git push %s HEAD\n"
+ "%s"
+ 
+ #: builtin/push.c:183
 @@
  msgstr "unpack-trees protokollieren"
  
  #:

[PATCH] l10n: update German translation

2018-11-30 Thread Ralf Thielow
erden; Sie werden von oben nach unten\n"
 "ausgeführt.\n"
 
 #: rebase-interactive.c:31 git-rebase--preserve-merges.sh:173
 msgid ""
@@ -4182,17 +4174,17 @@ msgstr "Fehlerhafter Feldname: %.*s"
 #, c-format
 msgid "unknown field name: %.*s"
 msgstr "Unbekannter Feldname: %.*s"
 
 #: ref-filter.c:539
 #, c-format
 msgid ""
 "not a git repository, but the field '%.*s' requires access to object data"
-msgstr ""
+msgstr "Kein Git-Repository, aber das Feld '%.*s' erfordert Zugriff auf 
Objektdaten."
 
 #: ref-filter.c:663
 #, c-format
 msgid "format: %%(if) atom used without a %%(then) atom"
 msgstr "format: %%(if) Atom ohne ein %%(then) Atom verwendet"
 
 #: ref-filter.c:726
 #, c-format
@@ -4446,113 +4438,111 @@ msgstr "doppelte ersetzende Referenz: %s"
 
 #: replace-object.c:73
 #, c-format
 msgid "replace depth too high for object %s"
 msgstr "Ersetzungstiefe zu hoch für Objekt %s"
 
 #: rerere.c:217 rerere.c:226 rerere.c:229
 msgid "corrupt MERGE_RR"
-msgstr ""
+msgstr "Fehlerhaftes MERGE_RR"
 
 #: rerere.c:264 rerere.c:269
-#, fuzzy
 msgid "unable to write rerere record"
-msgstr "Konnte Notiz-Objekt nicht schreiben"
+msgstr "Konnte Rerere-Eintrag nicht schreiben."
 
 #: rerere.c:485 rerere.c:692 sequencer.c:3136 sequencer.c:3162
 #, c-format
 msgid "could not write '%s'"
 msgstr "Konnte '%s' nicht schreiben."
 
 #: rerere.c:495
-#, fuzzy, c-format
+#, c-format
 msgid "there were errors while writing '%s' (%s)"
-msgstr "Lesefehler beim Indizieren von '%s'."
+msgstr "Fehler beim Schreiben von '%s' (%s)."
 
 #: rerere.c:498
-#, fuzzy, c-format
+#, c-format
 msgid "failed to flush '%s'"
-msgstr "Konnte '%s' nicht lesen"
+msgstr "Flush bei '%s' fehlgeschlagen."
 
 #: rerere.c:503 rerere.c:1039
-#, fuzzy, c-format
+#, c-format
 msgid "could not parse conflict hunks in '%s'"
-msgstr "Konnte Commit '%s' nicht parsen."
+msgstr "Konnte Konflikt-Blöcke in '%s' nicht parsen."
 
 #: rerere.c:684
-#, fuzzy, c-format
+#, c-format
 msgid "failed utime() on '%s'"
 msgstr "Fehler beim Aufruf von utime() auf '%s'."
 
 #: rerere.c:694
-#, fuzzy, c-format
+#, c-format
 msgid "writing '%s' failed"
-msgstr "Konnte '%s' nicht erstellen"
+msgstr "Schreiben von '%s' fehlgeschlagen."
 
 #: rerere.c:714
 #, c-format
 msgid "Staged '%s' using previous resolution."
-msgstr ""
+msgstr "'%s' mit vorheriger Konfliktauflösung zum Commit vorgemerkt."
 
 #: rerere.c:753
-#, fuzzy, c-format
+#, c-format
 msgid "Recorded resolution for '%s'."
-msgstr "aufgezeichnete Auflösung von Merge-Konflikten wiederverwenden"
+msgstr "Konfliktauflösung für '%s' aufgezeichnet."
 
 #: rerere.c:788
 #, c-format
 msgid "Resolved '%s' using previous resolution."
-msgstr ""
+msgstr "Konflikte in '%s' mit vorheriger Konfliktauflösung beseitigt."
 
 #: rerere.c:803
-#, fuzzy, c-format
+#, c-format
 msgid "cannot unlink stray '%s'"
-msgstr "kann symbolische Verknüpfung '%s' auf '%s' nicht erstellen"
+msgstr "Kann '%s' nicht löschen."
 
 #: rerere.c:807
-#, fuzzy, c-format
+#, c-format
 msgid "Recorded preimage for '%s'"
-msgstr "Konnte Log für '%s' nicht parsen."
+msgstr "Preimage für '%s' aufgezeichnet."
 
 #: rerere.c:881 submodule.c:1763 builtin/submodule--helper.c:1413
 #: builtin/submodule--helper.c:1423
 #, c-format
 msgid "could not create directory '%s'"
 msgstr "Konnte Verzeichnis '%s' nicht erstellen."
 
 #: rerere.c:1057
-#, fuzzy, c-format
+#, c-format
 msgid "failed to update conflicted state in '%s'"
-msgstr "Fehler beim Ausführen von 'git status' auf '%s'"
+msgstr "Fehler beim Aktualisieren des Konflikt-Status in '%s'."
 
 #: rerere.c:1068 rerere.c:1075
-#, fuzzy, c-format
+#, c-format
 msgid "no remembered resolution for '%s'"
-msgstr "Konnte Merge-Ergebnis von '%s' nicht hinzufügen."
+msgstr "Keine aufgezeichnete Konfliktauflösung für '%s'."
 
 #: rerere.c:1077
-#, fuzzy, c-format
+#, c-format
 msgid "cannot unlink '%s'"
-msgstr "kann Verweis '%s' nicht lesen"
+msgstr "Kann '%s' nicht löschen."
 
 #: rerere.c:1087
-#, fuzzy, c-format
+#, c-format
 msgid "Updated preimage for '%s'"
-msgstr "Ersetzende Referenz '%s' gelöscht."
+msgstr "Preimage für '%s' aktualisiert."
 
 #: rerere.c:1096
-#, fuzzy, c-format
+#, c-format
 msgid "Forgot resolution for '%s'\n"
-msgstr "Konnte Log für '%s' nicht parsen."
+msgstr "Aufgezeichnete Konfliktauflösung für '%s' gelöscht.\n"
 
 #: rerere.c:119

Re: [PATCH] doc: update diff-format.txt for removed ellipses in --raw

2018-11-25 Thread Junio C Hamano
Greg Hurrell  writes:

> Since 7cb6ac1e4b ("diff: diff_aligned_abbrev: remove ellipsis after
> abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff
> does not add ellipses in an attempt to align the output, but the
> documentation was not updated to reflect this.
>
> Signed-off-by: Greg Hurrell 
> ---
>
> The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now,
> to bring back the old output format, but that is already documented in
> git.txt, so I am not mentioning it here.

Thanks. Will queue.


Re: [RFC PATCH 1/7] Documentation: update INSTALL for NetBSD

2018-11-25 Thread Junio C Hamano
Carlo Marcelo Arenas Belón   writes:

> NetBSD added a BSD licensed reimplementation of GNU libintl to
> its base at least since release 4.0 (mid 2012) and git can be
> configured to build with it.
>
> Signed-off-by: Carlo Marcelo Arenas Belón 
> ---
>  INSTALL | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Looks good.

> diff --git a/INSTALL b/INSTALL
> index c39006e8e7..100718989f 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -154,11 +154,11 @@ Issues of note:
> git-gui, you can use NO_TCLTK.
>  
>   - A gettext library is used by default for localizing Git. The
> -   primary target is GNU libintl, but the Solaris gettext
> -   implementation also works.
> +   primary target is GNU libintl, but the Solaris and NetBSD gettext
> +   implementations also work.
>  
> We need a gettext.h on the system for C code, gettext.sh (or
> -   Solaris gettext(1)) for shell scripts, and libintl-perl for Perl
> +   gettext(1) utility) for shell scripts, and libintl-perl for Perl
> programs.
>  
> Set NO_GETTEXT to disable localization support and make Git only


[RFC PATCH 1/7] Documentation: update INSTALL for NetBSD

2018-11-25 Thread Carlo Marcelo Arenas Belón
NetBSD added a BSD licensed reimplementation of GNU libintl to
its base at least since release 4.0 (mid 2012) and git can be
configured to build with it.

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 INSTALL | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/INSTALL b/INSTALL
index c39006e8e7..100718989f 100644
--- a/INSTALL
+++ b/INSTALL
@@ -154,11 +154,11 @@ Issues of note:
  git-gui, you can use NO_TCLTK.
 
- A gettext library is used by default for localizing Git. The
- primary target is GNU libintl, but the Solaris gettext
- implementation also works.
+ primary target is GNU libintl, but the Solaris and NetBSD gettext
+ implementations also work.
 
  We need a gettext.h on the system for C code, gettext.sh (or
- Solaris gettext(1)) for shell scripts, and libintl-perl for Perl
+ gettext(1) utility) for shell scripts, and libintl-perl for Perl
  programs.
 
  Set NO_GETTEXT to disable localization support and make Git only
-- 
2.20.0.rc1



[PATCH] doc: update diff-format.txt for removed ellipses in --raw

2018-11-24 Thread Greg Hurrell
Since 7cb6ac1e4b ("diff: diff_aligned_abbrev: remove ellipsis after
abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff
does not add ellipses in an attempt to align the output, but the
documentation was not updated to reflect this.

Signed-off-by: Greg Hurrell 
---

The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now,
to bring back the old output format, but that is already documented in
git.txt, so I am not mentioning it here.

 Documentation/diff-format.txt | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
index 706916c94c..cdcc17f0ad 100644
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -26,12 +26,12 @@ line per changed file.
 An output line is formatted this way:
 
 
-in-place edit  :100644 100644 bcd1234... 0123456... M file0
-copy-edit  :100644 100644 abcd123... 1234567... C68 file1 file2
-rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3
-create :00 100644 000... 1234567... A file4
-delete :100644 00 1234567... 000... D file5
-unmerged   :00 00 000... 000... U file6
+in-place edit  :100644 100644 bcd1234 0123456 M file0
+copy-edit  :100644 100644 abcd123 1234567 C68 file1 file2
+rename-edit:100644 100644 abcd123 1234567 R86 file1 file3
+create :00 100644 000 1234567 A file4
+delete :100644 00 1234567 000 D file5
+unmerged   :00 00 000 000 U file6
 
 
 That is, from the left to the right:
@@ -75,7 +75,7 @@ and it is out of sync with the index.
 Example:
 
 
-:100644 100644 5be4a4.. 00.. M file.c
+:100644 100644 5be4a4a 000 M file.c
 
 
 Without the `-z` option, pathnames with "unusual" characters are
@@ -100,7 +100,7 @@ from the format described above in the following way:
 Example:
 
 
-::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM describe.c
+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM  describe.c
 
 
 Note that 'combined diff' lists only files which were modified from
-- 
2.19.0



Re: [PATCH] doc: update diff-format.txt for removed ellipses

2018-11-23 Thread Junio C Hamano
Thanks for a patch.

Greg Hurrell  writes:

> Commit 7cb6ac1e4b made the diff format omit ellipses by default, but
> there is still this place in the documentation where we show examples of
> output with ellipses.

We prefer to cite an existing commit with its title and date these
days, not just with its object name.

Since 7cb6ac1e ("diff: diff_aligned_abbrev: remove ellipsis after
abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff
does not add ellipsis in an attempt to align the output, but...

or something like that.  Note that saying this is about the raw format
is quite essential thing to tell the readers to explain this hange.

> The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now,
> to bring back the old output format, but that is already documented in
> git.txt, so I am not mentioning it here.

Yeah, I do not think it makes sense to use the workaround that is
planned for removal, which will later make us revise the example in
the documentation again, to end up with the text that you have right
now.  I do not think this three-line paragraph needs to be in the
log message, either, though.  Perhaps below the three-dash line.


Also please sign-off your patch here (see
Documentation/SubmittingPatches).

> ---

Thanks.

>  Documentation/diff-format.txt | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
> index 706916c94c..cdcc17f0ad 100644
> --- a/Documentation/diff-format.txt
> +++ b/Documentation/diff-format.txt
> @@ -26,12 +26,12 @@ line per changed file.
>  An output line is formatted this way:
>  
>  
> -in-place edit  :100644 100644 bcd1234... 0123456... M file0
> -copy-edit  :100644 100644 abcd123... 1234567... C68 file1 file2
> -rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3
> -create :00 100644 000... 1234567... A file4
> -delete :100644 00 1234567... 000... D file5
> -unmerged   :00 00 000... 000... U file6
> +in-place edit  :100644 100644 bcd1234 0123456 M file0
> +copy-edit  :100644 100644 abcd123 1234567 C68 file1 file2
> +rename-edit:100644 100644 abcd123 1234567 R86 file1 file3
> +create :00 100644 000 1234567 A file4
> +delete :100644 00 1234567 000 D file5
> +unmerged   :00 00 000 000 U file6
>  
>  
>  That is, from the left to the right:
> @@ -75,7 +75,7 @@ and it is out of sync with the index.
>  Example:
>  
>  
> -:100644 100644 5be4a4.. 00.. M file.c
> +:100644 100644 5be4a4a 000 M file.c
>  
>  
>  Without the `-z` option, pathnames with "unusual" characters are
> @@ -100,7 +100,7 @@ from the format described above in the following way:
>  Example:
>  
>  
> -::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM   describe.c
> +::100644 100644 100644 fabadb8 cc95eb0 4866510 MMdescribe.c
>  
>  
>  Note that 'combined diff' lists only files which were modified from


[PATCH] doc: update diff-format.txt for removed ellipses

2018-11-23 Thread Greg Hurrell
Commit 7cb6ac1e4b made the diff format omit ellipses by default, but
there is still this place in the documentation where we show examples of
output with ellipses.

The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now,
to bring back the old output format, but that is already documented in
git.txt, so I am not mentioning it here.
---
 Documentation/diff-format.txt | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
index 706916c94c..cdcc17f0ad 100644
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -26,12 +26,12 @@ line per changed file.
 An output line is formatted this way:
 
 
-in-place edit  :100644 100644 bcd1234... 0123456... M file0
-copy-edit  :100644 100644 abcd123... 1234567... C68 file1 file2
-rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3
-create :00 100644 000... 1234567... A file4
-delete :100644 00 1234567... 000... D file5
-unmerged   :00 00 000... 000... U file6
+in-place edit  :100644 100644 bcd1234 0123456 M file0
+copy-edit  :100644 100644 abcd123 1234567 C68 file1 file2
+rename-edit:100644 100644 abcd123 1234567 R86 file1 file3
+create :00 100644 000 1234567 A file4
+delete :100644 00 1234567 000 D file5
+unmerged   :00 00 000 000 U file6
 
 
 That is, from the left to the right:
@@ -75,7 +75,7 @@ and it is out of sync with the index.
 Example:
 
 
-:100644 100644 5be4a4.. 00.. M file.c
+:100644 100644 5be4a4a 000 M file.c
 
 
 Without the `-z` option, pathnames with "unusual" characters are
@@ -100,7 +100,7 @@ from the format described above in the following way:
 Example:
 
 
-::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM describe.c
+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM  describe.c
 
 
 Note that 'combined diff' lists only files which were modified from
-- 
2.19.0



[PATCH 0/1] mingw: update a link to the official documentation

2018-11-15 Thread Johannes Schindelin via GitGitGadget
It is a pretty neat thing that the bulky MSDN documentation has been
replaced by something a lot more open, something that can be updated via
Pull Requests on GitHub. Let's link to this new documentation instead of the
obsolete one.

I know, it is really close to the code freeze leading up to Git v2.20.0. But
this is just an update to a code comment... :-)

Johannes Schindelin (1):
  mingw: replace an obsolete link with the superseding one

 compat/mingw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: d166e6afe5f257217836ef24a73764eba390c58d
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-81%2Fdscho%2Fmingw-update-msdn-link-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-81/dscho/mingw-update-msdn-link-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/81
-- 
gitgitgadget


Re: [PATCH v2 1/1] Update .mailmap

2018-11-11 Thread Junio C Hamano
"Johannes Schindelin via GitGitGadget" 
writes:

> From: Johannes Schindelin 
>
> This patch makes the output of `git shortlog -nse v2.10.0..master`
> duplicate-free.
>
> Signed-off-by: Johannes Schindelin 
> ---

Thanks, will queue.


[PATCH v2 0/1] Update .mailmap

2018-11-09 Thread Johannes Schindelin via GitGitGadget
I noticed that there were a couple of duplicate entries in git shortlog -nse
v2.19.1.., and then continued a bit to remove the duplicate entries even for
v2.10.0..

Changes relative to v1:

 * Fixed the commit message (it talked about the opposite commit range than
   intended).
 * Added a formerly missing space between the email addresses of Masaya.

Johannes Schindelin (1):
  Update .mailmap

 .mailmap | 13 +
 1 file changed, 13 insertions(+)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-71%2Fdscho%2Fmailmap-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-71/dscho/mailmap-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/71

Range-diff vs v1:

 1:  b590a9bebf ! 1:  c121ebc474 Update .mailmap
 @@ -2,7 +2,7 @@
  
  Update .mailmap
  
 -This patch makes the output of `git shortlog -nse v2.10.0`
 +This patch makes the output of `git shortlog -nse v2.10.0..master`
  duplicate-free.
  
  Signed-off-by: Johannes Schindelin 
 @@ -47,7 +47,7 @@
   Mark Rada 
   Martin Langhoff  
   Martin von Zweigbergk  

 -+Masaya Suzuki 
 ++Masaya Suzuki  
   Matt Draisey  
   Matt Kraai  
   Matt McCutchen  

-- 
gitgitgadget


[PATCH v2 1/1] Update .mailmap

2018-11-09 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

This patch makes the output of `git shortlog -nse v2.10.0..master`
duplicate-free.

Signed-off-by: Johannes Schindelin 
---
 .mailmap | 13 +
 1 file changed, 13 insertions(+)

diff --git a/.mailmap b/.mailmap
index bef3352b0d..eb7b5fc7b9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -21,6 +21,8 @@ Anders Kaseorg  
 Aneesh Kumar K.V 
 Amos Waterland  
 Amos Waterland  
+Ben Peart  
+Ben Peart  
 Ben Walton  
 Benoit Sigoure  
 Bernt Hansen  
@@ -32,6 +34,7 @@ Bryan Larsen  
 Cheng Renquan 
 Chris Shoemaker 
 Chris Wright  
+Christian Ludwig  
 Cord Seele  
 Christian Couder  
 Christian Stimming  
@@ -51,6 +54,7 @@ David Reiss  

 David S. Miller 
 David Turner  
 David Turner  
+Derrick Stolee  
 Deskin Miller 
 Dirk Süsserott 
 Eric Blake  
@@ -98,6 +102,7 @@ Jens Axboe  
 Jens Lindström  Jens Lindstrom 
 Jim Meyering  
 Joachim Berdal Haga 
+Joachim Jablon  
 Johannes Schindelin  
 Johannes Sixt  
 Johannes Sixt  
@@ -150,6 +155,7 @@ Mark Levedahl  
 Mark Rada 
 Martin Langhoff  
 Martin von Zweigbergk  
+Masaya Suzuki  
 Matt Draisey  
 Matt Kraai  
 Matt McCutchen  
@@ -157,6 +163,7 @@ Matthias Kestenholz  

 Matthias Rüster  Matthias Ruester
 Matthias Urlichs  
 Matthias Urlichs  
+Matthieu Moy  
 Michael Coleman 
 Michael J Gruber  
 Michael J Gruber  
@@ -180,7 +187,11 @@ Nick Stokoe  Nick Woolley 

 Nick Stokoe  Nick Woolley 
 Nicolas Morey-Chaisemartin  

 Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

 Nicolas Sebrecht  
+Orgad Shaneh  
 Paolo Bonzini  
 Pascal Obry  
 Pascal Obry  
@@ -200,6 +211,7 @@ Philipp A. Hartmann  
 Philippe Bruhat 
 Ralf Thielow  
 Ramsay Jones  
+Randall S. Becker  
 René Scharfe  
 René Scharfe  Rene Scharfe
 Richard Hansen  
@@ -238,6 +250,7 @@ Steven Walter  

 Sven Verdoolaege  
 Sven Verdoolaege  
 SZEDER Gábor  
+Tao Qingyun  <845767...@qq.com>
 Tay Ray Chuan 
 Ted Percival  
 Theodore Ts'o 
-- 
gitgitgadget


Re: [PATCH 1/1] Update .mailmap

2018-11-09 Thread Johannes Schindelin
Hi Junio,

On Fri, 9 Nov 2018, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" 
> writes:
> 
> > From: Johannes Schindelin 
> >
> > This patch makes the output of `git shortlog -nse v2.10.0`
> > duplicate-free.
> 
> Did you mean "v2.10.0..master" or did you really mean this covers
> authors recorded up to v2.10.0?  Judging from the cover letter, I
> think you meant the former.

D'oh. Yes, I meant v2.10.0..master.

> Can you say a bit more about how one among multiple addresses for
> each person was chosen in the log message?  E.g. "After asking each
> author which one is the preferred one", "Picked the one with the
> most recent committer timestamps", "There were two for each but one
> of them were bouncing", etc.

I looked at each of the authors' mails and tried to determine which one
was the preferred one. For example, Masaya sent a patch from their gmail
account but signed off using their google account, so I figured the latter
was preferable. Nicolas, on the other hand, already had a couple of
entries in .mailmap, so I picked the one that seemed to be preferred
judging by the .mailmap even if it was not in use lately.

For Christian, it was gmail vs googlemail, and I picked the former, as it
is more common these days.

For Ben and Stolee, I used their Microsoft accounts (preferring the one
Ben used originally).

For Joachim, Matthieu, Randall and Tao, I used the more personalized email
address.

For Orgad, I used his personal one rather than his work email.

> > @@ -150,6 +155,7 @@ Mark Levedahl  
> >  Mark Rada 
> >  Martin Langhoff  
> >  Martin von Zweigbergk  
> > 
> > +Masaya Suzuki 
> 
> It is a bit surprising that we can take an entry without SP in
> between two addresses and still behave sensibley, but it probably
> makes more sense to add one just to look similar to other entries.

Whoops. Thanks, fixed!

> Thanks for working on this.

You're welcome.

v2 about to be sent,
Dscho


Re: [PATCH 1/1] Update .mailmap

2018-11-08 Thread Junio C Hamano
"Johannes Schindelin via GitGitGadget" 
writes:

> From: Johannes Schindelin 
>
> This patch makes the output of `git shortlog -nse v2.10.0`
> duplicate-free.

Did you mean "v2.10.0..master" or did you really mean this covers
authors recorded up to v2.10.0?  Judging from the cover letter, I
think you meant the former.

Can you say a bit more about how one among multiple addresses for
each person was chosen in the log message?  E.g. "After asking each
author which one is the preferred one", "Picked the one with the
most recent committer timestamps", "There were two for each but one
of them were bouncing", etc.

> @@ -150,6 +155,7 @@ Mark Levedahl  
>  Mark Rada 
>  Martin Langhoff  
>  Martin von Zweigbergk  
> 
> +Masaya Suzuki 

It is a bit surprising that we can take an entry without SP in
between two addresses and still behave sensibley, but it probably
makes more sense to add one just to look similar to other entries.

Thanks for working on this.


[PATCH 1/1] Update .mailmap

2018-11-08 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

This patch makes the output of `git shortlog -nse v2.10.0`
duplicate-free.

Signed-off-by: Johannes Schindelin 
---
 .mailmap | 13 +
 1 file changed, 13 insertions(+)

diff --git a/.mailmap b/.mailmap
index bef3352b0d..1d8310073a 100644
--- a/.mailmap
+++ b/.mailmap
@@ -21,6 +21,8 @@ Anders Kaseorg  
 Aneesh Kumar K.V 
 Amos Waterland  
 Amos Waterland  
+Ben Peart  
+Ben Peart  
 Ben Walton  
 Benoit Sigoure  
 Bernt Hansen  
@@ -32,6 +34,7 @@ Bryan Larsen  
 Cheng Renquan 
 Chris Shoemaker 
 Chris Wright  
+Christian Ludwig  
 Cord Seele  
 Christian Couder  
 Christian Stimming  
@@ -51,6 +54,7 @@ David Reiss  

 David S. Miller 
 David Turner  
 David Turner  
+Derrick Stolee  
 Deskin Miller 
 Dirk Süsserott 
 Eric Blake  
@@ -98,6 +102,7 @@ Jens Axboe  
 Jens Lindström  Jens Lindstrom 
 Jim Meyering  
 Joachim Berdal Haga 
+Joachim Jablon  
 Johannes Schindelin  
 Johannes Sixt  
 Johannes Sixt  
@@ -150,6 +155,7 @@ Mark Levedahl  
 Mark Rada 
 Martin Langhoff  
 Martin von Zweigbergk  
+Masaya Suzuki 
 Matt Draisey  
 Matt Kraai  
 Matt McCutchen  
@@ -157,6 +163,7 @@ Matthias Kestenholz  

 Matthias Rüster  Matthias Ruester
 Matthias Urlichs  
 Matthias Urlichs  
+Matthieu Moy  
 Michael Coleman 
 Michael J Gruber  
 Michael J Gruber  
@@ -180,7 +187,11 @@ Nick Stokoe  Nick Woolley 

 Nick Stokoe  Nick Woolley 
 Nicolas Morey-Chaisemartin  

 Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

 Nicolas Sebrecht  
+Orgad Shaneh  
 Paolo Bonzini  
 Pascal Obry  
 Pascal Obry  
@@ -200,6 +211,7 @@ Philipp A. Hartmann  
 Philippe Bruhat 
 Ralf Thielow  
 Ramsay Jones  
+Randall S. Becker  
 René Scharfe  
 René Scharfe  Rene Scharfe
 Richard Hansen  
@@ -238,6 +250,7 @@ Steven Walter  

 Sven Verdoolaege  
 Sven Verdoolaege  
 SZEDER Gábor  
+Tao Qingyun  <845767...@qq.com>
 Tay Ray Chuan 
 Ted Percival  
 Theodore Ts'o 
-- 
gitgitgadget


[PATCH 0/1] Update .mailmap

2018-11-08 Thread Johannes Schindelin via GitGitGadget
I noticed that there were a couple of duplicate entries in git shortlog -nse
v2.19.1.., and then continued a bit to remove the duplicate entries even for
v2.10.0..

Johannes Schindelin (1):
  Update .mailmap

 .mailmap | 13 +
 1 file changed, 13 insertions(+)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-71%2Fdscho%2Fmailmap-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-71/dscho/mailmap-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/71
-- 
gitgitgadget


Update

2018-11-03 Thread Bruce Blake
Hello Dear

how are you doing today? my name is Bruce Blake, the manager foreign affairs in 
City Finance Bank, we have a customer here in bank that has not accessed his 
account for the past 18 years, after some research made about him we found out 
he was a victim of the crashed mining company in mexico.

 he died a single man no wife One son from his first marriage that he divorced, 
his next of kin is that his only son which we can arrange and make your name 
his next of kin as his brother, and the mandatory rule of the bank is that 
after 25 years of a dormant account, the fund should be moved to Government 
Treasury account.

This Customer was my friend because i have known him since he opened an account 
with us, i can't claim his fund because i am a staff in this bank, that is why 
i want you to come in and claim this fund as my late clients Relative.

if you are interested get back to me so we could discuss and make arrangements 
for the paper work because everything has to be legit and Legal.

Regards

Bruce Blake.


Re: [PATCH v3 1/2] worktree: update documentation for lock_reason and lock_reason_valid

2018-10-30 Thread Junio C Hamano
nbelakov...@gmail.com writes:

> From: Nickolai Belakovski 
>
> Clarify that these fields are to be considered implementation details
> and direct the reader to use the is_worktree_locked function to retrieve
> said information.
>
> Signed-off-by: Nickolai Belakovski 
> ---
>  worktree.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/worktree.h b/worktree.h
> index df3fc30f7..6b12a3cf6 100644
> --- a/worktree.h
> +++ b/worktree.h
> @@ -10,12 +10,12 @@ struct worktree {
>   char *path;
>   char *id;
>   char *head_ref; /* NULL if HEAD is broken or detached */
> - char *lock_reason;  /* internal use */
> + char *lock_reason;  /* private - use is_worktree_locked */

s/use /used by/, probably.

>   struct object_id head_oid;
>   int is_detached;
>   int is_bare;
>   int is_current;
> - int lock_reason_valid;
> + int lock_reason_valid; /* private */
>  };

These annotations to the two fields are not wrong per-se, but I have
a feeling that it would equally be important to document what the
other "non-private" fields mean, if peeking them *is* the API this
subsystem offers.

Thanks.


[PATCH v3 1/2] worktree: update documentation for lock_reason and lock_reason_valid

2018-10-30 Thread nbelakovski
From: Nickolai Belakovski 

Clarify that these fields are to be considered implementation details
and direct the reader to use the is_worktree_locked function to retrieve
said information.

Signed-off-by: Nickolai Belakovski 
---
 worktree.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/worktree.h b/worktree.h
index df3fc30f7..6b12a3cf6 100644
--- a/worktree.h
+++ b/worktree.h
@@ -10,12 +10,12 @@ struct worktree {
char *path;
char *id;
char *head_ref; /* NULL if HEAD is broken or detached */
-   char *lock_reason;  /* internal use */
+   char *lock_reason;  /* private - use is_worktree_locked */
struct object_id head_oid;
int is_detached;
int is_bare;
int is_current;
-   int lock_reason_valid;
+   int lock_reason_valid; /* private */
 };
 
 /* Functions for acting on the information about worktrees. */
-- 
2.14.2



Re: [PATCH] update git-http-backend doc for lighttpd

2018-10-28 Thread Junio C Hamano
Glenn Strauss  writes:

> use "GIT_HTTP_EXPORT_ALL" => "1" with a value for best compatiblity.
> lighttpd 1.4.51 setenv.add-environment does add vars with empty value.
> lighttpd setenv.set-environment does, but was only introduced in 1.4.46
>
> git-http-backend may be found at /usr/libexec/git-core/git-http-backend
>
> scope lighttpd config directives for git-http-backend under "^/git"
>
> Signed-off-by: Glenn Strauss 
> ---
>  Documentation/git-http-backend.txt | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-http-backend.txt 
> b/Documentation/git-http-backend.txt
> index bb0db195cebd6..905aa1056d26f 100644
> --- a/Documentation/git-http-backend.txt
> +++ b/Documentation/git-http-backend.txt
> @@ -192,16 +192,16 @@ ScriptAlias /git/ /var/www/cgi-bin/gitweb.cgi/
>  
>  Lighttpd::
>   Ensure that `mod_cgi`, `mod_alias`, `mod_auth`, `mod_setenv` are
> - loaded, then set `GIT_PROJECT_ROOT` appropriately and redirect
> - all requests to the CGI:
> + loaded, then set path to git-http-backend, set `GIT_PROJECT_ROOT`
> + appropriately, and redirect all requests to the CGI:

The addition here is

set path to git-http-backend

That reads as if you are telling the reader to do this

GIT_PROJECT_ROOT => "/var/www/git",
path => "/usr/libexec/git-core/git-http-backend"

because the descriptions for these two are next to each other and so
similar, but I somehow do not think you meant there is a variable
whose name is `path` (note that I do not use lighttpd and am not an
expert on its configuration---which makes me the ideal guinea pig to
judge if your update makes sense to the target audience).

Do you mean something like

use `alias.url` to mark that `/git` hierarchy is handled by
the `git-http-backend` binary (use the full path to the
program).

I do not see any quoting in your updated text, but many of what the
end-user needs to type literally must be `quoted for monospace`, I
would think.

>  +
>  
> -alias.url += ( "/git" => "/usr/lib/git-core/git-http-backend" )
>  $HTTP["url"] =~ "^/git" {
> + alias.url += ("/git" => "/usr/libexec/git-core/git-http-backend")
>   cgi.assign = ("" => "")
>   setenv.add-environment = (
>   "GIT_PROJECT_ROOT" => "/var/www/git",
> - "GIT_HTTP_EXPORT_ALL" => ""
> + "GIT_HTTP_EXPORT_ALL" => "1"
>   )
>  }
>  
>
> --
> https://github.com/git/git/pull/546

Thanks.


[PATCH] update git-http-backend doc for lighttpd

2018-10-27 Thread Glenn Strauss
use "GIT_HTTP_EXPORT_ALL" => "1" with a value for best compatiblity.
lighttpd 1.4.51 setenv.add-environment does add vars with empty value.
lighttpd setenv.set-environment does, but was only introduced in 1.4.46

git-http-backend may be found at /usr/libexec/git-core/git-http-backend

scope lighttpd config directives for git-http-backend under "^/git"

Signed-off-by: Glenn Strauss 
---
 Documentation/git-http-backend.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-http-backend.txt 
b/Documentation/git-http-backend.txt
index bb0db195cebd6..905aa1056d26f 100644
--- a/Documentation/git-http-backend.txt
+++ b/Documentation/git-http-backend.txt
@@ -192,16 +192,16 @@ ScriptAlias /git/ /var/www/cgi-bin/gitweb.cgi/
 
 Lighttpd::
Ensure that `mod_cgi`, `mod_alias`, `mod_auth`, `mod_setenv` are
-   loaded, then set `GIT_PROJECT_ROOT` appropriately and redirect
-   all requests to the CGI:
+   loaded, then set path to git-http-backend, set `GIT_PROJECT_ROOT`
+   appropriately, and redirect all requests to the CGI:
 +
 
-alias.url += ( "/git" => "/usr/lib/git-core/git-http-backend" )
 $HTTP["url"] =~ "^/git" {
+   alias.url += ("/git" => "/usr/libexec/git-core/git-http-backend")
cgi.assign = ("" => "")
setenv.add-environment = (
"GIT_PROJECT_ROOT" => "/var/www/git",
-   "GIT_HTTP_EXPORT_ALL" => ""
+   "GIT_HTTP_EXPORT_ALL" => "1"
)
 }
 

--
https://github.com/git/git/pull/546


[PATCH 01/78] Update makefile in preparation for Documentation/config/*.txt

2018-10-27 Thread Nguyễn Thái Ngọc Duy
config.txt is going to be broken down in smaller pieces and put under
Documentation/config directory. Update build rules to take these files
into account.

A dummy file is added to make sure wildcard expansion is predictable
(depending on shell setting it could expand to nothing or becomes a
path if config directory is empty). The file will be deleted once the
move is over.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/Makefile | 2 +-
 Documentation/config/dummy.txt | 0
 Makefile   | 2 +-
 generate-cmdlist.sh| 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/config/dummy.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 95f6a321f2..48d261dc2c 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -285,7 +285,7 @@ docdep_prereqs = \
mergetools-list.made $(mergetools_txt) \
cmd-list.made $(cmds_txt)
 
-doc.dep : $(docdep_prereqs) $(wildcard *.txt) build-docdep.perl
+doc.dep : $(docdep_prereqs) $(wildcard *.txt) $(wildcard config/*.txt) 
build-docdep.perl
$(QUIET_GEN)$(RM) $@+ $@ && \
$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
mv $@+ $@
diff --git a/Documentation/config/dummy.txt b/Documentation/config/dummy.txt
new file mode 100644
index 00..e69de29bb2
diff --git a/Makefile b/Makefile
index b08d5ea258..e2ca83203f 100644
--- a/Makefile
+++ b/Makefile
@@ -2068,7 +2068,7 @@ $(BUILT_INS): git$X
 
 command-list.h: generate-cmdlist.sh command-list.txt
 
-command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt
+command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt 
Documentation/config/*.txt
$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ 
&& mv $@+ $@
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index fa1e5475e8..709d67405b 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -80,7 +80,7 @@ print_config_list () {
cat <

Re: [PATCH 10/10] builtin/fetch: check for submodule updates in any ref update

2018-10-26 Thread Jonathan Tan
> Gerrit, the code review tool, has a different workflow than our mailing
> list based approach. Usually users upload changes to a Gerrit server and
> continuous integration and testing happens by bots. Sometimes however a
> user wants to checkout a change locally and look at it locally. For this
> use case, Gerrit offers a command line snippet to copy and paste to your
> terminal, which looks like

As I said in my review of the previous patch, I think this should be
squashed into the previous patch.

Also, remember to add the version when formatting the e-mail ("[PATCH
v? ??/??]").


[PATCH 10/10] builtin/fetch: check for submodule updates in any ref update

2018-10-25 Thread Stefan Beller
Gerrit, the code review tool, has a different workflow than our mailing
list based approach. Usually users upload changes to a Gerrit server and
continuous integration and testing happens by bots. Sometimes however a
user wants to checkout a change locally and look at it locally. For this
use case, Gerrit offers a command line snippet to copy and paste to your
terminal, which looks like

  git fetch https:///gerrit refs/changes/ &&
  git checkout FETCH_HEAD

For Gerrit changes that contain changing submodule gitlinks, it would be
easy to extend both the fetch and checkout with the '--recurse-submodules'
flag, such that this command line snippet would produce the state of a
change locally.

However the functionality added in the previous patch, which would
ensure that we fetch the objects in the submodule that the gitlink pointed
at, only works for remote tracking branches so far, not for FETCH_HEAD.

Make sure that fetching a superproject to its FETCH_HEAD, also respects
the existence checks for objects in the submodule recursion.

Signed-off-by: Stefan Beller 
---
 builtin/fetch.c |  8 ++--
 t/t5526-fetch-submodules.sh | 24 
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 95c44bf6ff..f39012d7c2 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -700,8 +700,6 @@ static int update_local_ref(struct ref *ref,
what = _("[new ref]");
}
 
-   if (recurse_submodules != RECURSE_SUBMODULES_OFF)
-   check_for_new_submodule_commits(>new_oid);
r = s_update_ref(msg, ref, 0);
format_display(display, r ? '!' : '*', what,
   r ? _("unable to update local ref") : NULL,
@@ -715,8 +713,6 @@ static int update_local_ref(struct ref *ref,
strbuf_add_unique_abbrev(, >object.oid, 
DEFAULT_ABBREV);
strbuf_addstr(, "..");
strbuf_add_unique_abbrev(, >new_oid, 
DEFAULT_ABBREV);
-   if (recurse_submodules != RECURSE_SUBMODULES_OFF)
-   check_for_new_submodule_commits(>new_oid);
r = s_update_ref("fast-forward", ref, 1);
format_display(display, r ? '!' : ' ', quickref.buf,
   r ? _("unable to update local ref") : NULL,
@@ -729,8 +725,6 @@ static int update_local_ref(struct ref *ref,
strbuf_add_unique_abbrev(, >object.oid, 
DEFAULT_ABBREV);
strbuf_addstr(, "...");
strbuf_add_unique_abbrev(, >new_oid, 
DEFAULT_ABBREV);
-   if (recurse_submodules != RECURSE_SUBMODULES_OFF)
-   check_for_new_submodule_commits(>new_oid);
r = s_update_ref("forced-update", ref, 1);
format_display(display, r ? '!' : '+', quickref.buf,
   r ? _("unable to update local ref") : _("forced 
update"),
@@ -826,6 +820,8 @@ static int store_updated_refs(const char *raw_url, const 
char *remote_name,
ref->force = rm->peer_ref->force;
}
 
+   if (recurse_submodules != RECURSE_SUBMODULES_OFF)
+   check_for_new_submodule_commits(>old_oid);
 
if (!strcmp(rm->name, "HEAD")) {
kind = "";
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 5a75b57852..799785783f 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -631,4 +631,28 @@ test_expect_success "fetch new submodule commits on-demand 
outside standard refs
)
 '
 
+test_expect_success 'fetch new submodule commits on-demand in FETCH_HEAD' '
+   # depends on the previous test for setup
+
+   C=$(git -C submodule commit-tree -m "another change outside refs/heads" 
HEAD^{tree}) &&
+   git -C submodule update-ref refs/changes/1 $C &&
+   git update-index --cacheinfo 16 $C submodule &&
+   test_tick &&
+
+   D=$(git -C sub1 commit-tree -m "another change outside refs/heads" 
HEAD^{tree}) &&
+   git -C sub1 update-ref refs/changes/2 $D &&
+   git update-index --cacheinfo 16 $D sub1 &&
+
+   git commit -m "updated submodules outside of refs/heads" &&
+   E=$(git rev-parse HEAD) &&
+   git update-ref refs/changes/2 $E &&
+   (
+   cd downstream &&
+   git fetch --recurse-submodules origin refs/changes/2 &&
+   git -C submodule cat-file -t $C &&
+   git -C sub1 cat-file -t $D &&
+   git checkout --recurse-submodules FETCH_HEAD
+   )
+'
+
 test_done
-- 
2.19.0



[PATCH 6/6] doc: fix formatting in git-update-ref

2018-10-22 Thread Andreas Heiduk
Remove the parapgraph numbers from lines explaining the reflog format
and typeset these lines in monospace.

Signed-off-by: Andreas Heiduk 
---
 Documentation/git-update-ref.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index fda8516677..9671423117 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -129,8 +129,8 @@ a line to the log file "$GIT_DIR/logs/" (dereferencing 
all
 symbolic refs before creating the log name) describing the change
 in ref value.  Log lines are formatted as:
 
-. oldsha1 SP newsha1 SP committer LF
-+
+oldsha1 SP newsha1 SP committer LF
+
 Where "oldsha1" is the 40 character hexadecimal value previously
 stored in , "newsha1" is the 40 character hexadecimal value of
  and "committer" is the committer's name, email address
@@ -138,8 +138,8 @@ and date in the standard Git committer ident format.
 
 Optionally with -m:
 
-. oldsha1 SP newsha1 SP committer TAB message LF
-+
+oldsha1 SP newsha1 SP committer TAB message LF
+
 Where all fields are as described above and "message" is the
 value supplied to the -m option.
 
-- 
2.19.1



Re: [PATCH] receive: denyCurrentBranch=updateinstead should not blindly update

2018-10-22 Thread Johannes Schindelin
Hi Junio,

On Fri, 19 Oct 2018, Junio C Hamano wrote:

> The handling of receive.denyCurrentBranch=updateInstead was added to
> a switch statement that handles other values of the variable, but
> all the other case arms only checked a condition to reject the
> attempted push, or let later logic in the same function to still
> intervene, so that a push that does not fast-forward (which is
> checked after the switch statement in question) is still rejected.
> 
> But the handling of updateInstead incorrectly took immediate effect,
> without giving other checks a chance to intervene.
> 
> Instead of calling update_worktree() that causes the side effect
> immediately, just note the fact that we will need to call the
> funciton later, and first give other checks chance to reject the
> request.  After the update-hook gets a chance to reject the push
> (which happens as the last step in a series of checks), call
> update_worktree() when we earlier detected the need to.

Nicely explained, and the patch looks good to me, too.

Thanks,
Dscho

> 
> Reported-by: Rajesh Madamanchi
> Signed-off-by: Junio C Hamano 
> ---
>  builtin/receive-pack.c | 12 +---
>  t/t5516-fetch-push.sh  |  8 +++-
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 95740f4f0e..79ee320948 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1026,6 +1026,7 @@ static const char *update(struct command *cmd, struct 
> shallow_info *si)
>   const char *ret;
>   struct object_id *old_oid = >old_oid;
>   struct object_id *new_oid = >new_oid;
> + int do_update_worktree = 0;
>  
>   /* only refs/... are allowed */
>   if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
> @@ -1051,9 +1052,8 @@ static const char *update(struct command *cmd, struct 
> shallow_info *si)
>   refuse_unconfigured_deny();
>   return "branch is currently checked out";
>   case DENY_UPDATE_INSTEAD:
> - ret = update_worktree(new_oid->hash);
> - if (ret)
> - return ret;
> + /* pass -- let other checks intervene first */
> + do_update_worktree = 1;
>   break;
>   }
>   }
> @@ -1118,6 +1118,12 @@ static const char *update(struct command *cmd, struct 
> shallow_info *si)
>   return "hook declined";
>   }
>  
> + if (do_update_worktree) {
> + ret = update_worktree(new_oid->hash);
> + if (ret)
> + return ret;
> + }
> +
>   if (is_null_oid(new_oid)) {
>   struct strbuf err = STRBUF_INIT;
>   if (!parse_object(the_repository, old_oid)) {
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 7a8f56db53..7316365a24 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1577,7 +1577,13 @@ test_expect_success 'receive.denyCurrentBranch = 
> updateInstead' '
>   test $(git -C .. rev-parse master) = $(git rev-parse HEAD) &&
>   git diff --quiet &&
>   git diff --cached --quiet
> - )
> + ) &&
> +
> + # (6) updateInstead intervened by fast-forward check
> + test_must_fail git push void master^:master &&
> + test $(git -C void rev-parse HEAD) = $(git rev-parse master) &&
> + git -C void diff --quiet &&
> + git -C void diff --cached --quiet
>  '
>  
>  test_expect_success 'updateInstead with push-to-checkout hook' '
> -- 
> 2.19.1-450-ga4b8ab5363
> 
> 


Re: [PATCH] receive: denyCurrentBranch=updateinstead should not blindly update

2018-10-18 Thread Eric Sunshine
On Fri, Oct 19, 2018 at 12:57 AM Junio C Hamano  wrote:
> The handling of receive.denyCurrentBranch=updateInstead was added to
> a switch statement that handles other values of the variable, but
> all the other case arms only checked a condition to reject the
> attempted push, or let later logic in the same function to still
> intervene, so that a push that does not fast-forward (which is
> checked after the switch statement in question) is still rejected.
>
> But the handling of updateInstead incorrectly took immediate effect,
> without giving other checks a chance to intervene.
>
> Instead of calling update_worktree() that causes the side effect
> immediately, just note the fact that we will need to call the
> funciton later, and first give other checks chance to reject the

s/funciton/function
s/chance/a &/

> request.  After the update-hook gets a chance to reject the push
> (which happens as the last step in a series of checks), call
> update_worktree() when we earlier detected the need to.
>
> Reported-by: Rajesh Madamanchi
> Signed-off-by: Junio C Hamano 


[PATCH] receive: denyCurrentBranch=updateinstead should not blindly update

2018-10-18 Thread Junio C Hamano
The handling of receive.denyCurrentBranch=updateInstead was added to
a switch statement that handles other values of the variable, but
all the other case arms only checked a condition to reject the
attempted push, or let later logic in the same function to still
intervene, so that a push that does not fast-forward (which is
checked after the switch statement in question) is still rejected.

But the handling of updateInstead incorrectly took immediate effect,
without giving other checks a chance to intervene.

Instead of calling update_worktree() that causes the side effect
immediately, just note the fact that we will need to call the
funciton later, and first give other checks chance to reject the
request.  After the update-hook gets a chance to reject the push
(which happens as the last step in a series of checks), call
update_worktree() when we earlier detected the need to.

Reported-by: Rajesh Madamanchi
Signed-off-by: Junio C Hamano 
---
 builtin/receive-pack.c | 12 +---
 t/t5516-fetch-push.sh  |  8 +++-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 95740f4f0e..79ee320948 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1026,6 +1026,7 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
const char *ret;
struct object_id *old_oid = >old_oid;
struct object_id *new_oid = >new_oid;
+   int do_update_worktree = 0;
 
/* only refs/... are allowed */
if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
@@ -1051,9 +1052,8 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
refuse_unconfigured_deny();
return "branch is currently checked out";
case DENY_UPDATE_INSTEAD:
-   ret = update_worktree(new_oid->hash);
-   if (ret)
-   return ret;
+   /* pass -- let other checks intervene first */
+   do_update_worktree = 1;
break;
}
}
@@ -1118,6 +1118,12 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
return "hook declined";
}
 
+   if (do_update_worktree) {
+   ret = update_worktree(new_oid->hash);
+   if (ret)
+   return ret;
+   }
+
if (is_null_oid(new_oid)) {
struct strbuf err = STRBUF_INIT;
if (!parse_object(the_repository, old_oid)) {
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 7a8f56db53..7316365a24 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1577,7 +1577,13 @@ test_expect_success 'receive.denyCurrentBranch = 
updateInstead' '
test $(git -C .. rev-parse master) = $(git rev-parse HEAD) &&
git diff --quiet &&
git diff --cached --quiet
-   )
+   ) &&
+
+   # (6) updateInstead intervened by fast-forward check
+   test_must_fail git push void master^:master &&
+   test $(git -C void rev-parse HEAD) = $(git rev-parse master) &&
+   git -C void diff --quiet &&
+   git -C void diff --cached --quiet
 '
 
 test_expect_success 'updateInstead with push-to-checkout hook' '
-- 
2.19.1-450-ga4b8ab5363



Re: [PATCH 2/2] t3430: update to test with custom commentChar

2018-10-02 Thread Johannes Schindelin
Hi Daniel,

[I forgot to address this mail earlier, my apologies]

On Tue, 10 Jul 2018, Daniel Harding wrote:

> On Tue, 10 Jul 2018 at 16:08:57 +0300, Johannes Schindelin wrote:>
> > On Tue, 10 Jul 2018, Daniel Harding wrote:
> > 
> > > On Mon, 09 Jul 2018 at 22:14:58 +0300, Johannes Schindelin wrote:
> > > >
> > > > On Mon, 9 Jul 2018, Daniel Harding wrote:
> > > > >
> > > > > On Mon, 09 Jul 2018 at 00:02:00 +0300, brian m. carlson wrote:
> > > > > >
> > > > > > Should this affect the "# Merge the topic branch" line (and the "#
> > > > > > C",
> > > > > > "# E", and "# H" lines in the next test) that appears below this?
> > > > > > It
> > > > > > would seem those would qualify as comments as well.
> > > > >
> > > > > I intentionally did not change that behavior for two reasons:
> > > > >
> > > > > a) from a Git perspective, comment characters are only effectual for
> > > > > comments
> > > > > if they are the first character in a line
> > > > >
> > > > > and
> > > > >
> > > > > b) there are places where a '#' character from the todo list is
> > > > > actually
> > > > > parsed and used e.g. [0] and [1].  I have not yet gotten to the point
> > > > > of
> > > > > grokking what is going on there, so I didn't want to risk breaking
> > > > > something I
> > > > > didn't understand.  Perhaps Johannes could shed some light on whether
> > > > > the
> > > > > cases you mentioned should be changed to use the configured
> > > > > commentChar or
> > > > > not.
> > > > >
> > > > > [0]
> > > > > https://github.com/git/git/blob/53f9a3e157dbbc901a02ac2c73346d375e24978c/sequencer.c#L2869
> > > > > [1]
> > > > > https://github.com/git/git/blob/53f9a3e157dbbc901a02ac2c73346d375e24978c/sequencer.c#L3797
> > > >
> > > > These are related. The first one tries to support
> > > >
> > > >   merge -C cafecafe second-branch third-branch # Octopus 2nd/3rd branch
> > > >
> > > > i.e. use '#' to separate between the commit(s) to merge and the oneline
> > > > (the latter for the reader's pleasure, just like the onelines in the
> > > > `pick
> > > >  ` lines.
> > > >
> > > > The second ensures that there is no valid label `#`.
> > > >
> > > > I have not really thought about the ramifications of changing this to
> > > > comment_line_char, but I guess it *could* work if both locations were
> > > > changed.
> > >
> > > Is there interest in such a change?  I'm happy to take a stab at it if
> > > there
> > > is, otherwise I'll leave things as they are.
> > 
> > I think it would be a fine change, once we convinced ourselves that it
> > does not break things (I am a little worried about this because I remember
> > just how long I had to reflect about the ramifications with regards to the
> > label: `#` is a valid ref name, after all, and that was the reason why I
> > had to treat it specially, and I wonder whether allowing arbitrary comment
> > chars will require us to add more such special handling that is not
> > necessary if we stick to `#`).
> 
> Would it simpler/safer to perhaps put the oneline on its own commented line
> above?  I know it isn't quite consistent with the way onelines are displayed
> for normal commits, but it might be a worthwhile tradeoff for the sake of the
> code.  As an idea of what I am suggesting, your example above would become
> perhaps
> 
> # Merge: Octopus 2nd/3rd branch
> merge -C cafecafe second-branch third-branch
> 
> or perhaps just
> 
> # Octopus 2nd/3rd branch
> merge -C cafecafe second-branch third-branch
> 
> Thoughts?

That is a very good idea, if you ask me! Unfortunately, I forgot about
this suggestion, and IIUC v2.19.0 shipped with my previously-suggested
version.

But maybe you want to spend a little time on the code to change it
according to your suggestion?

The `merge` commands are generated here:
https://github.com/git/git/blob/v2.19.0/sequencer.c#L4096-L4120

> > Not that the comment line char feature seems to be all that safe. I could
> > imagine that setting it to ' ' (i.e. a single space) wreaks havoc with
> > Git, and we have no safeguard to error out in this obviously broken case.
> 
> Technically, I think a single space might actually work with commit messages
> (at least, I can't off the top of my head think of a case where git would
> insert a non-comment line starting with a space if it wasn't already present
> in a commit message).  But if someone were actually crazy enough to do that I
> might suggest a diagnosis of "if it hurts, don't do that" rather than trying
> to equip git defend against that sort of thing.

I did want to have an extra special visual marker for users (such as
myself), so a space would not quite be enough. That's why I settled for
the comment char.

Ciao,
Johannes


[PATCH] submodule update --remote: introduce pinning

2018-10-01 Thread Stefan Beller
gitmodules(5) sayeth:

   submodule..branch
   A remote branch name for tracking updates in the upstream
   submodule. If the option is not specified, it defaults to master.

This doesn't allow having a "pinned" submodule that should not be updated
from upstream. We should change this to have no default --- if branch is
not specified, don't update that submodule, just like in Gerrit's
corresponding feature[1].

[1] 
https://gerrit-review.googlesource.com/Documentation/user-submodules.html#_defining_the_submodule_branch


However changing defaults is difficult as it may surprise the user,
Jonathan came up with a 4 step plan:
Step 0: introduce

# current behavior:
git submodule update --remote --remote-default-to-master

# new behavior:
git submodule update --remote --remote-pinned

and treat plain "git submodule update --remote" as --default-to-master.

Step 1: when neither --default-to-master nor --no-default-to-master
has been passed, warn when encountering a submodule with no branch
and treat it as "master".

Step 2: when neither --default-to-master nor --no-default-to-master
has been passed, error out when encountering a submodule with no
branch.

Step 3: when neither --default-to-master nor --no-default-to-master
has been passed, warn when encountering a submodule with no branch
and treat it as pinned.

Step 4: eliminate the warning.

This implements step 0 and 1.

Signed-off-by: Stefan Beller 
---

Sorry that this took so long, this is a rough patch for steps 0 & 1,
I'd still need to update docs.

Stefan


 Documentation/gitmodules.txt | 11 ++-
 builtin/submodule--helper.c  | 36 +---
 git-submodule.sh | 34 +-
 t/t7406-submodule-update.sh  | 29 +
 4 files changed, 89 insertions(+), 21 deletions(-)

diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 4d63def206..fe42dbdb3e 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -50,11 +50,12 @@ submodule..update::
 
 submodule..branch::
A remote branch name for tracking updates in the upstream submodule.
-   If the option is not specified, it defaults to 'master'.  A special
-   value of `.` is used to indicate that the name of the branch in the
-   submodule should be the same name as the current branch in the
-   current repository.  See the `--remote` documentation in
-   linkgit:git-submodule[1] for details.
+   A special value of `.` is used to indicate that the name of the
+   branch in the submodule should be the same name as the current
+   branch in the current repository.  See the `--remote` documentation
+   in linkgit:git-submodule[1] for details.
+   If not set, the default for `git submodule update --remote` is
+   to update the submodule to the superproject's recorded object id.
 
 submodule..fetchRecurseSubmodules::
This option can be used to control recursive fetching of this
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 40844870cf..6413f2b410 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1889,7 +1889,7 @@ static int resolve_relative_path(int argc, const char 
**argv, const char *prefix
return 0;
 }
 
-static const char *remote_submodule_branch(const char *path)
+static const char *remote_submodule_branch(const char *path, int 
default_pinned)
 {
const struct submodule *sub;
const char *branch = NULL;
@@ -1904,8 +1904,12 @@ static const char *remote_submodule_branch(const char 
*path)
branch = sub->branch;
free(key);
 
-   if (!branch)
-   return "master";
+   if (!branch) {
+   if (default_pinned)
+   return "";
+   else
+   return "master";
+   }
 
if (!strcmp(branch, ".")) {
const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
@@ -1932,12 +1936,30 @@ static int resolve_remote_submodule_branch(int argc, 
const char **argv,
 {
const char *ret;
struct strbuf sb = STRBUF_INIT;
-   if (argc != 2)
-   die("submodule--helper remote-branch takes exactly one 
arguments, got %d", argc);
+   int default_pinned = 0;
+
+   struct option remote_options[] = {
+   OPT_SET_INT(0, "default-master", _pinned,
+   N_("unconfigured submodules default to master 
branch"), 0),
+   OPT_SET_INT(0, "default-pinned", _pinned,
+   N_("unconfigured submodules default to 
superproject object id"), 1),
+   OPT_END()
+   };
+
+   const char *const git_submodule_helper_usage[] = {
+   N_("

Re: [PATCH v9 04/21] stash: update test cases conform to coding guidelines

2018-09-30 Thread Thomas Gummerer
> Subject: stash: update test cases conform to coding guidelines

s/stash/t3903/
s/conform/to &/

Alternatively the subject could also be "t3903: modernize style",
which would be a bit shorter, and still convey the same information to
a reader of 'git log --oneline'.

On 09/26, Paul-Sebastian Ungureanu wrote:
> Removed whitespaces after redirection operators.

s/Removed/Remove/.  Commit messages should always use the imperative
mood, as described in Documentation/SubmittingPatches.

> 
> Signed-off-by: Paul-Sebastian Ungureanu 
> ---
>  t/t3903-stash.sh | 120 ---
>  1 file changed, 61 insertions(+), 59 deletions(-)
> 
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index af7586d43d..de6cab1fe7 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -8,22 +8,22 @@ test_description='Test git stash'
>  . ./test-lib.sh
>  
>  test_expect_success 'stash some dirty working directory' '
> - echo 1 > file &&
> + echo 1 >file &&
>   git add file &&
>   echo unrelated >other-file &&
>   git add other-file &&
>   test_tick &&
>   git commit -m initial &&
> - echo 2 > file &&
> + echo 2 >file &&
>   git add file &&
> - echo 3 > file &&
> + echo 3 >file &&
>   test_tick &&
>   git stash &&
>   git diff-files --quiet &&
>   git diff-index --cached --quiet HEAD
>  '
>  
> -cat > expect << EOF
> +cat >expect <  diff --git a/file b/file
>  index 0cfbf08..00750ed 100644
>  --- a/file
> @@ -35,7 +35,7 @@ EOF
>  
>  test_expect_success 'parents of stash' '
>   test $(git rev-parse stash^) = $(git rev-parse HEAD) &&
> - git diff stash^2..stash > output &&
> + git diff stash^2..stash >output &&
>   test_cmp output expect
>  '
>  
> @@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' '
>  
>  test_expect_success 'apply stashed changes (including index)' '
>   git reset --hard HEAD^ &&
> - echo 6 > other-file &&
> + echo 6 >other-file &&
>   git add other-file &&
>   test_tick &&
>   git commit -m other-file &&
> @@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra 
> options' '
>  
>  test_expect_success 'drop top stash' '
>   git reset --hard &&
> - git stash list > stashlist1 &&
> - echo 7 > file &&
> + git stash list >expected &&
> + echo 7 >file &&
>   git stash &&
>   git stash drop &&
> - git stash list > stashlist2 &&
> - test_cmp stashlist1 stashlist2 &&
> + git stash list >actual &&
> + test_cmp expected actual &&
>   git stash apply &&
>   test 3 = $(cat file) &&
>   test 1 = $(git show :file) &&
> @@ -113,9 +113,9 @@ test_expect_success 'drop top stash' '
>  
>  test_expect_success 'drop middle stash' '
>   git reset --hard &&
> - echo 8 > file &&
> + echo 8 >file &&
>   git stash &&
> - echo 9 > file &&
> + echo 9 >file &&
>   git stash &&
>   git stash drop stash@{1} &&
>   test 2 = $(git stash list | wc -l) &&
> @@ -160,7 +160,7 @@ test_expect_success 'stash pop' '
>   test 0 = $(git stash list | wc -l)
>  '
>  
> -cat > expect << EOF
> +cat >expect <  diff --git a/file2 b/file2
>  new file mode 100644
>  index 000..1fe912c
> @@ -170,7 +170,7 @@ index 000..1fe912c
>  +bar2
>  EOF
>  
> -cat > expect1 << EOF
> +cat >expect1 <  diff --git a/file b/file
>  index 257cc56..5716ca5 100644
>  --- a/file
> @@ -180,7 +180,7 @@ index 257cc56..5716ca5 100644
>  +bar
>  EOF
>  
> -cat > expect2 << EOF
> +cat >expect2 <  diff --git a/file b/file
>  index 7601807..5716ca5 100644
>  --- a/file
> @@ -198,79 +198,79 @@ index 000..1fe912c
>  EOF
>  
>  test_expect_success 'stash branch' '
> - echo foo > file &&
> + echo foo >file &&
>   git commit file -m first &&
> - echo bar > file &&
> - echo bar2 > file2 &&
> + echo bar >file &&
> + echo bar2 >file2 &&
>   git add file2 &&
>   git stash &&
> - echo baz > file &&
> + echo 

Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread Junio C Hamano
Ben Peart  writes:

> Junio, can you squash in the following patch or would you prefer I
> reroll the entire series?

Squash it to f8cd77d5 ("fsmonitor: update GIT_TEST_FSMONITOR
support", 2018-09-18) and use the two new lines in the log message?

I can do that.

Thanks.


Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread Ben Peart




On 9/28/2018 10:21 AM, Ben Peart wrote:



On 9/28/2018 6:01 AM, SZEDER Gábor wrote:

On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote:

diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the 
uncommon pack-objects code

  path where deltas larger than this limit require extra memory
  allocation for bookkeeping.
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.


Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we
are good to go.


diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
  # To run the entire git test suite using fsmonitor:
  #
  # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.


But this old comment is different, suggesting copying that script to
our $PATH.

I prefer your instructions above, because it's only a single step,
and, more importantly, it won't pollute my $PATH.  I think this
comment should be updated to make the advices in both places
consistent.  Or perhaps even removed, now that all GIT_TEST variables
are documented in the same place?



I prefer the suggestion to simply remove this text from the test script 
now that there is documentation for it in the t/README file.


Junio, can you squash in the following patch or would you prefer I 
reroll the entire series?


Thanks,

Ben

From 393007340dc1baf3539ab727e0a8128e7c408a27 Mon Sep 17 00:00:00 2001
From: Ben Peart 
Date: Fri, 28 Sep 2018 10:23:18 -0400
Subject: fixup! fsmonitor: remove outdated instructions from test

Remove the outdated instructions on how to run the test suite utilizing
fsmonitor now that it is properly documented in t/README.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: git-test-cleanup-v3
Web-Diff: https://github.com/benpeart/git/commit/393007340d
Checkout: git fetch https://github.com/benpeart/git 
git-test-cleanup-v1 && git checkout 393007340d


 t/t7519-status-fsmonitor.sh | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 8308d6d5b1..3f0dd98010 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -4,13 +4,6 @@ test_description='git status with file system watcher'

 . ./test-lib.sh

-#
-# To run the entire git test suite using fsmonitor:
-#
-# copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
-#
-
 # Note, after "git reset --hard HEAD" no extensions exist other than 
'TREE'

 # "git update-index --fsmonitor" can be used to get the extension written
 # before testing the results.

base-commit: 043246d9369fb851c5c2b922466f77fc7ef0327b
--
2.18.0.windows.1



Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread Ben Peart




On 9/28/2018 6:01 AM, SZEDER Gábor wrote:

On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote:

diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
pack-objects code
  path where deltas larger than this limit require extra memory
  allocation for bookkeeping.
  
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor

+code path for utilizing a file system monitor to speed up detecting
+new or changed files.


Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we
are good to go.


diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
  # To run the entire git test suite using fsmonitor:
  #
  # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.


But this old comment is different, suggesting copying that script to
our $PATH.

I prefer your instructions above, because it's only a single step,
and, more importantly, it won't pollute my $PATH.  I think this
comment should be updated to make the advices in both places
consistent.  Or perhaps even removed, now that all GIT_TEST variables
are documented in the same place?



I prefer the suggestion to simply remove this text from the test script 
now that there is documentation for it in the t/README file.


Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread SZEDER Gábor
On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote:
> diff --git a/t/README b/t/README
> index 56a417439c..47165f7eab 100644
> --- a/t/README
> +++ b/t/README
> @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
> pack-objects code
>  path where deltas larger than this limit require extra memory
>  allocation for bookkeeping.
>  
> +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
> +code path for utilizing a file system monitor to speed up detecting
> +new or changed files.

Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we
are good to go.

> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> index 756beb0d8e..d77012ea6d 100755
> --- a/t/t7519-status-fsmonitor.sh
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -8,7 +8,7 @@ test_description='git status with file system watcher'
>  # To run the entire git test suite using fsmonitor:
>  #
>  # copy t/t7519/fsmonitor-all to a location in your path and then set
> -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
> +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.

But this old comment is different, suggesting copying that script to
our $PATH.

I prefer your instructions above, because it's only a single step,
and, more importantly, it won't pollute my $PATH.  I think this
comment should be updated to make the advices in both places
consistent.  Or perhaps even removed, now that all GIT_TEST variables
are documented in the same place?



Re: [PATCH 1/1] read-cache: update index format default to v4

2018-09-26 Thread Matthias Sohn
On Tue, Sep 25, 2018 at 8:01 PM Stefan Beller  wrote:
>
> On Tue, Sep 25, 2018 at 7:30 AM Derrick Stolee  wrote:
> >
> > On 9/25/2018 3:06 AM, Patrick Steinhardt wrote:
> > > On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote:
> > >> On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via 
> > >> GitGitGadget wrote:
> > >>> From: Derrick Stolee 
> > >>>
> > >>> The index v4 format has been available since 2012 with 9d22778
> > >>> "reach-cache.c: write prefix-compressed names in the index". Since
> > >>> the format has been stable for so long, almost all versions of Git
> > >>> in use today understand version 4, removing one barrier to upgrade
> > >>> -- that someone may want to downgrade and needs a working repo.
> > >> What about alternative implementations, like JGit, libgit2, etc.?
> > > Speaking of libgit2, we are able to read and write index v4 since
> > > commit c1b370e93
> >
> > This is a good point, Szeder.
> >
> > Patrick: I'm glad LibGit2 is up-to-date with index formats.
> >
> > Unfortunately, taking a look (for the first time) at the JGit code
> > reveals that they don't appear to have v4 support. In
> > org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the
> > DirCache.readFrom() method: lines 488-494, I see the following snippet:
> >
> >  final int ver = NB.decodeInt32(hdr, 4);
> >  boolean extended = false;
> >  if (ver == 3)
> >  extended = true;
> >  else if (ver != 2)
> >  throw new
> > CorruptObjectException(MessageFormat.format(
> > JGitText.get().unknownDIRCVersion, Integer.valueOf(ver)));
> >
> > It looks like this will immediately throw with versions other than 2 or 3.
> >
> > I'm adding Jonathan Nieder to CC so he can check with JGit people about
> > the impact of this change.
>
> JGit is used both on the server (which doesn't use index/staging area)
> as well as client side as e.g. an Eclipse integration, which would
> very much like to use the index.
>
> Adding Matthias Sohn as well, who is active in JGit and cares
> more about the client side than Googlers who only make use
> of the server side part of JGit.

thanks for the heads up, in fact JGit does not yet support index version 4.
I will look into this.

-Matthias


[PATCH v9 04/21] stash: update test cases conform to coding guidelines

2018-09-25 Thread Paul-Sebastian Ungureanu
Removed whitespaces after redirection operators.

Signed-off-by: Paul-Sebastian Ungureanu 
---
 t/t3903-stash.sh | 120 ---
 1 file changed, 61 insertions(+), 59 deletions(-)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index af7586d43d..de6cab1fe7 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -8,22 +8,22 @@ test_description='Test git stash'
 . ./test-lib.sh
 
 test_expect_success 'stash some dirty working directory' '
-   echo 1 > file &&
+   echo 1 >file &&
git add file &&
echo unrelated >other-file &&
git add other-file &&
test_tick &&
git commit -m initial &&
-   echo 2 > file &&
+   echo 2 >file &&
git add file &&
-   echo 3 > file &&
+   echo 3 >file &&
test_tick &&
git stash &&
git diff-files --quiet &&
git diff-index --cached --quiet HEAD
 '
 
-cat > expect << EOF
+cat >expect < output &&
+   git diff stash^2..stash >output &&
test_cmp output expect
 '
 
@@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' '
 
 test_expect_success 'apply stashed changes (including index)' '
git reset --hard HEAD^ &&
-   echo 6 > other-file &&
+   echo 6 >other-file &&
git add other-file &&
test_tick &&
git commit -m other-file &&
@@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra options' 
'
 
 test_expect_success 'drop top stash' '
git reset --hard &&
-   git stash list > stashlist1 &&
-   echo 7 > file &&
+   git stash list >expected &&
+   echo 7 >file &&
git stash &&
git stash drop &&
-   git stash list > stashlist2 &&
-   test_cmp stashlist1 stashlist2 &&
+   git stash list >actual &&
+   test_cmp expected actual &&
git stash apply &&
test 3 = $(cat file) &&
test 1 = $(git show :file) &&
@@ -113,9 +113,9 @@ test_expect_success 'drop top stash' '
 
 test_expect_success 'drop middle stash' '
git reset --hard &&
-   echo 8 > file &&
+   echo 8 >file &&
git stash &&
-   echo 9 > file &&
+   echo 9 >file &&
git stash &&
git stash drop stash@{1} &&
test 2 = $(git stash list | wc -l) &&
@@ -160,7 +160,7 @@ test_expect_success 'stash pop' '
test 0 = $(git stash list | wc -l)
 '
 
-cat > expect << EOF
+cat >expect < expect1 << EOF
+cat >expect1 < expect2 << EOF
+cat >expect2 < file &&
+   echo foo >file &&
git commit file -m first &&
-   echo bar > file &&
-   echo bar2 > file2 &&
+   echo bar >file &&
+   echo bar2 >file2 &&
git add file2 &&
git stash &&
-   echo baz > file &&
+   echo baz >file &&
git commit file -m second &&
git stash branch stashbranch &&
test refs/heads/stashbranch = $(git symbolic-ref HEAD) &&
test $(git rev-parse HEAD) = $(git rev-parse master^) &&
-   git diff --cached > output &&
+   git diff --cached >output &&
test_cmp output expect &&
-   git diff > output &&
+   git diff >output &&
test_cmp output expect1 &&
git add file &&
git commit -m alternate\ second &&
-   git diff master..stashbranch > output &&
+   git diff master..stashbranch >output &&
test_cmp output expect2 &&
test 0 = $(git stash list | wc -l)
 '
 
 test_expect_success 'apply -q is quiet' '
-   echo foo > file &&
+   echo foo >file &&
git stash &&
-   git stash apply -q > output.out 2>&1 &&
+   git stash apply -q >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'save -q is quiet' '
-   git stash save --quiet > output.out 2>&1 &&
+   git stash save --quiet >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'pop -q is quiet' '
-   git stash pop -q > output.out 2>&1 &&
+   git stash pop -q >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'pop -q --index works and is quiet' '
-   echo foo > file &&
+   echo foo >file &&
git add file &&
git stash save --quiet &&
-   git stash pop -q --index > output.out 2>&1 &&
+   git stash pop -q --index >output.out 2>&1 &&
test foo = "$(git show :file)" &&
test_must_be_empty output.out
 '
 
 test_expect_success 'drop -q is quiet' '
git stash &&
-   git stash drop -q > output.out 2>&1 &&
+   git stash drop -q >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'stash -k' '
-   echo bar3 > file &&
-   echo bar4 > file2 &&
+   echo bar3 >file &&
+   echo bar4 >file2 &&
git add file2 &&
git stash -k &&
test bar,bar4 = $(cat file),$(cat file2)
 '
 
 test_expect_success 'stash --no-keep-index' '
-   echo bar33 > file &&
-   echo bar44 > file2 &&
+   echo bar33 >file &&
+   echo 

[GSoC][PATCH v9 03/21] stash: update test cases conform to coding guidelines

2018-09-25 Thread Paul-Sebastian Ungureanu
Removed whitespaces after redirection operators.

Signed-off-by: Paul-Sebastian Ungureanu 
---
 t/t3903-stash.sh | 120 ---
 1 file changed, 61 insertions(+), 59 deletions(-)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index af7586d43d..de6cab1fe7 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -8,22 +8,22 @@ test_description='Test git stash'
 . ./test-lib.sh
 
 test_expect_success 'stash some dirty working directory' '
-   echo 1 > file &&
+   echo 1 >file &&
git add file &&
echo unrelated >other-file &&
git add other-file &&
test_tick &&
git commit -m initial &&
-   echo 2 > file &&
+   echo 2 >file &&
git add file &&
-   echo 3 > file &&
+   echo 3 >file &&
test_tick &&
git stash &&
git diff-files --quiet &&
git diff-index --cached --quiet HEAD
 '
 
-cat > expect << EOF
+cat >expect < output &&
+   git diff stash^2..stash >output &&
test_cmp output expect
 '
 
@@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' '
 
 test_expect_success 'apply stashed changes (including index)' '
git reset --hard HEAD^ &&
-   echo 6 > other-file &&
+   echo 6 >other-file &&
git add other-file &&
test_tick &&
git commit -m other-file &&
@@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra options' 
'
 
 test_expect_success 'drop top stash' '
git reset --hard &&
-   git stash list > stashlist1 &&
-   echo 7 > file &&
+   git stash list >expected &&
+   echo 7 >file &&
git stash &&
git stash drop &&
-   git stash list > stashlist2 &&
-   test_cmp stashlist1 stashlist2 &&
+   git stash list >actual &&
+   test_cmp expected actual &&
git stash apply &&
test 3 = $(cat file) &&
test 1 = $(git show :file) &&
@@ -113,9 +113,9 @@ test_expect_success 'drop top stash' '
 
 test_expect_success 'drop middle stash' '
git reset --hard &&
-   echo 8 > file &&
+   echo 8 >file &&
git stash &&
-   echo 9 > file &&
+   echo 9 >file &&
git stash &&
git stash drop stash@{1} &&
test 2 = $(git stash list | wc -l) &&
@@ -160,7 +160,7 @@ test_expect_success 'stash pop' '
test 0 = $(git stash list | wc -l)
 '
 
-cat > expect << EOF
+cat >expect < expect1 << EOF
+cat >expect1 < expect2 << EOF
+cat >expect2 < file &&
+   echo foo >file &&
git commit file -m first &&
-   echo bar > file &&
-   echo bar2 > file2 &&
+   echo bar >file &&
+   echo bar2 >file2 &&
git add file2 &&
git stash &&
-   echo baz > file &&
+   echo baz >file &&
git commit file -m second &&
git stash branch stashbranch &&
test refs/heads/stashbranch = $(git symbolic-ref HEAD) &&
test $(git rev-parse HEAD) = $(git rev-parse master^) &&
-   git diff --cached > output &&
+   git diff --cached >output &&
test_cmp output expect &&
-   git diff > output &&
+   git diff >output &&
test_cmp output expect1 &&
git add file &&
git commit -m alternate\ second &&
-   git diff master..stashbranch > output &&
+   git diff master..stashbranch >output &&
test_cmp output expect2 &&
test 0 = $(git stash list | wc -l)
 '
 
 test_expect_success 'apply -q is quiet' '
-   echo foo > file &&
+   echo foo >file &&
git stash &&
-   git stash apply -q > output.out 2>&1 &&
+   git stash apply -q >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'save -q is quiet' '
-   git stash save --quiet > output.out 2>&1 &&
+   git stash save --quiet >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'pop -q is quiet' '
-   git stash pop -q > output.out 2>&1 &&
+   git stash pop -q >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'pop -q --index works and is quiet' '
-   echo foo > file &&
+   echo foo >file &&
git add file &&
git stash save --quiet &&
-   git stash pop -q --index > output.out 2>&1 &&
+   git stash pop -q --index >output.out 2>&1 &&
test foo = "$(git show :file)" &&
test_must_be_empty output.out
 '
 
 test_expect_success 'drop -q is quiet' '
git stash &&
-   git stash drop -q > output.out 2>&1 &&
+   git stash drop -q >output.out 2>&1 &&
test_must_be_empty output.out
 '
 
 test_expect_success 'stash -k' '
-   echo bar3 > file &&
-   echo bar4 > file2 &&
+   echo bar3 >file &&
+   echo bar4 >file2 &&
git add file2 &&
git stash -k &&
test bar,bar4 = $(cat file),$(cat file2)
 '
 
 test_expect_success 'stash --no-keep-index' '
-   echo bar33 > file &&
-   echo bar44 > file2 &&
+   echo bar33 >file &&
+   echo 

Re: [PATCH 1/1] read-cache: update index format default to v4

2018-09-25 Thread Ben Peart




On 9/25/2018 2:01 PM, Stefan Beller wrote:

On Tue, Sep 25, 2018 at 7:30 AM Derrick Stolee  wrote:


On 9/25/2018 3:06 AM, Patrick Steinhardt wrote:

On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote:

On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget wrote:

From: Derrick Stolee 

The index v4 format has been available since 2012 with 9d22778
"reach-cache.c: write prefix-compressed names in the index". Since
the format has been stable for so long, almost all versions of Git
in use today understand version 4, removing one barrier to upgrade
-- that someone may want to downgrade and needs a working repo.

What about alternative implementations, like JGit, libgit2, etc.?

Speaking of libgit2, we are able to read and write index v4 since
commit c1b370e93


This is a good point, Szeder.

Patrick: I'm glad LibGit2 is up-to-date with index formats.

Unfortunately, taking a look (for the first time) at the JGit code
reveals that they don't appear to have v4 support. In
org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the
DirCache.readFrom() method: lines 488-494, I see the following snippet:

  final int ver = NB.decodeInt32(hdr, 4);
  boolean extended = false;
  if (ver == 3)
  extended = true;
  else if (ver != 2)
  throw new
CorruptObjectException(MessageFormat.format(
JGitText.get().unknownDIRCVersion, Integer.valueOf(ver)));

It looks like this will immediately throw with versions other than 2 or 3.

I'm adding Jonathan Nieder to CC so he can check with JGit people about
the impact of this change.


JGit is used both on the server (which doesn't use index/staging area)
as well as client side as e.g. an Eclipse integration, which would
very much like to use the index.

Adding Matthias Sohn as well, who is active in JGit and cares
more about the client side than Googlers who only make use
of the server side part of JGit.

Stefan



In addition to the compatibility with other implementations of git 
question, I think we should include Duy's patch [1] to "optimize reading 
index format v4" before flipping the default to V4.


In my testing, this reduced the index load time of V4 by 10%-15% making 
the CPU cost only ~2% more expensive than V2 or V3 indexes.  This 
increase in CPU cost is more than offset by the significant reduction in 
file IO due to the reduced index file size.


[1] https://public-inbox.org/git/20180825064458.28484-1-pclo...@gmail.com/


Re: [PATCH 1/1] read-cache: update index format default to v4

2018-09-25 Thread Stefan Beller
On Tue, Sep 25, 2018 at 7:30 AM Derrick Stolee  wrote:
>
> On 9/25/2018 3:06 AM, Patrick Steinhardt wrote:
> > On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote:
> >> On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget 
> >> wrote:
> >>> From: Derrick Stolee 
> >>>
> >>> The index v4 format has been available since 2012 with 9d22778
> >>> "reach-cache.c: write prefix-compressed names in the index". Since
> >>> the format has been stable for so long, almost all versions of Git
> >>> in use today understand version 4, removing one barrier to upgrade
> >>> -- that someone may want to downgrade and needs a working repo.
> >> What about alternative implementations, like JGit, libgit2, etc.?
> > Speaking of libgit2, we are able to read and write index v4 since
> > commit c1b370e93
>
> This is a good point, Szeder.
>
> Patrick: I'm glad LibGit2 is up-to-date with index formats.
>
> Unfortunately, taking a look (for the first time) at the JGit code
> reveals that they don't appear to have v4 support. In
> org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the
> DirCache.readFrom() method: lines 488-494, I see the following snippet:
>
>  final int ver = NB.decodeInt32(hdr, 4);
>  boolean extended = false;
>  if (ver == 3)
>  extended = true;
>  else if (ver != 2)
>  throw new
> CorruptObjectException(MessageFormat.format(
> JGitText.get().unknownDIRCVersion, Integer.valueOf(ver)));
>
> It looks like this will immediately throw with versions other than 2 or 3.
>
> I'm adding Jonathan Nieder to CC so he can check with JGit people about
> the impact of this change.

JGit is used both on the server (which doesn't use index/staging area)
as well as client side as e.g. an Eclipse integration, which would
very much like to use the index.

Adding Matthias Sohn as well, who is active in JGit and cares
more about the client side than Googlers who only make use
of the server side part of JGit.

Stefan


Re: [PATCH 1/1] read-cache: update index format default to v4

2018-09-25 Thread Derrick Stolee

On 9/25/2018 3:06 AM, Patrick Steinhardt wrote:

On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote:

On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget wrote:

From: Derrick Stolee 

The index v4 format has been available since 2012 with 9d22778
"reach-cache.c: write prefix-compressed names in the index". Since
the format has been stable for so long, almost all versions of Git
in use today understand version 4, removing one barrier to upgrade
-- that someone may want to downgrade and needs a working repo.

What about alternative implementations, like JGit, libgit2, etc.?

Speaking of libgit2, we are able to read and write index v4 since
commit c1b370e93


This is a good point, Szeder.

Patrick: I'm glad LibGit2 is up-to-date with index formats.

Unfortunately, taking a look (for the first time) at the JGit code 
reveals that they don't appear to have v4 support. In 
org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the 
DirCache.readFrom() method: lines 488-494, I see the following snippet:


    final int ver = NB.decodeInt32(hdr, 4);
    boolean extended = false;
    if (ver == 3)
    extended = true;
    else if (ver != 2)
    throw new 
CorruptObjectException(MessageFormat.format(

JGitText.get().unknownDIRCVersion, Integer.valueOf(ver)));

It looks like this will immediately throw with versions other than 2 or 3.

I'm adding Jonathan Nieder to CC so he can check with JGit people about 
the impact of this change.


Thanks,

-Stolee




Re: [PATCH 1/1] read-cache: update index format default to v4

2018-09-25 Thread Patrick Steinhardt
On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote:
> On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget 
> wrote:
> > From: Derrick Stolee 
> > 
> > The index v4 format has been available since 2012 with 9d22778
> > "reach-cache.c: write prefix-compressed names in the index". Since
> > the format has been stable for so long, almost all versions of Git
> > in use today understand version 4, removing one barrier to upgrade
> > -- that someone may want to downgrade and needs a working repo.
> 
> What about alternative implementations, like JGit, libgit2, etc.?

Speaking of libgit2, we are able to read and write index v4 since
commit c1b370e93 (Merge pull request #3837 from
novalis/dturner/indexv4, 2016-08-17), released with v0.25 in
December 2016. Due to insufficient tests, our read support was
initially broken, which got fixed with commit 3bc95cfe3 (Merge
pull request #4236 from pks-t/pks/index-v4-fixes, 2017-06-07) and
released with v0.26 in June 2017.

Right now I'm not aware of additional bugs in our index v4
support, so we'd be fine with changing the default.

Patrick

> > Despite being stable for a long time, this index version was never
> > adopted as the default. This prefix-compressed version of the format
> > can get significant space savings on repos with large working
> > directories (which naturally tend to have deep nesting). This version
> > is set as the default for some external tools, such as VFS for Git.
> > Because of this external use, the format has had a lot of "testing in
> > production" and also is subject to continuous integration in these
> > environments.
> > 
> > Previously, to test version 4 indexes, we needed to run the test
> > suite with GIT_TEST_INDEX_VERSION=4 (or TEST_GIT_INDEX_VERSION=4).
> > 
> > One potential, but short-term, downside is that we lose coverage of
> > the version 3 indexes. The trade-off is that we may want to cover
> > that version using GIT_TEST_INDEX_VERSION=3.
> > 
> > Signed-off-by: Derrick Stolee 
> > ---
> >  read-cache.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/read-cache.c b/read-cache.c
> > index 372588260e..af6c8f2a67 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -1484,7 +1484,7 @@ struct cache_entry *refresh_cache_entry(struct 
> > cache_entry *ce,
> >   * Index File I/O
> >   */
> >  
> > -#define INDEX_FORMAT_DEFAULT 3
> > +#define INDEX_FORMAT_DEFAULT 4
> >  
> >  static unsigned int get_index_format_default(void)
> >  {
> > -- 
> > gitgitgadget


signature.asc
Description: PGP signature


Re: [PATCH 1/1] read-cache: update index format default to v4

2018-09-24 Thread Junio C Hamano
SZEDER Gábor  writes:

> On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget 
> wrote:
>> From: Derrick Stolee 
>> 
>> The index v4 format has been available since 2012 with 9d22778
>> "reach-cache.c: write prefix-compressed names in the index". Since
>> the format has been stable for so long, almost all versions of Git
>> in use today understand version 4, removing one barrier to upgrade
>> -- that someone may want to downgrade and needs a working repo.
>
> What about alternative implementations, like JGit, libgit2, etc.?

Good question.

Because the index-version of an index file is designed to be sticky,
repos that need to be accessed by other implementations can keep
whatever current version.  A new repo that need to be accessed by
them can be (forcibly) written in v2 and keep its v2ness, I would
think.

And that would serve as an incentive for the implementations to
catch up ;-)





Re: [PATCH 1/1] read-cache: update index format default to v4

2018-09-24 Thread SZEDER Gábor
On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee 
> 
> The index v4 format has been available since 2012 with 9d22778
> "reach-cache.c: write prefix-compressed names in the index". Since
> the format has been stable for so long, almost all versions of Git
> in use today understand version 4, removing one barrier to upgrade
> -- that someone may want to downgrade and needs a working repo.

What about alternative implementations, like JGit, libgit2, etc.?

> Despite being stable for a long time, this index version was never
> adopted as the default. This prefix-compressed version of the format
> can get significant space savings on repos with large working
> directories (which naturally tend to have deep nesting). This version
> is set as the default for some external tools, such as VFS for Git.
> Because of this external use, the format has had a lot of "testing in
> production" and also is subject to continuous integration in these
> environments.
> 
> Previously, to test version 4 indexes, we needed to run the test
> suite with GIT_TEST_INDEX_VERSION=4 (or TEST_GIT_INDEX_VERSION=4).
> 
> One potential, but short-term, downside is that we lose coverage of
> the version 3 indexes. The trade-off is that we may want to cover
> that version using GIT_TEST_INDEX_VERSION=3.
> 
> Signed-off-by: Derrick Stolee 
> ---
>  read-cache.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/read-cache.c b/read-cache.c
> index 372588260e..af6c8f2a67 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1484,7 +1484,7 @@ struct cache_entry *refresh_cache_entry(struct 
> cache_entry *ce,
>   * Index File I/O
>   */
>  
> -#define INDEX_FORMAT_DEFAULT 3
> +#define INDEX_FORMAT_DEFAULT 4
>  
>  static unsigned int get_index_format_default(void)
>  {
> -- 
> gitgitgadget


[PATCH 1/1] read-cache: update index format default to v4

2018-09-24 Thread Derrick Stolee via GitGitGadget
From: Derrick Stolee 

The index v4 format has been available since 2012 with 9d22778
"reach-cache.c: write prefix-compressed names in the index". Since
the format has been stable for so long, almost all versions of Git
in use today understand version 4, removing one barrier to upgrade
-- that someone may want to downgrade and needs a working repo.

Despite being stable for a long time, this index version was never
adopted as the default. This prefix-compressed version of the format
can get significant space savings on repos with large working
directories (which naturally tend to have deep nesting). This version
is set as the default for some external tools, such as VFS for Git.
Because of this external use, the format has had a lot of "testing in
production" and also is subject to continuous integration in these
environments.

Previously, to test version 4 indexes, we needed to run the test
suite with GIT_TEST_INDEX_VERSION=4 (or TEST_GIT_INDEX_VERSION=4).

One potential, but short-term, downside is that we lose coverage of
the version 3 indexes. The trade-off is that we may want to cover
that version using GIT_TEST_INDEX_VERSION=3.

Signed-off-by: Derrick Stolee 
---
 read-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index 372588260e..af6c8f2a67 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1484,7 +1484,7 @@ struct cache_entry *refresh_cache_entry(struct 
cache_entry *ce,
  * Index File I/O
  */
 
-#define INDEX_FORMAT_DEFAULT 3
+#define INDEX_FORMAT_DEFAULT 4
 
 static unsigned int get_index_format_default(void)
 {
-- 
gitgitgadget


[PATCH 0/1] read-cache: update index format default to v4

2018-09-24 Thread Derrick Stolee via GitGitGadget
After discussing this with several people off-list, I thought I would open
the question up to the list:

Should we update the default index version to v4?

The .git/index file stores the list of every path tracked by Git in the
working directory, including merge information, staged paths, and
information about the file system contents (based on modified time). The
only major update in v4 is that the paths are prefix-compressed. This
compression works best in repos with a lot of paths, especially deep paths.
For this reason, we set the index to v4 in VFS for Git.

Among VFS for Git contributors, we were talking about how the v4 format is
not covered by the test suite by default. We are working to increase the
number of CI builds that set extra GIT_TEST_* variables that we need.
However, I thought it worth having a discussion of whether this is a good
thing to recommend for all users of Git.

Personally, I'm not an expert here, but I am happy to start the
conversation.

Thanks, -Stolee

Derrick Stolee (1):
  read-cache: update index format default to v4

 read-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 53f9a3e157dbbc901a02ac2c73346d375e24978c
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-41%2Fderrickstolee%2Findex-v4-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-41/derrickstolee/index-v4-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/41
-- 
gitgitgadget


Re: [PATCH] receive-pack: update comment with check_everything_connected

2018-09-24 Thread Junio C Hamano
Jeff King  writes:

> That function is now called "check_connected()", but we forgot to update
> this comment in 7043c7071c (check_everything_connected: use a struct
> with named options, 2016-07-15).
>
> Signed-off-by: Jeff King 
> ---
> Just a minor annoyance I happened to notice while discussing in another
> thread. I notice both of us still called it check-everything-connected
> in our emails; old habits die hard, I suppose. ;)

Yup, and now I think I caught up ;-)  Thanks.

>
>  builtin/receive-pack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index a3bb13af10..3b7432c8e4 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1834,7 +1834,7 @@ static void prepare_shallow_update(struct command 
> *commands,
>   /*
>* keep hooks happy by forcing a temporary shallow file via
>* env variable because we can't add --shallow-file to every
> -  * command. check_everything_connected() will be done with
> +  * command. check_connected() will be done with
>* true .git/shallow though.
>*/
>   setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1);


[PATCH] receive-pack: update comment with check_everything_connected

2018-09-21 Thread Jeff King
That function is now called "check_connected()", but we forgot to update
this comment in 7043c7071c (check_everything_connected: use a struct
with named options, 2016-07-15).

Signed-off-by: Jeff King 
---
Just a minor annoyance I happened to notice while discussing in another
thread. I notice both of us still called it check-everything-connected
in our emails; old habits die hard, I suppose. ;)

 builtin/receive-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a3bb13af10..3b7432c8e4 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1834,7 +1834,7 @@ static void prepare_shallow_update(struct command 
*commands,
/*
 * keep hooks happy by forcing a temporary shallow file via
 * env variable because we can't add --shallow-file to every
-* command. check_everything_connected() will be done with
+* command. check_connected() will be done with
 * true .git/shallow though.
 */
setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1);
-- 
2.19.0.762.gbd1f43ccbf


[PATCH v3 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support

2018-09-18 Thread Ben Peart
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with
the other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.

Signed-off-by: Ben Peart 
---
 preload-index.c | 2 +-
 t/README| 3 +++
 t/t7519-status-fsmonitor.sh | 4 ++--
 t/test-lib.sh   | 1 +
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/preload-index.c b/preload-index.c
index 0a4e2933bb..a850e197c2 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -85,7 +85,7 @@ static void preload_index(struct index_state *index,
return;
 
threads = index->cache_nr / THREAD_COST;
-   if ((index->cache_nr > 1) && (threads < 2) && 
git_env_bool("GIT_FORCE_PRELOAD_TEST", 0))
+   if ((index->cache_nr > 1) && (threads < 2) && 
git_env_bool("GIT_TEST_PRELOAD_INDEX", 0))
threads = 2;
if (threads < 2)
return;
diff --git a/t/README b/t/README
index 9b13f6d12e..5670c7aad0 100644
--- a/t/README
+++ b/t/README
@@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write 
code path
 for the index version specified.  Can be set to any valid version
 (currently 2, 3, or 4).
 
+GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path
+by overriding the minimum number of cache entries required per thread.
+
 Naming Tests
 
 
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,9 @@ do
git config core.preloadIndex $preload_val &&
if test $preload_val = true
then
-   GIT_FORCE_PRELOAD_TEST=$preload_val; export 
GIT_FORCE_PRELOAD_TEST
+   GIT_TEST_PRELOAD_INDEX=$preload_val; export 
GIT_TEST_PRELOAD_INDEX
else
-   unset GIT_FORCE_PRELOAD_TEST
+   sane_unset GIT_TEST_PRELOAD_INDEX
fi
'
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e80c84d13c..8ef86e05a3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -154,6 +154,7 @@ check_var_migration () {
 
 check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
 check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
+check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
 
 # Use specific version of the index file format
 if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
-- 
2.18.0.windows.1



[PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-18 Thread Ben Peart
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.

Signed-off-by: Ben Peart 
---
 config.c|  2 +-
 t/README|  4 
 t/t1700-split-index.sh  |  2 +-
 t/t7519-status-fsmonitor.sh |  2 +-
 t/test-lib.sh   | 20 
 5 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
 int git_config_get_fsmonitor(void)
 {
if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
-   core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+   core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
 
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
 Naming Tests
 
 
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b3b4d83eaf..f6a856f24c 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -6,7 +6,7 @@ test_description='split index mode tests'
 
 # We need total control of index splitting here
 sane_unset GIT_TEST_SPLIT_INDEX
-sane_unset GIT_FSMONITOR_TEST
+sane_unset GIT_TEST_FSMONITOR
 
 test_expect_success 'enable split index' '
git config splitIndex.maxPercentChange 100 &&
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # To run the entire git test suite using fsmonitor:
 #
 # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..653688c067 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,6 +140,26 @@ then
export GIT_INDEX_VERSION
 fi
 
+check_var_migration () {
+   old_name=$1 new_name=$2
+   eval "old_isset=\${${old_name}:+isset}"
+   eval "new_isset=\${${new_name}:+isset}"
+   case "$old_isset,$new_isset" in
+   isset,)
+   echo >&2 "warning: $old_name is now $new_name"
+   echo >&2 "hint: set $new_name too during the transition period"
+   eval "$new_name=\$$old_name"
+   ;;
+   isset,isset)
+   # do this later
+   # echo >&2 "warning: $old_name is now $new_name"
+   # echo >&2 "hint: remove $old_name"
+   ;;
+   esac
+}
+
+check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-- 
2.18.0.windows.1



[PATCH v3 4/5] read-cache: update TEST_GIT_INDEX_VERSION support

2018-09-18 Thread Ben Peart
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with
the other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.

Signed-off-by: Ben Peart 
---
 Makefile  |  6 +++---
 t/README  |  4 
 t/test-lib.sh | 14 --
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 5a969f5830..9e84ef02f7 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,7 @@ all::
 # (defaults to "man") if you want to have a different default when
 # "git help" is called without a parameter specifying the format.
 #
-# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite
+# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite
 # with a different indexfile format version.  If it isn't set the index
 # file format used is index-v[23].
 #
@@ -2599,8 +2599,8 @@ endif
 ifdef GIT_INTEROP_MAKE_OPTS
@echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+
 endif
-ifdef TEST_GIT_INDEX_VERSION
-   @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+
+ifdef GIT_TEST_INDEX_VERSION
+   @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+
 endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
 
diff --git a/t/README b/t/README
index 47165f7eab..9b13f6d12e 100644
--- a/t/README
+++ b/t/README
@@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the 
fsmonitor
 code path for utilizing a file system monitor to speed up detecting
 new or changed files.
 
+GIT_TEST_INDEX_VERSION= exercises the index read/write code path
+for the index version specified.  Can be set to any valid version
+(currently 2, 3, or 4).
+
 Naming Tests
 
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 653688c067..e80c84d13c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,12 +134,6 @@ export EDITOR
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
-then
-   GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
-   export GIT_INDEX_VERSION
-fi
-
 check_var_migration () {
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
@@ -159,6 +153,14 @@ check_var_migration () {
 }
 
 check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
+
+# Use specific version of the index file format
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
+then
+   GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
+   export GIT_INDEX_VERSION
+fi
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
-- 
2.18.0.windows.1



Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support

2018-09-14 Thread Junio C Hamano
Junio C Hamano  writes:

> Ben Peart  writes:
>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 653688c067..397eb71578 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -134,9 +134,9 @@ export EDITOR
>>  GIT_TRACE_BARE=1
>>  export GIT_TRACE_BARE
>>  
>> -if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
>> +if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
>>  then
>> -GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
>> +GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
>>  export GIT_INDEX_VERSION
>>  fi
>
> Is this done a bit before ...
>
>> @@ -159,6 +159,7 @@ check_var_migration () {
>>  }
>>  
>>  check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
>> +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
>
> ... this has a chance to kick in to say things like "Whoa you have
> TEST_GIT_INDEX_VERSION that is an old spelling of
> GIT_TEST_INDEX_VERSION", isn't it?

So, the obvious fix would look like the patch below.

One problem with warning is that

$ TEST_GIT_INDEX_VERSION=4 sh ./t-basic.sh

(or any other depreated variable set without its modern counterpart
set) would fail due to extra output produced to the standard error
stream.

 t/test-lib.sh | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 17a56f44ad..8ef86e05a3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,12 +134,6 @@ export EDITOR
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
-then
-   GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
-   export GIT_INDEX_VERSION
-fi
-
 check_var_migration () {
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
@@ -162,6 +156,13 @@ check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
 check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
 check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
 
+# Use specific version of the index file format
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
+then
+   GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
+   export GIT_INDEX_VERSION
+fi
+
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||


Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support

2018-09-14 Thread Junio C Hamano
Ben Peart  writes:

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 653688c067..397eb71578 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -134,9 +134,9 @@ export EDITOR
>  GIT_TRACE_BARE=1
>  export GIT_TRACE_BARE
>  
> -if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
> +if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
>  then
> - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
> + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
>   export GIT_INDEX_VERSION
>  fi

Is this done a bit before ...

> @@ -159,6 +159,7 @@ check_var_migration () {
>  }
>  
>  check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
> +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION

... this has a chance to kick in to say things like "Whoa you have
TEST_GIT_INDEX_VERSION that is an old spelling of
GIT_TEST_INDEX_VERSION", isn't it?

>  # Add libc MALLOC and MALLOC_PERTURB test
>  # only if we are not executing the test with valgrind


[PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support

2018-09-14 Thread Ben Peart
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with
the other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.

Signed-off-by: Ben Peart 
---
 Makefile  | 6 +++---
 t/README  | 4 
 t/test-lib.sh | 5 +++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 5a969f5830..9e84ef02f7 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,7 @@ all::
 # (defaults to "man") if you want to have a different default when
 # "git help" is called without a parameter specifying the format.
 #
-# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite
+# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite
 # with a different indexfile format version.  If it isn't set the index
 # file format used is index-v[23].
 #
@@ -2599,8 +2599,8 @@ endif
 ifdef GIT_INTEROP_MAKE_OPTS
@echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+
 endif
-ifdef TEST_GIT_INDEX_VERSION
-   @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+
+ifdef GIT_TEST_INDEX_VERSION
+   @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+
 endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
 
diff --git a/t/README b/t/README
index 47165f7eab..9b13f6d12e 100644
--- a/t/README
+++ b/t/README
@@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the 
fsmonitor
 code path for utilizing a file system monitor to speed up detecting
 new or changed files.
 
+GIT_TEST_INDEX_VERSION= exercises the index read/write code path
+for the index version specified.  Can be set to any valid version
+(currently 2, 3, or 4).
+
 Naming Tests
 
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 653688c067..397eb71578 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,9 +134,9 @@ export EDITOR
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
 then
-   GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+   GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
export GIT_INDEX_VERSION
 fi
 
@@ -159,6 +159,7 @@ check_var_migration () {
 }
 
 check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
-- 
2.18.0.windows.1



[PATCH v2 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Ben Peart
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.

Signed-off-by: Ben Peart 
---
 config.c|  2 +-
 t/README|  4 
 t/t1700-split-index.sh  |  2 +-
 t/t7519-status-fsmonitor.sh |  2 +-
 t/test-lib.sh   | 20 
 5 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
 int git_config_get_fsmonitor(void)
 {
if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
-   core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+   core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
 
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
 Naming Tests
 
 
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b3b4d83eaf..f6a856f24c 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -6,7 +6,7 @@ test_description='split index mode tests'
 
 # We need total control of index splitting here
 sane_unset GIT_TEST_SPLIT_INDEX
-sane_unset GIT_FSMONITOR_TEST
+sane_unset GIT_TEST_FSMONITOR
 
 test_expect_success 'enable split index' '
git config splitIndex.maxPercentChange 100 &&
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # To run the entire git test suite using fsmonitor:
 #
 # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..653688c067 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,6 +140,26 @@ then
export GIT_INDEX_VERSION
 fi
 
+check_var_migration () {
+   old_name=$1 new_name=$2
+   eval "old_isset=\${${old_name}:+isset}"
+   eval "new_isset=\${${new_name}:+isset}"
+   case "$old_isset,$new_isset" in
+   isset,)
+   echo >&2 "warning: $old_name is now $new_name"
+   echo >&2 "hint: set $new_name too during the transition period"
+   eval "$new_name=\$$old_name"
+   ;;
+   isset,isset)
+   # do this later
+   # echo >&2 "warning: $old_name is now $new_name"
+   # echo >&2 "hint: remove $old_name"
+   ;;
+   esac
+}
+
+check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-- 
2.18.0.windows.1



[PATCH v2 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support

2018-09-14 Thread Ben Peart
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with
the other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.

Signed-off-by: Ben Peart 
---
 preload-index.c | 2 +-
 t/README| 3 +++
 t/t7519-status-fsmonitor.sh | 4 ++--
 t/test-lib.sh   | 1 +
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/preload-index.c b/preload-index.c
index 0a4e2933bb..a850e197c2 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -85,7 +85,7 @@ static void preload_index(struct index_state *index,
return;
 
threads = index->cache_nr / THREAD_COST;
-   if ((index->cache_nr > 1) && (threads < 2) && 
git_env_bool("GIT_FORCE_PRELOAD_TEST", 0))
+   if ((index->cache_nr > 1) && (threads < 2) && 
git_env_bool("GIT_TEST_PRELOAD_INDEX", 0))
threads = 2;
if (threads < 2)
return;
diff --git a/t/README b/t/README
index 9b13f6d12e..5670c7aad0 100644
--- a/t/README
+++ b/t/README
@@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write 
code path
 for the index version specified.  Can be set to any valid version
 (currently 2, 3, or 4).
 
+GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path
+by overriding the minimum number of cache entries required per thread.
+
 Naming Tests
 
 
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,9 @@ do
git config core.preloadIndex $preload_val &&
if test $preload_val = true
then
-   GIT_FORCE_PRELOAD_TEST=$preload_val; export 
GIT_FORCE_PRELOAD_TEST
+   GIT_TEST_PRELOAD_INDEX=$preload_val; export 
GIT_TEST_PRELOAD_INDEX
else
-   unset GIT_FORCE_PRELOAD_TEST
+   sane_unset GIT_TEST_PRELOAD_INDEX
fi
'
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 397eb71578..17a56f44ad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -160,6 +160,7 @@ check_var_migration () {
 
 check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
 check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
+check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
-- 
2.18.0.windows.1



Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Junio C Hamano
Ben Peart  writes:

> The difference here is that core.fsmonitor isn't a boolean value.  It
> is a string to a command that is executed so it can't be moved over to
> get_env_bool().

Ah, of course ;-)

Then please take the following as a review comment for 4/4; checking
if each getenv(VAR) should or should not become git_env_bool() and
updating them should be done as a separate change for variables
whether they are being renamed or not in this series.

>> I _think_ the renaming should be done without getting mixed with
>> other changes like the git_env_bool() done in 4/4.  The idea to use
>> git_env_bool() in stead of getenv() may be a good one, but then we
>> should consistently do so when appropriate, and that would make a
>> fine theme for another topic.
>>


Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Ben Peart




On 9/14/2018 1:15 PM, Junio C Hamano wrote:

Ben Peart  writes:


diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
  int git_config_get_fsmonitor(void)
  {
if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
-   core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+   core_fsmonitor = getenv("GIT_TEST_FSMONITOR");


Sorry for not noticing earlier, but unlike 4/4 that changed
getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to
getenv(VAR), meaning "if it is set to any non-empty string, it is
true".  Is there a reason for this discrepancy?



The difference here is that core.fsmonitor isn't a boolean value.  It is 
a string to a command that is executed so it can't be moved over to 
get_env_bool().



I _think_ the renaming should be done without getting mixed with
other changes like the git_env_bool() done in 4/4.  The idea to use
git_env_bool() in stead of getenv() may be a good one, but then we
should consistently do so when appropriate, and that would make a
fine theme for another topic.



Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Junio C Hamano
Ben Peart  writes:

> diff --git a/config.c b/config.c
> index 3461993f0a..3555c63f28 100644
> --- a/config.c
> +++ b/config.c
> @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
>  int git_config_get_fsmonitor(void)
>  {
>   if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
> - core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
> + core_fsmonitor = getenv("GIT_TEST_FSMONITOR");

Sorry for not noticing earlier, but unlike 4/4 that changed
getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to
getenv(VAR), meaning "if it is set to any non-empty string, it is
true".  Is there a reason for this discrepancy?

I _think_ the renaming should be done without getting mixed with
other changes like the git_env_bool() done in 4/4.  The idea to use
git_env_bool() in stead of getenv() may be a good one, but then we
should consistently do so when appropriate, and that would make a
fine theme for another topic.



Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Junio C Hamano
Junio C Hamano  writes:

> Ben Peart  writes:
>
>> +if test -n "$GIT_FSMONITOR_TEST"
>> +then
>> +if test -n "$GIT_TEST_FSMONITOR"
>> +then
>> +echo "warning: the GIT_FSMONITOR_TEST variable has been renamed 
>> to GIT_TEST_FSMONITOR"
>> +else
>> +echo "error: the GIT_FSMONITOR_TEST variable has been renamed 
>> to GIT_TEST_FSMONITOR"
>> +exit 1
>> +fi
>> +fi
>
> I would have expected that, because we are now doing multiple pairs
> of variables in a single series, we would add a helper function that
> can be called like so:
>
>   check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
>
> in the earliest step.  Perhaps something like this.
>
> check_var_migration () {
>   old_name=$1 new_name=$2
>   eval "old_isset=\${${old_name}:+isset}"
>   eval "new_isset=\${${new_name}:+isset}"
>   case "$old_isset,$new_isset" in
>   isset,)
>   echo >&2 "error: $old_name is now $new_name"
>   exit 1 ;;
>   isset,isset)
>   # enable this, once $old_name no longer is valid anywhere
>   # echo >&2 "warning: $old_name is now $new_name"
>   # echo >&2 "hint: remove $old_name"
>   ;;
>   esac
> }

Alternatively, we could do this, to warn and then migrate the value
given to the old variable automatically to the new variable and let
the test proceed.

check_var_migration () {
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
eval "new_isset=\${${new_name}:+isset}"
case "$old_isset,$new_isset" in
isset,)
echo >&2 "warning: $old_name is now $new_name"
echo >&2 "hint: set $new_name too during the transition period"
eval "$new_name=\$$old_name"
;;
isset,isset)
# do this later
# echo >&2 "warning: $old_name is now $new_name"
# echo >&2 "hint: remove $old_name"
;;
esac
}


Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Junio C Hamano
Ben Peart  writes:

> +if test -n "$GIT_FSMONITOR_TEST"
> +then
> + if test -n "$GIT_TEST_FSMONITOR"
> + then
> + echo "warning: the GIT_FSMONITOR_TEST variable has been renamed 
> to GIT_TEST_FSMONITOR"
> + else
> + echo "error: the GIT_FSMONITOR_TEST variable has been renamed 
> to GIT_TEST_FSMONITOR"
> + exit 1
> + fi
> +fi

I would have expected that, because we are now doing multiple pairs
of variables in a single series, we would add a helper function that
can be called like so:

check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR

in the earliest step.  Perhaps something like this.

check_var_migration () {
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
eval "new_isset=\${${new_name}:+isset}"
case "$old_isset,$new_isset" in
isset,)
echo >&2 "error: $old_name is now $new_name"
exit 1 ;;
isset,isset)
# enable this, once $old_name no longer is valid anywhere
# echo >&2 "warning: $old_name is now $new_name"
# echo >&2 "hint: remove $old_name"
;;
esac
}



[PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Ben Peart
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.

Signed-off-by: Ben Peart 
---
 config.c|  2 +-
 t/README|  4 
 t/t1700-split-index.sh  |  2 +-
 t/t7519-status-fsmonitor.sh |  2 +-
 t/test-lib.sh   | 11 +++
 5 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
 int git_config_get_fsmonitor(void)
 {
if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
-   core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+   core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
 
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
 Naming Tests
 
 
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b3b4d83eaf..f6a856f24c 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -6,7 +6,7 @@ test_description='split index mode tests'
 
 # We need total control of index splitting here
 sane_unset GIT_TEST_SPLIT_INDEX
-sane_unset GIT_FSMONITOR_TEST
+sane_unset GIT_TEST_FSMONITOR
 
 test_expect_success 'enable split index' '
git config splitIndex.maxPercentChange 100 &&
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # To run the entire git test suite using fsmonitor:
 #
 # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..0ef111d808 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,6 +140,17 @@ then
export GIT_INDEX_VERSION
 fi
 
+if test -n "$GIT_FSMONITOR_TEST"
+then
+   if test -n "$GIT_TEST_FSMONITOR"
+   then
+   echo "warning: the GIT_FSMONITOR_TEST variable has been renamed 
to GIT_TEST_FSMONITOR"
+   else
+   echo "error: the GIT_FSMONITOR_TEST variable has been renamed 
to GIT_TEST_FSMONITOR"
+   exit 1
+   fi
+fi
+
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-- 
2.18.0.windows.1



[PATCH v1 3/4] read-cache: update TEST_GIT_INDEX_VERSION support

2018-09-14 Thread Ben Peart
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with
the other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.

Signed-off-by: Ben Peart 
---
 Makefile  |  6 +++---
 t/README  |  4 
 t/test-lib.sh | 15 +--
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 5a969f5830..9e84ef02f7 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,7 @@ all::
 # (defaults to "man") if you want to have a different default when
 # "git help" is called without a parameter specifying the format.
 #
-# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite
+# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite
 # with a different indexfile format version.  If it isn't set the index
 # file format used is index-v[23].
 #
@@ -2599,8 +2599,8 @@ endif
 ifdef GIT_INTEROP_MAKE_OPTS
@echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+
 endif
-ifdef TEST_GIT_INDEX_VERSION
-   @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+
+ifdef GIT_TEST_INDEX_VERSION
+   @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+
 endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
 
diff --git a/t/README b/t/README
index 47165f7eab..9b13f6d12e 100644
--- a/t/README
+++ b/t/README
@@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the 
fsmonitor
 code path for utilizing a file system monitor to speed up detecting
 new or changed files.
 
+GIT_TEST_INDEX_VERSION= exercises the index read/write code path
+for the index version specified.  Can be set to any valid version
+(currently 2, 3, or 4).
+
 Naming Tests
 
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0ef111d808..5f5f0f4b55 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,12 +134,23 @@ export EDITOR
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
 then
-   GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+   GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
export GIT_INDEX_VERSION
 fi
 
+if test -n "$TEST_GIT_INDEX_VERSION"
+then
+   if test -n "$GIT_TEST_INDEX_VERSION"
+   then
+   echo "warning: the TEST_GIT_INDEX_VERSION variable has been 
renamed to GIT_TEST_INDEX_VERSION"
+   else
+   echo "error: the TEST_GIT_INDEX_VERSION variable has been 
renamed to GIT_TEST_INDEX_VERSION"
+   exit 1
+   fi
+fi
+
 if test -n "$GIT_FSMONITOR_TEST"
 then
if test -n "$GIT_TEST_FSMONITOR"
-- 
2.18.0.windows.1



[PATCH v1 4/4] preload-index: update GIT_FORCE_PRELOAD_TEST support

2018-09-14 Thread Ben Peart
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with
the other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.

Signed-off-by: Ben Peart 
---
 preload-index.c |  3 ++-
 t/README|  3 +++
 t/t7519-status-fsmonitor.sh |  4 ++--
 t/test-lib.sh   | 11 +++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/preload-index.c b/preload-index.c
index 71cd2437a3..a850e197c2 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -5,6 +5,7 @@
 #include "pathspec.h"
 #include "dir.h"
 #include "fsmonitor.h"
+#include "config.h"
 
 #ifdef NO_PTHREADS
 static void preload_index(struct index_state *index,
@@ -84,7 +85,7 @@ static void preload_index(struct index_state *index,
return;
 
threads = index->cache_nr / THREAD_COST;
-   if ((index->cache_nr > 1) && (threads < 2) && 
getenv("GIT_FORCE_PRELOAD_TEST"))
+   if ((index->cache_nr > 1) && (threads < 2) && 
git_env_bool("GIT_TEST_PRELOAD_INDEX", 0))
threads = 2;
if (threads < 2)
return;
diff --git a/t/README b/t/README
index 9b13f6d12e..5670c7aad0 100644
--- a/t/README
+++ b/t/README
@@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write 
code path
 for the index version specified.  Can be set to any valid version
 (currently 2, 3, or 4).
 
+GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path
+by overriding the minimum number of cache entries required per thread.
+
 Naming Tests
 
 
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,9 @@ do
git config core.preloadIndex $preload_val &&
if test $preload_val = true
then
-   GIT_FORCE_PRELOAD_TEST=$preload_val; export 
GIT_FORCE_PRELOAD_TEST
+   GIT_TEST_PRELOAD_INDEX=$preload_val; export 
GIT_TEST_PRELOAD_INDEX
else
-   unset GIT_FORCE_PRELOAD_TEST
+   sane_unset GIT_TEST_PRELOAD_INDEX
fi
'
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5f5f0f4b55..3f447b8ddc 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -162,6 +162,17 @@ then
fi
 fi
 
+if test -n "$GIT_FORCE_PRELOAD_TEST"
+then
+   if test -n "$GIT_TEST_PRELOAD_INDEX"
+   then
+   echo "warning: the GIT_FORCE_PRELOAD_TEST variable has been 
renamed to GIT_TEST_PRELOAD_INDEX"
+   else
+   echo "error: the GIT_FORCE_PRELOAD_TEST variable has been 
renamed to GIT_TEST_PRELOAD_INDEX"
+   exit 1
+   fi
+fi
+
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-- 
2.18.0.windows.1



Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-14 Thread Ben Peart




On 9/13/2018 2:54 PM, Ævar Arnfjörð Bjarmason wrote:


On Thu, Sep 13 2018, Ben Peart wrote:


diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
  int git_config_get_fsmonitor(void)
  {
if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
-   core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+   core_fsmonitor = getenv("GIT_TEST_FSMONITOR");

if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 9028b47d92..545438c820 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon 
pack-objects code
  path where deltas larger than this limit require extra memory
  allocation for bookkeeping.

+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
  Naming Tests
  


I've seen this & will watch out for it, but still, when I'm updating to
"next" in a couple of months I may not be tracking the exact state of
the integration of this patch, and then running with
GIT_FSMONITOR_TEST=... will suddenly be a noop.

So maybe something like this to test-lib.sh as well (or directly in
config.c):

 if test -n "$GIT_FSMONITOR_TEST"
 then
 echo "The GIT_FSMONITOR_TEST variable has been renamed to 
GIT_TEST_FSMONITOR"
 exit 1
 fi

Maybe I'm being too nitpicky and there's only two of us who run the test
with this anyway, and we can deal with it.



I agree that there are probably only 2 people in the world who ever used 
this but I'm happy to add the additional test to make it obvious it has 
been renamed.



It just rubs me the wrong way that we have a test mode that silently
stops being picked up because a command-line option or env variable got
renamed, especially since we've had it for 4 stable releases, especially
since it's so easy for us to avoid that confusion (just die),
v.s. potential time wasted downstream (wondering why fsmonitor stuff
broke on $SOME_OS even though we're testing for it during package
build...).



Re: [PATCH v1] preload-index: update GIT_FORCE_PRELOAD_TEST support

2018-09-13 Thread Junio C Hamano
Ben Peart  writes:

> Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD for consistency with the
> other GIT_TEST_ special setups and properly document its use.
>
> Signed-off-by: Ben Peart 
> ---
>

Among the two unrelated changes that are not mentioned in the
proposed log message, the change to the test to use sane_unset is
probably OK, but replacing a call to getenv() with git_env_bool()
without making sure necessary header file(s) are included would
break the build.

I _think_ config.h is the header to include; I didn't try it,
though.

Thanks.


> Notes:
> Base Ref: v2.19.0
> Web-Diff: https://github.com/benpeart/git/commit/dcd201b920
> Checkout: git fetch https://github.com/benpeart/git git-test-preload-v1 
> && git checkout dcd201b920
>
>  preload-index.c | 2 +-
>  t/README| 3 +++
>  t/t7519-status-fsmonitor.sh | 4 ++--
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/preload-index.c b/preload-index.c
> index 71cd2437a3..cc8a7333c2 100644
> --- a/preload-index.c
> +++ b/preload-index.c
> @@ -84,7 +84,7 @@ static void preload_index(struct index_state *index,
>   return;
>  
>   threads = index->cache_nr / THREAD_COST;
> - if ((index->cache_nr > 1) && (threads < 2) && 
> getenv("GIT_FORCE_PRELOAD_TEST"))
> + if ((index->cache_nr > 1) && (threads < 2) && 
> git_env_bool("GIT_TEST_PRELOAD", 0))
>   threads = 2;
>   if (threads < 2)
>   return;
> diff --git a/t/README b/t/README
> index 9028b47d92..73fb09560f 100644
> --- a/t/README
> +++ b/t/README
> @@ -319,6 +319,9 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon 
> pack-objects code
>  path where deltas larger than this limit require extra memory
>  allocation for bookkeeping.
>  
> +GIT_TEST_PRELOAD= exercises the preload-index code path by
> +overriding the minimum number of cache entries required per thread.
> +
>  Naming Tests
>  
>  
> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> index 756beb0d8e..9b703d49b5 100755
> --- a/t/t7519-status-fsmonitor.sh
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -245,9 +245,9 @@ do
>   git config core.preloadIndex $preload_val &&
>   if test $preload_val = true
>   then
> - GIT_FORCE_PRELOAD_TEST=$preload_val; export 
> GIT_FORCE_PRELOAD_TEST
> + GIT_TEST_PRELOAD=$preload_val; export GIT_TEST_PRELOAD
>   else
> - unset GIT_FORCE_PRELOAD_TEST
> + sane_unset GIT_TEST_PRELOAD
>   fi
>   '
>  
>
> base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670


Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support

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


On Thu, Sep 13 2018, Ben Peart wrote:

> diff --git a/config.c b/config.c
> index 3461993f0a..3555c63f28 100644
> --- a/config.c
> +++ b/config.c
> @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
>  int git_config_get_fsmonitor(void)
>  {
>   if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
> - core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
> + core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
>
>   if (core_fsmonitor && !*core_fsmonitor)
>   core_fsmonitor = NULL;
> diff --git a/t/README b/t/README
> index 9028b47d92..545438c820 100644
> --- a/t/README
> +++ b/t/README
> @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon 
> pack-objects code
>  path where deltas larger than this limit require extra memory
>  allocation for bookkeeping.
>
> +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
> +code path for utilizing a file system monitor to speed up detecting
> +new or changed files.
> +
>  Naming Tests
>  

I've seen this & will watch out for it, but still, when I'm updating to
"next" in a couple of months I may not be tracking the exact state of
the integration of this patch, and then running with
GIT_FSMONITOR_TEST=... will suddenly be a noop.

So maybe something like this to test-lib.sh as well (or directly in
config.c):

if test -n "$GIT_FSMONITOR_TEST"
then
echo "The GIT_FSMONITOR_TEST variable has been renamed to 
GIT_TEST_FSMONITOR"
exit 1
fi

Maybe I'm being too nitpicky and there's only two of us who run the test
with this anyway, and we can deal with it.

It just rubs me the wrong way that we have a test mode that silently
stops being picked up because a command-line option or env variable got
renamed, especially since we've had it for 4 stable releases, especially
since it's so easy for us to avoid that confusion (just die),
v.s. potential time wasted downstream (wondering why fsmonitor stuff
broke on $SOME_OS even though we're testing for it during package
build...).


[PATCH v2] read-cache: update TEST_GIT_INDEX_VERSION support

2018-09-13 Thread Ben Peart
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with
the other GIT_TEST_ special setups and properly document its use.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: v2.19.0
Web-Diff: https://github.com/benpeart/git/commit/e26ccb9004
Checkout: git fetch https://github.com/benpeart/git 
git-test-index-version-v2 && git checkout e26ccb9004

### Interdiff (v1..v2):

diff --git a/Makefile b/Makefile
index 5a969f5830..9e84ef02f7 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,7 @@ all::
 # (defaults to "man") if you want to have a different default when
 # "git help" is called without a parameter specifying the format.
 #
-# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite
+# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite
 # with a different indexfile format version.  If it isn't set the index
 # file format used is index-v[23].
 #
@@ -2599,8 +2599,8 @@ endif
 ifdef GIT_INTEROP_MAKE_OPTS
@echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+
 endif
-ifdef TEST_GIT_INDEX_VERSION
-   @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+
+ifdef GIT_TEST_INDEX_VERSION
+   @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst 
','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+
 endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi

diff --git a/read-cache.c b/read-cache.c
index d140ce9989..7b1354d759 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1570,45 +1570,26 @@ static unsigned int get_index_format_default(void)
char *envversion = getenv("GIT_INDEX_VERSION");
char *endp;
int value;
-   unsigned int version = -1;
+   unsigned int version = INDEX_FORMAT_DEFAULT;

-   if (envversion) {
-   version = strtoul(envversion, , 10);
-   if (*endp ||
-   version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) 
{
-   warning(_("GIT_INDEX_VERSION set, but the value is 
invalid.\n"
-   "Using version %i"), INDEX_FORMAT_DEFAULT);
-   version = INDEX_FORMAT_DEFAULT;
-   }
-   }
-
-   if (version == -1) {
-   if (!git_config_get_int("index.version", )) {
+   if (!envversion) {
+   if (!git_config_get_int("index.version", ))
version = value;
if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
warning(_("index.version set, but the value is 
invalid.\n"
  "Using version %i"), INDEX_FORMAT_DEFAULT);
-   version = INDEX_FORMAT_DEFAULT;
-   }
+   return INDEX_FORMAT_DEFAULT;
}
+   return version;
}

-   if (version == -1) {
-   envversion = getenv("GIT_TEST_INDEX_VERSION");
-   if (envversion) {
version = strtoul(envversion, , 10);
if (*endp ||
version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
-   warning(_("GIT_TEST_INDEX_VERSION set, but the 
value is invalid.\n"
+   warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
  "Using version %i"), INDEX_FORMAT_DEFAULT);
version = INDEX_FORMAT_DEFAULT;
}
-   }
-   }
-
-   if (version == -1)
-   version = INDEX_FORMAT_DEFAULT;
-
return version;
 }

diff --git a/t/README b/t/README
index f872638a78..fb6359b17b 100644
--- a/t/README
+++ b/t/README
@@ -320,8 +320,7 @@ path where deltas larger than this limit require extra 
memory
 allocation for bookkeeping.

 GIT_TEST_INDEX_VERSION= exercises the index read/write code path
-for the index version specified.  Can be set to any valid version
-but the non-default version 4 is probably the most beneficial.
+for the index version specified.  Can be set to any valid version.

 Naming Tests
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..31698c01c4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,9 +134,9 @@ export EDITOR
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE

-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
 then
-   GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+   GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
export GIT_INDEX_VERSION
 fi

### Patches

 Makefile  | 6 +++---
 t/README  | 5 -
 t/test-lib.sh | 4 ++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile

Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-13 Thread Junio C Hamano
Ben Peart  writes:

> Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
> other GIT_TEST_ special setups and properly document its use.

Makes sense.

Thanks for such an attention to detail.

>
> Signed-off-by: Ben Peart 
> ---
>
> Notes:
> Base Ref: v2.19.0
> Web-Diff: https://github.com/benpeart/git/commit/311484a684
> Checkout: git fetch https://github.com/benpeart/git git-test-fsmonitor-v1 
> && git checkout 311484a684
>
>  config.c| 2 +-
>  t/README| 4 
>  t/t1700-split-index.sh  | 2 +-
>  t/t7519-status-fsmonitor.sh | 2 +-
>  4 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/config.c b/config.c
> index 3461993f0a..3555c63f28 100644
> --- a/config.c
> +++ b/config.c
> @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
>  int git_config_get_fsmonitor(void)
>  {
>   if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
> - core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
> + core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
>  
>   if (core_fsmonitor && !*core_fsmonitor)
>   core_fsmonitor = NULL;
> diff --git a/t/README b/t/README
> index 9028b47d92..545438c820 100644
> --- a/t/README
> +++ b/t/README
> @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon 
> pack-objects code
>  path where deltas larger than this limit require extra memory
>  allocation for bookkeeping.
>  
> +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
> +code path for utilizing a file system monitor to speed up detecting
> +new or changed files.
> +
>  Naming Tests
>  
>  
> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index b3b4d83eaf..f6a856f24c 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -6,7 +6,7 @@ test_description='split index mode tests'
>  
>  # We need total control of index splitting here
>  sane_unset GIT_TEST_SPLIT_INDEX
> -sane_unset GIT_FSMONITOR_TEST
> +sane_unset GIT_TEST_FSMONITOR
>  
>  test_expect_success 'enable split index' '
>   git config splitIndex.maxPercentChange 100 &&
> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> index 756beb0d8e..d77012ea6d 100755
> --- a/t/t7519-status-fsmonitor.sh
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -8,7 +8,7 @@ test_description='git status with file system watcher'
>  # To run the entire git test suite using fsmonitor:
>  #
>  # copy t/t7519/fsmonitor-all to a location in your path and then set
> -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
> +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
>  #
>  
>  # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
>
> base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670


[PATCH v1] preload-index: update GIT_FORCE_PRELOAD_TEST support

2018-09-13 Thread Ben Peart
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD for consistency with the
other GIT_TEST_ special setups and properly document its use.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: v2.19.0
Web-Diff: https://github.com/benpeart/git/commit/dcd201b920
Checkout: git fetch https://github.com/benpeart/git git-test-preload-v1 && 
git checkout dcd201b920

 preload-index.c | 2 +-
 t/README| 3 +++
 t/t7519-status-fsmonitor.sh | 4 ++--
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/preload-index.c b/preload-index.c
index 71cd2437a3..cc8a7333c2 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -84,7 +84,7 @@ static void preload_index(struct index_state *index,
return;
 
threads = index->cache_nr / THREAD_COST;
-   if ((index->cache_nr > 1) && (threads < 2) && 
getenv("GIT_FORCE_PRELOAD_TEST"))
+   if ((index->cache_nr > 1) && (threads < 2) && 
git_env_bool("GIT_TEST_PRELOAD", 0))
threads = 2;
if (threads < 2)
return;
diff --git a/t/README b/t/README
index 9028b47d92..73fb09560f 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,9 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon 
pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
+GIT_TEST_PRELOAD= exercises the preload-index code path by
+overriding the minimum number of cache entries required per thread.
+
 Naming Tests
 
 
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..9b703d49b5 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,9 @@ do
git config core.preloadIndex $preload_val &&
if test $preload_val = true
then
-   GIT_FORCE_PRELOAD_TEST=$preload_val; export 
GIT_FORCE_PRELOAD_TEST
+   GIT_TEST_PRELOAD=$preload_val; export GIT_TEST_PRELOAD
else
-   unset GIT_FORCE_PRELOAD_TEST
+   sane_unset GIT_TEST_PRELOAD
fi
'
 

base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
-- 
2.18.0.windows.1



[PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-13 Thread Ben Peart
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: v2.19.0
Web-Diff: https://github.com/benpeart/git/commit/311484a684
Checkout: git fetch https://github.com/benpeart/git git-test-fsmonitor-v1 
&& git checkout 311484a684

 config.c| 2 +-
 t/README| 4 
 t/t1700-split-index.sh  | 2 +-
 t/t7519-status-fsmonitor.sh | 2 +-
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
 int git_config_get_fsmonitor(void)
 {
if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
-   core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+   core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
 
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 9028b47d92..545438c820 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon 
pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
 Naming Tests
 
 
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b3b4d83eaf..f6a856f24c 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -6,7 +6,7 @@ test_description='split index mode tests'
 
 # We need total control of index splitting here
 sane_unset GIT_TEST_SPLIT_INDEX
-sane_unset GIT_FSMONITOR_TEST
+sane_unset GIT_TEST_FSMONITOR
 
 test_expect_success 'enable split index' '
git config splitIndex.maxPercentChange 100 &&
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # To run the entire git test suite using fsmonitor:
 #
 # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'

base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
-- 
2.18.0.windows.1



[PATCH v5 03/12] t0000: update tests for SHA-256

2018-09-12 Thread brian m. carlson
Test t tests the "basics of the basics" and as such, checks that we
have various fixed hard-coded object IDs.  The tests relying on these
assertions have been marked with the SHA1 prerequisite, as they will
obviously not function in their current form with SHA-256.

Use the test_oid helper to update these assertions and provide values
for both SHA-1 and SHA-256.

These object IDs were synthesized using a set of scripts that created
the objects for both SHA-1 and SHA-256 using the same method to ensure
that they are indeed the correct values.

Signed-off-by: brian m. carlson 
---
 t/t-basic.sh | 163 +--
 1 file changed, 102 insertions(+), 61 deletions(-)

diff --git a/t/t-basic.sh b/t/t-basic.sh
index a9dc534048..391f910c6a 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -861,6 +861,47 @@ test_expect_success 'test_oid can look up data for 
SHA-256' '
 
 # Basics of the basics
 
+test_oid_cache <<\EOF
+path0f sha1:f87290f8eb2cbbea7857214459a0739927eab154
+path0f sha256:638106af7c38be056f3212cbd7ac65bc1bac74f420ca5a436ff006a9d025d17d
+
+path0s sha1:15a98433ae33114b085f3eb3bb03b832b3180a01
+path0s sha256:3a24cc53cf68edddac490bbf94a418a52932130541361f685df685e41dd6c363
+
+path2f sha1:3feff949ed00a62d9f7af97c15cd8a30595e7ac7
+path2f sha256:2a7f36571c6fdbaf0e3f62751a0b25a3f4c54d2d1137b3f4af9cb794bb498e5f
+
+path2s sha1:d8ce161addc5173867a3c3c730924388daedbc38
+path2s sha256:18fd611b787c2e938ddcc248fabe4d66a150f9364763e9ec133dd01d5bb7c65a
+
+path2d sha1:58a09c23e2ca152193f2786e06986b7b6712bdbe
+path2d sha256:00e4b32b96e7e3d65d79112dcbea53238a22715f896933a62b811377e2650c17
+
+path3f sha1:0aa34cae68d0878578ad119c86ca2b5ed5b28376
+path3f sha256:09f58616b951bd571b8cb9dc76d372fbb09ab99db2393f5ab3189d26c45099ad
+
+path3s sha1:8599103969b43aff7e430efea79ca4636466794f
+path3s sha256:fce1aed087c053306f3f74c32c1a838c662bbc4551a7ac2420f5d6eb061374d0
+
+path3d sha1:21ae8269cacbe57ae09138dcc3a2887f904d02b3
+path3d sha256:9b60497be959cb830bf3f0dc82bcc9ad9e925a24e480837ade46b2295e47efe1
+
+subp3f sha1:00fb5908cb97c2564a9783c0c64087333b3b464f
+subp3f sha256:a1a9e16998c988453f18313d10375ee1d0ddefe757e710dcae0d66aa1e0c58b3
+
+subp3s sha1:6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c
+subp3s sha256:81759d9f5e93c6546ecfcadb560c1ff057314b09f93fe8ec06e2d8610d34ef10
+
+subp3d sha1:3c5e5399f3a333eddecce7a9b9465b63f65f51e2
+subp3d sha256:76b4ef482d4fa1c754390344cf3851c7f883b27cf9bc999c6547928c46aeafb7
+
+root sha1:087704a96baf1c2d1c869a8b084481e121c88b5b
+root sha256:9481b52abab1b2ffeedbf9de63ce422b929f179c1b98ff7bee5f8f1bc0710751
+
+simpletree sha1:7bb943559a305bdd6bdee2cef6e5df2413c3d30a
+simpletree 
sha256:1710c07a6c86f9a3c7376364df04c47ee39e5a5e221fcdd84b743bc9bb7e2bc5
+EOF
+
 # updating a new file without --add should fail.
 test_expect_success 'git update-index without --add should fail adding' '
test_must_fail git update-index should-be-empty
@@ -876,8 +917,8 @@ test_expect_success 'writing tree out with git write-tree' '
 '
 
 # we know the shape and contents of the tree and know the object ID for it.
-test_expect_success SHA1 'validate object ID of a known tree' '
-   test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
+test_expect_success 'validate object ID of a known tree' '
+   test "$tree" = "$(test_oid simpletree)"
 '
 
 # Removing paths.
@@ -919,16 +960,16 @@ test_expect_success 'showing stage with git ls-files 
--stage' '
git ls-files --stage >current
 '
 
-test_expect_success SHA1 'validate git ls-files output for a known tree' '
-   cat >expected <<-\EOF &&
-   100644 f87290f8eb2cbbea7857214459a0739927eab154 0   path0
-   12 15a98433ae33114b085f3eb3bb03b832b3180a01 0   path0sym
-   100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0   path2/file2
-   12 d8ce161addc5173867a3c3c730924388daedbc38 0   path2/file2sym
-   100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0   path3/file3
-   12 8599103969b43aff7e430efea79ca4636466794f 0   path3/file3sym
-   100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0   
path3/subp3/file3
-   12 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0   
path3/subp3/file3sym
+test_expect_success 'validate git ls-files output for a known tree' '
+   cat >expected <<-EOF &&
+   100644 $(test_oid path0f) 0 path0
+   12 $(test_oid path0s) 0 path0sym
+   100644 $(test_oid path2f) 0 path2/file2
+   12 $(test_oid path2s) 0 path2/file2sym
+   100644 $(test_oid path3f) 0 path3/file3
+   12 $(test_oid path3s) 0 path3/file3sym
+   100644 $(test_oid subp3f) 0 path3/subp3/file3
+   12 $(test_oid subp3s) 0 path3/subp3/file3sym
EOF
test_cmp expected current
 '
@@ -937,20 +978,20 @@ test_exp

Re: [PATCH] submodule.sh update --remote: default to oid instead of master

2018-09-06 Thread Jonathan Nieder
Stefan Beller wrote:
> On Wed, Sep 5, 2018 at 4:10 PM Jonathan Nieder  wrote:

>> Broader comment: do you think people will be surprised by this new
>> behavior?  Is there anything special we'd need to do to call it out
>> (e.g., print a warning or put something in release notes)?
>
> I guess. Not sure how to approach this best. Maybe we can
> extend the output of 'submodule update' to print that branch names
> instead of hashes for the configured case and keep printing hashes
> only for this case. Although that would not help someone who relies
> on the default solely.

Thinking more out loud: often the simplest migration path involves
multiple steps:

 1. Warn in the case that is going to change, with no behavior change
yet.

 2. Treat the case that will change as an error.  This should
help flush out cases where people were relying on the old behavior.

 3. Introduce the new behavior.  Warn that old versions of Git don't
support it yet.

 4. Eliminate the warning.  You're all clear now.

Sometimes some of these steps can be combined.

Another possible approach is to measure.  For example, is there some
way to find out how many people are relying in this "git submodule
update --remote" defaulting behavior?  One example of this approach is
to make the change (all in one step) in "next" and deploy to some
relevant subpopulation and see if anyone screams.  By making the
change in "next" instead of something with more stability guarantees,
you get the ability to roll back quickly.

There are other tools at our disposal --- e.g. command-line flags,
config, other kinds of research.

Here my first instinct would be to say this should be a command-line
flag.  To start out, we can keep the historical behavior as a default,
but introduce a command-line option for the new behavior.  This way,
people can pass the negation of that command-line option if they want
the older behavior, throughout the transition.

For example (please ignore names):

 Step 0: introduce

git submodule update --remote --default-to-master; # current behavior
git submodule update --remote --no-default-to-master; # new behavior

 and treat plain "git submodule update --remote" as --default-to-master.

 Step 1: when neither --default-to-master nor --no-default-to-master
 has been passed, warn when encountering a submodule with no branch
 and treat it as "master".

 Step 2: when neither --default-to-master nor --no-default-to-master
 has been passed, error out when encountering a submodule with no
 branch.

 Step 3: when neither --default-to-master nor --no-default-to-master
 has been passed, warn when encountering a submodule with no branch
 and treat it as pinned.

 Step 4: eliminate the warning.

What do you think?

Thanks,
Jonathan


Re: [PATCH] submodule.sh update --remote: default to oid instead of master

2018-09-06 Thread Stefan Beller
On Wed, Sep 5, 2018 at 4:10 PM Jonathan Nieder  wrote:
>
> Stefan Beller wrote:
>
> > Subject: submodule.sh update --remote: default to oid instead of master
>
> Yay!
>
> Nit: it wasn't clear to me at first what default this subject line was
> referring to.  Perhaps:
>
> submodule update --remote: skip GITLINK update when no branch is set
>
> [...]
> > --- a/Documentation/gitmodules.txt
> > +++ b/Documentation/gitmodules.txt
> > @@ -50,11 +50,12 @@ submodule..update::
> >
> >  submodule..branch::
> [...]
> > + If the option is not specified, do not update to any branch but
> > + the object id of the remote.
>
> Likewise: how about something like
>
> If not set, the default is for `git submodule update --remote`
> to update the submodule to the superproject's recorded SHA-1.

... recorded object id.

sounds good.

> > + git add .gitmodules &&
> > + git commit --allow-empty -m "submodules: pin in superproject 
> > branch"
> > + ) &&
>
> I wonder if we can do simpler by using -C + some helpers: something like
>
> git config --unset -f super/.gitmodules ... &&
> test_commit -C submodule ... &&
> git -C super submodule update ... &&
> test_cmp_rev ...
>
> Unfortunately test_cmp_rev doesn't accept a -C argument.

and the lack of fortune goes further, as test_cmp_rev needs to have
2 revisions in the same repository, i.e. both need to exist,
which is not the case.

> Broader comment: do you think people will be surprised by this new
> behavior?  Is there anything special we'd need to do to call it out
> (e.g., print a warning or put something in release notes)?

I guess. Not sure how to approach this best. Maybe we can
extend the output of 'submodule update' to print that branch names
instead of hashes for the configured case and keep printing hashes
only for this case. Although that would not help someone who relies
on the default solely.


Re: [PATCH] submodule.sh update --remote: default to oid instead of master

2018-09-05 Thread Jonathan Nieder
Stefan Beller wrote:

> Subject: submodule.sh update --remote: default to oid instead of master

Yay!

Nit: it wasn't clear to me at first what default this subject line was
referring to.  Perhaps:

submodule update --remote: skip GITLINK update when no branch is set

[...]
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -50,11 +50,12 @@ submodule..update::
>  
>  submodule..branch::
[...]
> + If the option is not specified, do not update to any branch but
> + the object id of the remote.

Likewise: how about something like

If not set, the default is for `git submodule update --remote`
to update the submodule to the superproject's recorded SHA-1.

[...]
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -568,16 +568,19 @@ cmd_update()
>   if test -n "$remote"
>   then
>   branch=$(git submodule--helper remote-branch "$sm_path")
> - if test -z "$nofetch"
> + if test -n "$branch"
>   then
> - # Fetch remote before determining tracking $sha1
> - fetch_in_submodule "$sm_path" $depth ||
> - die "$(eval_gettext "Unable to fetch in 
> submodule path '\$sm_path'")"
> + if test -z "$nofetch"
> + then
> + # Fetch remote before determining 
> tracking $sha1
> + fetch_in_submodule "$sm_path" $depth ||

Makes sense.  If $sha1 isn't available in the submodule, it will fetch
again later.

[...]
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -260,6 +260,28 @@ test_expect_success 'submodule update --remote should 
> fetch upstream changes wit
>   )
>  '
>  
> +test_expect_success 'submodule update --remote should not fetch upstream 
> when no branch is set' '
> + (
> + cd super &&
> + test_might_fail git config --unset -f .gitmodules 
> submodule."submodule".branch &&

Not about this patch: the quoting here is strange.

> + git add .gitmodules &&
> + git commit --allow-empty -m "submodules: pin in superproject 
> branch"
> + ) &&

I wonder if we can do simpler by using -C + some helpers: something like

git config --unset -f super/.gitmodules ... &&
test_commit -C submodule ... &&
git -C super submodule update ... &&
test_cmp_rev ...

Unfortunately test_cmp_rev doesn't accept a -C argument.

Broader comment: do you think people will be surprised by this new
behavior?  Is there anything special we'd need to do to call it out
(e.g., print a warning or put something in release notes)?

Thanks,
Jonathan


[PATCH] submodule.sh update --remote: default to oid instead of master

2018-09-05 Thread Stefan Beller
gitmodules(5) sayeth:

   submodule..branch
   A remote branch name for tracking updates in the upstream
   submodule. If the option is not specified, it defaults to master.

This doesn't allow having a "pinned" submodule that should not be updated
from upstream. We should change this to have no default --- if branch is
not specified, don't update that submodule, just like in Gerrit's
corresponding feature[1].

[1] 
https://gerrit-review.googlesource.com/Documentation/user-submodules.html#_defining_the_submodule_branch

Signed-off-by: Stefan Beller 
---
 Documentation/gitmodules.txt | 11 ++-
 git-submodule.sh | 19 +++
 t/t7406-submodule-update.sh  | 22 ++
 3 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 4d63def2069..3b8739f8294 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -50,11 +50,12 @@ submodule..update::
 
 submodule..branch::
A remote branch name for tracking updates in the upstream submodule.
-   If the option is not specified, it defaults to 'master'.  A special
-   value of `.` is used to indicate that the name of the branch in the
-   submodule should be the same name as the current branch in the
-   current repository.  See the `--remote` documentation in
-   linkgit:git-submodule[1] for details.
+   A special value of `.` is used to indicate that the name of the
+   branch in the submodule should be the same name as the current
+   branch in the current repository.  See the `--remote` documentation
+   in linkgit:git-submodule[1] for details.
+   If the option is not specified, do not update to any branch but
+   the object id of the remote.
 
 submodule..fetchRecurseSubmodules::
This option can be used to control recursive fetching of this
diff --git a/git-submodule.sh b/git-submodule.sh
index f7fd80345cd..342050ae934 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -568,16 +568,19 @@ cmd_update()
if test -n "$remote"
then
branch=$(git submodule--helper remote-branch "$sm_path")
-   if test -z "$nofetch"
+   if test -n "$branch"
then
-   # Fetch remote before determining tracking $sha1
-   fetch_in_submodule "$sm_path" $depth ||
-   die "$(eval_gettext "Unable to fetch in 
submodule path '\$sm_path'")"
+   if test -z "$nofetch"
+   then
+   # Fetch remote before determining 
tracking $sha1
+   fetch_in_submodule "$sm_path" $depth ||
+   die "$(eval_gettext "Unable to fetch in 
submodule path '\$sm_path'")"
+   fi
+   remote_name=$(sanitize_submodule_env; cd 
"$sm_path" && get_default_remote)
+   sha1=$(sanitize_submodule_env; cd "$sm_path" &&
+   git rev-parse --verify 
"${remote_name}/${branch}") ||
+   die "$(eval_gettext "Unable to find current 
\${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
fi
-   remote_name=$(sanitize_submodule_env; cd "$sm_path" && 
get_default_remote)
-   sha1=$(sanitize_submodule_env; cd "$sm_path" &&
-   git rev-parse --verify 
"${remote_name}/${branch}") ||
-   die "$(eval_gettext "Unable to find current 
\${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
fi
 
if ! $(git config -f "$(git rev-parse 
--git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 10dc91620a6..f04884743fd 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -260,6 +260,28 @@ test_expect_success 'submodule update --remote should 
fetch upstream changes wit
)
 '
 
+test_expect_success 'submodule update --remote should not fetch upstream when 
no branch is set' '
+   (
+   cd super &&
+   test_might_fail git config --unset -f .gitmodules 
submodule."submodule".branch &&
+   git add .gitmodules &&
+   git commit --allow-empty -m "submodules: pin in superproject 
branch"
+  

[PATCH 0/2] --no-deref and --stdin compatibility for update-ref

2018-09-05 Thread Elijah Newren
Currently, the --no-deref and --stdin options of update-ref cannot be
used together (the code aborts immediately with a usage message), though
it makes sense to do so and is easier than repeatedly specifying on
stdin that each ref should not be dereferenced.  Also, the documentation
for the --no-deref option has a few problems, making it unclear what it
is and isn't compatible with.

The first patch is just a minor code fixup that the second lightly
depends on.  The second patch has the changes to fix the issues stated
above.


Elijah Newren (2):
  update-ref: fix type of update_flags variable to match its usage
  update-ref: allow --no-deref with --stdin

 Documentation/git-update-ref.txt |  2 +-
 builtin/update-ref.c | 25 ++---
 t/t1400-update-ref.sh| 31 +++
 3 files changed, 46 insertions(+), 12 deletions(-)

-- 
2.19.0.rc2.2.g1aedc61e22



[PATCH 2/2] update-ref: allow --no-deref with --stdin

2018-09-05 Thread Elijah Newren
If passed both --no-deref and --stdin, update-ref would error out with a
general usage message that did not at all suggest these options were
incompatible.  The manpage for update-ref did suggest through its
synopsis line that --no-deref and --stdin were incompatible, but it sadly
also incorrectly suggested that -d and --no-deref were incompatible.  So
the help around the --no-deref option is buggy in a few ways.

The --stdin option did provide a different mechanism for avoiding
dereferencing symbolic-refs: adding a line reading
  option no-deref
before every other directive in the input.  (Technically, if the user
wants to do the extra work of first determining which refs they want to
update or delete are symbolic, then they only need to put the extra
"option no-deref" lines before the updates of those refs.  But in some
cases, that's more work than just adding the "option no-deref" before
every other directive.)

It's easier to allow the user to just pass --no-deref along with --stdin
in order to tell update-ref that the user doesn't want any symbolic ref
to be dereferenced.  It also makes the update-ref documentation simpler.
Implement that, and update the documentation to match.

Signed-off-by: Elijah Newren 
---
 Documentation/git-update-ref.txt |  2 +-
 builtin/update-ref.c | 23 +--
 t/t1400-update-ref.sh| 31 +++
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index bc8fdfd469..fda8516677 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely
 SYNOPSIS
 
 [verse]
-'git update-ref' [-m ] (-d  [] | [--no-deref] 
[--create-reflog]   [] | --stdin [-z])
+'git update-ref' [-m ] [--no-deref] (-d  [] | 
[--create-reflog]   [] | --stdin [-z])
 
 DESCRIPTION
 ---
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 54fac01f21..2d8f7f0578 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -15,6 +15,7 @@ static const char * const git_update_ref_usage[] = {
 
 static char line_termination = '\n';
 static unsigned int update_flags;
+static unsigned int default_flags;
 static unsigned create_reflog_flag;
 static const char *msg;
 
@@ -205,7 +206,7 @@ static const char *parse_cmd_update(struct ref_transaction 
*transaction,
   msg, ))
die("%s", err.buf);
 
-   update_flags = 0;
+   update_flags = default_flags;
free(refname);
strbuf_release();
 
@@ -237,7 +238,7 @@ static const char *parse_cmd_create(struct ref_transaction 
*transaction,
   msg, ))
die("%s", err.buf);
 
-   update_flags = 0;
+   update_flags = default_flags;
free(refname);
strbuf_release();
 
@@ -273,7 +274,7 @@ static const char *parse_cmd_delete(struct ref_transaction 
*transaction,
   update_flags, msg, ))
die("%s", err.buf);
 
-   update_flags = 0;
+   update_flags = default_flags;
free(refname);
strbuf_release();
 
@@ -302,7 +303,7 @@ static const char *parse_cmd_verify(struct ref_transaction 
*transaction,
   update_flags, ))
die("%s", err.buf);
 
-   update_flags = 0;
+   update_flags = default_flags;
free(refname);
strbuf_release();
 
@@ -357,7 +358,6 @@ int cmd_update_ref(int argc, const char **argv, const char 
*prefix)
const char *refname, *oldval;
struct object_id oid, oldoid;
int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0;
-   unsigned int flags = 0;
int create_reflog = 0;
struct option options[] = {
    OPT_STRING( 'm', NULL, , N_("reason"), N_("reason of the 
update")),
@@ -378,6 +378,11 @@ int cmd_update_ref(int argc, const char **argv, const char 
*prefix)
 
create_reflog_flag = create_reflog ? REF_FORCE_CREATE_REFLOG : 0;
 
+   if (no_deref) {
+   default_flags = REF_NO_DEREF;
+   update_flags = default_flags;
+   }
+
if (read_stdin) {
struct strbuf err = STRBUF_INIT;
struct ref_transaction *transaction;
@@ -385,7 +390,7 @@ int cmd_update_ref(int argc, const char **argv, const char 
*prefix)
transaction = ref_transaction_begin();
if (!transaction)
die("%s", err.buf);
-   if (delete || no_deref || argc > 0)
+   if (delete || argc > 0)
usage_with_options(git_update_ref_usage, options);
if (end_null)
line_termination = '\0';
@@ -427,8 +432,6 @@ int cmd_update_ref(int arg

[PATCH 1/2] update-ref: fix type of update_flags variable to match its usage

2018-09-05 Thread Elijah Newren
The ref_transaction_*() family of functions expect a flags parameter
which is of type unsigned int.  Make the update_flags variable, which
is passed as that parameter, be of the same type.

Signed-off-by: Elijah Newren 
---
 builtin/update-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 4fa3c0a86f..54fac01f21 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -14,7 +14,7 @@ static const char * const git_update_ref_usage[] = {
 };
 
 static char line_termination = '\n';
-static int update_flags;
+static unsigned int update_flags;
 static unsigned create_reflog_flag;
 static const char *msg;
 
-- 
2.19.0.rc2.2.g1aedc61e22



  1   2   3   4   5   6   7   8   9   10   >