[PATCH] RelNotes: minor typo fixes in 2.16.0 draft

2017-12-13 Thread Todd Zullinger
Signed-off-by: Todd Zullinger 
---
 Documentation/RelNotes/2.16.0.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/RelNotes/2.16.0.txt 
b/Documentation/RelNotes/2.16.0.txt
index 3eeeb83674..f7fca7123f 100644
--- a/Documentation/RelNotes/2.16.0.txt
+++ b/Documentation/RelNotes/2.16.0.txt
@@ -304,10 +304,10 @@ Fixes since v2.15
  * "git branch --set-upstream" has been deprecated and (sort of)
removed, as "--set-upstream-to" is the preferred one these days.
The documentation still had "--set-upstream" listed on its
-   synopsys section, which has been corrected.
+   synopsis section, which has been corrected.
(merge a060f3d3d8 tz/branch-doc-remove-set-upstream later to maint).
 
- * Internaly we use 0{40} as a placeholder object name to signal the
+ * Internally we use 0{40} as a placeholder object name to signal the
codepath that there is no such object (e.g. the fast-forward check
while "git fetch" stores a new remote-tracking ref says "we know
there is no 'old' thing pointed at by the ref, as we are creating
-- 
2.15.1



[PATCH] git-svn: convert CRLF to LF in commit message to SVN

2017-12-13 Thread Eric Wong
"Bennett, Brian"  wrote:
> Environment:
> 
> Desktop: Windows 7 Enterprise 64-bit
> svn client (if applicable): 1.8.8 from Apache
> git (https://git-for-windows.github.io/): git version 2.10.1.windows.1
> GitTfs (https://github.com/git-tfs/git-tfs): git-tfs version 0.27.0.0 (TFS 
> client library 14.0.0.0 (MS)) (32-bit)
> Team Foundation Server: 2010
> Visual Studio installation: 2010 and 2015



Thanks for the report and research!

> I've researched enough to believe that the commit message
> being used by git svn contains a carriage return character
> (x'0D') and that has not been allowed in Subversion since
> version 1.6 (I can replicate this specific error message using
> an SVN dump file that contains x'0D' characters in the log
> messages.). However, I cannot find where I have any control
> over the log message that git svn is trying to use nor can I
> observe it. Note that I've also used the '-v' switch with the
> 'git svn dcommit', but do not receive anything other than what
> I am showing above.

Maybe git-for-windows isn't filtering CRLF into LF as "git commit"
does on GNU/Linux when the original commit was made?

I had to use "git commit-tree" to reproduce the error in testing
(instead of "git commit)"

Anyways, the one-line fix below should be enough for you.
Care to give it a shot?  Thanks again.


Junio: please pull when Brian confirms, thanks.

The following changes since commit 95ec6b1b3393eb6e26da40c565520a8db9796e9f:

  RelNotes: the eighth batch (2017-12-06 09:29:50 -0800)

are available in the git repository at:

  git://bogomips.org/git-svn.git svn-crlf

for you to fetch changes up to 95450bbbaaacaf2d603a4fbded25d55243dfb291:

  git-svn: convert CRLF to LF in commit message to SVN (2017-12-14 00:09:38 
+)


Eric Wong (1):
  git-svn: convert CRLF to LF in commit message to SVN

 git-svn.perl|  1 +
 t/t9169-git-svn-dcommit-crlf.sh | 27 +++
 2 files changed, 28 insertions(+)
 create mode 100755 t/t9169-git-svn-dcommit-crlf.sh

--8<
Subject: [PATCH] git-svn: convert CRLF to LF in commit message to SVN

Subversion since 1.6 does not accept CR characters in the commit
message, so filter it out on our end before 'git svn dcommit' sets
the svn:log property.

Reported-by: Brian Bennett 
Signed-off-by: Eric Wong 
---
 git-svn.perl|  1 +
 t/t9169-git-svn-dcommit-crlf.sh | 27 +++
 2 files changed, 28 insertions(+)
 create mode 100755 t/t9169-git-svn-dcommit-crlf.sh

diff --git a/git-svn.perl b/git-svn.perl
index d2404184ba..aa242d4f4f 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1865,6 +1865,7 @@ sub get_commit_entry {
}
}
$msgbuf =~ s/\s+$//s;
+   $msgbuf =~ s/\r\n/\n/sg; # SVN 1.6+ disallows CRLF
if ($Git::SVN::_add_author_from && defined($author)
&& !$saw_from) {
$msgbuf .= "\n\nFrom: $author";
diff --git a/t/t9169-git-svn-dcommit-crlf.sh b/t/t9169-git-svn-dcommit-crlf.sh
new file mode 100755
index 00..54b1f61a2a
--- /dev/null
+++ b/t/t9169-git-svn-dcommit-crlf.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+test_description='git svn dcommit CRLF'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup commit repository' '
+   svn_cmd mkdir -m "$test_description" "$svnrepo/dir" &&
+   git svn clone "$svnrepo" work &&
+   (
+   cd work &&
+   echo foo >>foo &&
+   git update-index --add foo &&
+   printf "a\\r\\n\\r\\nb\\r\\nc\\r\\n" >cmt &&
+   p=$(git rev-parse HEAD) &&
+   t=$(git write-tree) &&
+   cmt=$(git commit-tree -p $p $t out &&
+   test_cmp cmt out &&
+   git svn dcommit &&
+   printf "a\\n\\nb\\nc\\n" >exp &&
+   git cat-file commit HEAD | sed -ne 6,9p >out &&
+   test_cmp exp out
+   )
+'
+
+test_done
-- 
EW


Re: Apparent bug in 'git stash push ' loses untracked files

2017-12-13 Thread Igor Djordjevic
Hi Thomas,

On 14/12/2017 00:14, Thomas Gummerer wrote:
> 
> > For what it`s worth, using `git stash save ` instead seems
> > to (still) work as expected...
> 
> I think that depends on what you expect ;) 'git stash save ' 
> will create a stash of the whole working directory with the message 
> "". So while it would indeed work for the presumably 
> simplified example Reid provided, it would not do what you'd expect
> if there are any tracked and modified files outside of the .
> 
> In that case 'git stash save ' would include the tracked
> files outside of , while what I assume Reid wanted is to keep
> them in place, and only stash the files in .

Indeed, I didn`t pay enough attention to the fact that even `git 
stash save\push` produced different output messages, the difference 
being exactly automatic (push) versus provided (save) stash message.

And I did use `git stash save ` in the past... :$ Not too 
often, I guess.

> > but on the other hand, `git-stash`[1] manpage seems not to mention
> > this usage ("save" with "pathspec")?
> 
> "stash save" with "pathspec" doesn't exist, and it will probably
> never exist. We decided to introduce a new "push" verb for 'git
> stash' because the command line for 'git stash save' takes a message
> as its last argument, instead of taking the message with a -m flag
> like other commands do. Introducing a pathspec argument for "git
> stash save" would have either broken backward compatibility, or it
> would have had some syntax that's very inconsistent with other git
> commands.

Yeah, I`m aware of the "transition", thus teaching myself to use `git 
stash push` lately. That`s also what made me curious to try out the 
"old" `git stash save` behavior, but obviously in a bit hasty manner.

Sorry for the confusion, and thanks, for both clarification 
and your work.

Regards, Buga


Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

2017-12-13 Thread Thomas Gummerer
On 12/13, Lars Schneider wrote:
> 
> > On 13 Dec 2017, at 18:38, Junio C Hamano  wrote:
> > 
> > Lars Schneider  writes:
> > 
> >> I think your solution points into the right direction.
> >> Right now we have the following test matrix:
> >> 
> >> 1. Linux - clang
> >> 2. Linux - gcc
> >> 3. Mac - clang
> >> 4. Mac - gcc
> >> 5. Linux - gcc - GET_TEXT_POISION
> >> 6. Linux - gcc - 32bit
> >> 7. Windows
> >> 
> >> AFAIK your solution would run the split index test for 
> >> 1, 2, 3, and 4. I think that is too much.
> > 
> > Not that it matters too much, but I meant to add it to 1. and
> > 6. when I said "only one of 64-bit build plus 32-bit one".  
> 
> Ah. Sorry, I didn't get that.
> 
> 
> >> 1 runs the fastest and I would like to keep it that way
> >> to get a quick "general" result. I think only 2 should be
> >> extended in the way you are suggesting. We could run
> >> the tests with different env variables there. What else
> >> do we have besides GIT_TEST_SPLIT_INDEX?
> >> 
> >> Would that work for everyone?
> > 
> > I am OK to make 2. use split-index (which unfortunately would mean
> > we lose tests without split-index under gcc), or add 2.5 that is a
> > copy of 2. plus split-index.
> 
> 
> I think I experessed myself poorly. As far as I understand it,
> GIT_TEST_SPLIT_INDEX is an environment variable that only needs
> to be set at test time and not at compile time. Is this right?

Yes, that's correct.

> If yes, my idea for 2. is as follows:
> - build Git with gcc
> - run tests with "make --quiet test"
> - run tests with "GIT_TEST_SPLIT_INDEX=YesPlease make --quiet test"
> 
> This should be quicker than a new 2.5 target because we don't need to
> spin up the machine and build Git. Plus, we could run the tests
> a few more times with other test flags.

I'd be happy with that.  Thanks for the discussion on this, will
change this in my re-roll.

> - Lars


Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

2017-12-13 Thread Thomas Gummerer
On 12/12, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> >
> > The breakages wen the split-index code fails tend to break things in
> > much more obvious manners than a wrong message, usually git ends up
> > dying if it gets broken.  Both of the bugs that were fixed here would
> > have been caught with the change in my patch.
> >
> > But yeah I can see the argument that it doesn't give us a guarantee
> > that it catches all things the test suite could catch.
> 
> I think you misunderstood me.  When split index is much easier to
> break than poison tests, combining them together would hurt the test
> coverage of poison tests.  If you value poison tests much more than
> how well split index mode works, that is a worse outcome.

Sorry, indeed I misunderstood you.  I do agree with the above.

> >> I wonder if it makes more sense to update ci/run-tests.sh so that
> >> its final step is run twice with different settings, like so?
> >
> > I kind of wanted to avoid that, because it ends up running the test
> > suite twice on travis for every test, which seems a bit overkill.  But
> > I don't exactly how worried we are about cycles on travis (I don't
> > check it very often personally, so I don't really know).  If we aren't
> > worried about cycles what you have below would certainly make sense.
> 
> I think 64-bit gcc/clang builds tend to cost us about 10-20 minutes
> each, and 32-bit linux builds about 10 minutes or so.  I wonder if
> it makes sense to do the "run twice" for only one of 64-bit builds
> (perhaps the clang one, as I suspect 32-bit linux one uses gcc) and
> the 32-bit linux build, and nowhere else.

Yeah I'd be happy with something like that.  And I see there's some
more discussion about it in another part of the thread, will follow up
there.  Thanks!


Re: Apparent bug in 'git stash push ' loses untracked files

2017-12-13 Thread Thomas Gummerer
On 12/13, Igor Djordjevic wrote:
> Hi Reid,
> 
> On 13/12/2017 18:32, Reid Price wrote:
> > 
> > When running 'git stash push ' if there are both tracked and
> > untracked files in this subdirectory, the tracked files are stashed
> > but the untracked files are discarded.
> 
> I can reproduce this as well (git version 2.15.1.windows.2).
> 
> For what it`s worth, using `git stash save ` instead seems to 
> (still) work as expected...

I think that depends on what you expect ;)  'git stash save '
will create a stash of the whole working directory with the message
"".  So while it would indeed work for the presumably
simplified example Reid provided, it would not do what you'd expect if
there are any tracked and modified files outside of the .

In that case 'git stash save ' would include the tracked files
outside of , while what I assume Reid wanted is to keep them
in place, and only stash the files in .

> but on the other hand, `git-stash`[1] 
> manpage seems not to mention this usage ("save" with "pathspec")?

"stash save" with "pathspec" doesn't exist, and it will probably never
exist.  We decided to introduce a new "push" verb for 'git stash'
because the command line for 'git stash save' takes a message as its
last argument, instead of taking the message with a -m flag like other
commands do.  Introducing a pathspec argument for "git stash save"
would have either broken backward compatibility, or it would have had
some syntax that's very inconsistent with other git commands.

> Regards, Buga
> 
> [1] https://git-scm.com/docs/git-stash


Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

2017-12-13 Thread Lars Schneider

> On 12 Dec 2017, at 19:43, SZEDER Gábor  wrote:
> 
> On Tue, Dec 12, 2017 at 7:00 PM, Lars Schneider
>  wrote:
>> 
>>> On 12 Dec 2017, at 00:34, SZEDER Gábor  wrote:
>>> 
>>> While the build logic was embedded in our '.travis.yml', Travis CI
>>> used to produce a nice trace log including all commands executed in
>>> those embedded scriptlets.  Since 657343a60 (travis-ci: move Travis CI
>>> code into dedicated scripts, 2017-09-10), however, we only see the
>>> name of the dedicated scripts, but not what those scripts are actually
>>> doing, resulting in a less useful trace log.  A patch later in this
>>> series will move setting environment variables from '.travis.yml' to
>>> the 'ci/*' scripts, so not even those will be included in the trace
>>> log.
>>> 
>>> Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other
>>> 'ci/*' scripts, so we get trace log about the commands executed in all
>>> of those scripts.
>> 
>> I kind of did that intentionally to avoid clutter in the logs.
>> However, I also agree with your reasoning. Therefore, the change
>> looks good to me!
> 
> Great, 'cause I'm starting to have second thoughts about this change :)
> 
> It sure helped a lot while I worked on this patch series and a couple of
> other Travis CI related patches (will submit them later)...  OTOH it
> definitely creates clutter in the trace log.  The worst offender might
> be 'ci/print-test-failures.sh', which iterates over all
> 't/test-results/*.exit' files to find which tests failed and to show
> their output, and 'set -x' makes every iteration visible.  And we have
> about 800 tests, which means 800 iterations.  Yuck.
> 
> Perhaps we should use other means to show what's going on instead, e.g.
> use more 'echo's and '--verbose' options, or just avoid using '--quiet'.
> And if some brave souls really want to tweak '.travis.yml' or the 'ci/*'
> scripts, then they can set 'set -x' for themselves during development...
> as the patch below shows it's easy enough, just a single character :)

Hm... in that case. Would it be an option to "set -x" only in the header
of "install-dependencies.sh"?

In "lib-travisci.sh" we could keep your "set -x" and execute
"set +x" at the end of the file. Wouldn't that give us the 
interesting traces without much clutter (e.g. what is $PATH etc)?

- Lars


> 
> 
> Gábor
> 
> 
>>> 
>>> Signed-off-by: SZEDER Gábor 
>>> ---
>>> ci/lib-travisci.sh | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
>>> index ac05f1f46..a0c8ae03f 100755
>>> --- a/ci/lib-travisci.sh
>>> +++ b/ci/lib-travisci.sh
>>> @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () {
>>> 
>>> # Set 'exit on error' for all CI scripts to let the caller know that
>>> # something went wrong
>>> -set -e
>>> +set -ex
>>> 
>>> skip_branch_tip_with_tag
>>> 
>>> --
>>> 2.15.1.421.gc469ca1de
>>> 
>> 



Re: Apparent bug in 'git stash push ' loses untracked files

2017-12-13 Thread Thomas Gummerer
Hi,

On 12/13, Reid Price wrote:
> When running 'git stash push ' if there are both tracked and
> untracked files in this subdirectory, the tracked files are stashed
> but the untracked files are discarded.
> 
> I can reproduce this on my system (OSX, git 2.14.1) by running the
> below script as
> 
> bash -x ./stashbug.sh &> output.txt
> 
> I could not find this indicated anywhere as an existing issue by
> performing generic searches, apologies if this is known.

Thanks for the detailed bug report below.  This is indeed a bug, and
indeed has been broken ever since I introduced the pathspec feature.
Sorry about that.

While I did implement this feature in the first place, I must admit
that I'm a bit out of my depth in terms of my shell foo to fix this
cleanly here.

Just to demonstrate what is wrong, the below diff fixes it (but it
does so in an ugly way that needs a temporary file, and needs a
specific version of the 'read' utility, that supports the '-d' flag,
which isn't in POSIX).

For the pathspec case what we're doing after creating the stash is to
essentially emulate what 'git reset --hard ' would do.  As
'git reset --hard ' doesn't exist, we do so using the
sequence below of reset -> checkout-index -> clean for the given
pathspec.

This works when the pathspec doesn't match any untracked files, but if
it matches untracked files such as in your case it removes them as
well as the files it should have removed.

One solution to that would be to limit the pathspec given to 'git
clean'.  And that is where my shell foo doesn't go far enough for me
being able to do that, as lists of filenames, which can include spaces
tend to get quite hairy.

Maybe the best solution would be to introduce 'git reset --hard --
', or maybe someone who knows shell programming a little
better than me has an idea? 

diff --git a/git-stash.sh b/git-stash.sh
index 1114005ce2..01bf74015e 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -322,10 +322,15 @@ push_stash () {
 
if test $# != 0
then
+   git ls-files -z >"$(git rev-parse 
--git-dir)"/stash-to-remove
git reset -q -- "$@"
git ls-files -z --modified -- "$@" |
git checkout-index -z --force --stdin
-   git clean --force -q -d -- "$@"
+   while read -r -d '' to_delete
+   do
+   git clean --force -q -d -- "$to_delete"
+   done <"$(git rev-parse --git-dir)"/stash-to-remove
+   rm "$(git rev-parse --git-dir)"/stash-to-remove
else
git reset --hard -q
fi
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 39c7f2ebd7..6952a031b2 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1064,4 +1064,20 @@ test_expect_success 'stash -k --  leaves 
unstaged files intact' '
test foo,bar = $(cat foo),$(cat bar)
 '
 
+test_expect_success 'stash --  leaves untracked files in subdir 
intact' '
+   git reset &&
+   >subdir/untracked &&
+   >subdir/tracked1 &&
+   >subdir/tracked2 &&
+   git add subdir/tracked* &&
+   git stash -- subdir/ &&
+   test_path_is_missing subdir/tracked1 &&
+   test_path_is_missing subdir/tracked2 &&
+   test_path_is_file subdir/untracked &&
+   git stash pop &&
+   test_path_is_file subdir/tracked1 &&
+   test_path_is_file subdir/tracked2 &&
+   test_path_is_file subdir/untracked
+'
+
 test_done

>   -Reid
> 
> Contents of stashbug.sh
> 
> #!/bin/sh
> 
> uname -a
> git --version
> mkdir -p stashbug
> cd stashbug
> git init
> mkdir dir
> touch dir/tracked
> git add dir/tracked
> git commit -m 'initial'
> tree; git status
> mkdir dir/untracked_dir
> touch dir/untracked_dir/casualty1
> touch dir/casualty2
> echo 'contents' > dir/tracked
> tree; git status
> git stash push dir/
> git stash show -v
> tree; git status
> git stash pop
> tree; git status
> 
> 
> Resulting output.txt
> -
> + uname -a
> Darwin Reids-MacBook-Pro.local 15.6.0 Darwin Kernel Version
> 15.6.0: Tue Apr 11 16:00:51 PDT 2017;
> root:xnu-3248.60.11.5.3~1/RELEASE_X86_64 x86_64
> + git --version
> git version 2.14.1
> + mkdir -p stashbug
> + cd stashbug
> + git init
> Initialized empty Git repository in /Users/reid/git/stashbug/.git/
> + mkdir dir
> + touch dir/tracked
> + git add dir/tracked
> + git commit -m initial
> [master (root-commit) 895197e] initial
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  create mode 100644 dir/tracked
> + tree
> .
> └── dir
> └── tracked
> 
> 1 directory, 1 file
> + git status
> On branch master
> nothing to commit, working tree clean
> + mkdir 

Re: [PATCH v1] convert: add support for 'encoding' attribute

2017-12-13 Thread Lars Schneider

> On 13 Dec 2017, at 19:11, Junio C Hamano  wrote:
> 
> Lars Schneider  writes:
> 
>> ... In a perfect world I think I would store
>> the encoding of a file in the tree object. I didn't pursue that solution
>> as this would change the Git data model which would open a can of worms
>> for a problem that not that many people have (almost everyone is on
>> UTF-8 anyways).
> 
> Having that "encoding" trailt recorded in the tree that contains the
> blob would mean that the same blob can be recorded with one
> "encoding" trait in a tree, and in a different tree it can be
> recorded with a different "encoding" trait.  I doubt it really makes
> sense.

The "blob object" would store the text data encoded in a canonical 
UTF-8 form. The "tree object" would store the encoding. On checkin 
Git would convert the text from the stored encoding to UTF-8 and on 
checkout it would do the reverse.

That way you could control the encoding for a text file specific
for each path similar to the "mode bits". That also means you could
change the encoding of a file while the blob content stays the same.

Changing the encoding of a file with the .gitattributes approach
can be difficult if you have configured the attribute with a very 
broad pattern (e.g. *.foo). You would either need to rename the
file or limit the scope of your pattern.

- Lars


What's cooking in git.git (Dec 2017, #03; Wed, 13)

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

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

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

--
[Graduated to "master"]

* ab/pcre2-grep (2017-11-24) 2 commits
  (merged to 'next' on 2017-12-05 at 88f1927207)
 + grep: fix segfault under -P + PCRE2 <=10.30 + (*NO_JIT)
 + test-lib: add LIBPCRE1 & LIBPCRE2 prerequisites

 "git grep" compiled with libpcre2 sometimes triggered a segfault,
 which is being fixed.


* bc/hash-algo (2017-11-28) 5 commits
  (merged to 'next' on 2017-11-28 at 5c74632345)
 + repository: fix a sparse 'using integer as NULL pointer' warning
  (merged to 'next' on 2017-11-27 at 98cc9ef2a8)
 + Switch empty tree and blob lookups to use hash abstraction
 + Integrate hash algorithm support with repo setup
 + Add structure representing hash algorithm
 + setup: expose enumerated repo info

 An infrastructure to define what hash function is used in Git is
 introduced, and an effort to plumb that throughout various
 codepaths has been started.


* ds/for-each-file-in-obj-micro-optim (2017-12-04) 1 commit
  (merged to 'next' on 2017-12-05 at 55ba487ccd)
 + sha1_file: use strbuf_add() instead of strbuf_addf()

 The code to iterate over loose object files got optimized.


* en/remove-stripspace (2017-12-05) 1 commit
  (merged to 'next' on 2017-12-06 at c926373a49)
 + strbuf: remove unused stripspace function alias

 An internal function that was left for backward compatibility has
 been removed, as there is no remaining callers.


* fk/sendmail-from-path (2017-11-28) 1 commit
  (merged to 'next' on 2017-12-05 at deb7a5f4a8)
 + git-send-email: honor $PATH for sendmail binary

 "git send-email" tries to see if the sendmail program is available
 in /usr/lib and /usr/sbin; extend the list of locations to be
 checked to also include directories on $PATH.


* jc/receive-pack-hook-doc (2017-11-24) 1 commit
  (merged to 'next' on 2017-12-05 at ffa14b1d48)
 + hooks doc: clarify when receive-pack invokes its hooks

 Doc update.


* jk/no-optional-locks (2017-11-27) 1 commit
  (merged to 'next' on 2017-12-06 at e642dde605)
 + git-status.txt: mention --no-optional-locks

 Doc update for a feature available in Git v2.14 and upwards.


* jk/progress-delay-fix (2017-12-04) 2 commits
  (merged to 'next' on 2017-12-05 at 8e62c2b18b)
 + progress: drop delay-threshold code
 + progress: set default delay threshold to 100%, not 0%

 A regression in the progress eye-candy was fixed.


* js/hashmap-update-sample (2017-12-05) 1 commit
  (merged to 'next' on 2017-12-06 at 40ceee9a18)
 + hashmap: adjust documentation to reflect reality

 Code comment update.


* ks/doc-checkout-previous (2017-11-28) 1 commit
  (merged to 'next' on 2017-12-05 at 02f17e3c55)
 + Doc/checkout: checking out using @{-N} can lead to detached state

 @{-N} in "git checkout @{-N}" may refer to a detached HEAD state,
 but the documentation was not clear about it, which has been fixed.


* pc/submodule-helper (2017-11-26) 1 commit
  (merged to 'next' on 2017-12-05 at fdf56787be)
 + submodule--helper.c: i18n: add a missing space in message

 A message fix.


* ra/decorate-limit-refs (2017-11-22) 1 commit
  (merged to 'next' on 2017-12-05 at 02c66aa4e0)
 + log: add option to choose which refs to decorate

 The tagnames "git log --decorate" uses to annotate the commits can
 now be limited to subset of available refs with the two additional
 options, --decorate-refs[-exclude]=.


* tg/t-readme-updates (2017-11-27) 2 commits
  (merged to 'next' on 2017-12-05 at c0b0e2d65b)
 + t/README: document test_cmp_rev
 + t/README: remove mention of adding copyright notices

 Developer doc updates.

--
[New Topics]

* es/worktree-checkout-hook (2017-12-07) 1 commit
 - worktree: invoke post-checkout hook (unless --no-checkout)

 "git worktree add" learned to run the post-checkout hook, just like
 "git checkout" does, after the initial checkout.

 Will merge to 'next'.


* rs/am-builtin-leakfix (2017-12-07) 1 commit
 - am: release strbuf after use in split_mail_mbox()

 Leakfix.

 Will merge to 'next'.


* rs/fmt-merge-msg-string-leak-fix (2017-12-07) 1 commit
 - fmt-merge-msg: avoid leaking strbuf in shortlog()

 Leakfix.

 Will merge to 'next'.


* rs/strbuf-read-once-reset-length (2017-12-07) 1 commit
 - strbuf: release memory on read error in strbuf_read_once()

 Leakfix.

 Will merge to 'next'.


* db/doc-workflows-neuter-the-maintainer (2017-12-08) 1 commit
 - doc: reword gitworkflows.txt for neutrality

 Docfix.

 Will merge to 'next'.


* es/clone-shared-worktree (2017-12-11) 1 commit
 - clone: support 'clone --shared' from a worktree

 

Re: [PATCH] partial-clone: design doc

2017-12-13 Thread Jeff Hostetler



On 12/8/2017 3:14 PM, Junio C Hamano wrote:

Jeff Hostetler  writes:


From: Jeff Hostetler 

First draft of design document for partial clone feature.

Signed-off-by: Jeff Hostetler 
Signed-off-by: Jonathan Tan 
---


Thanks.


+Non-Goals
+-
+
+Partial clone is independent of and not intended to conflict with
+shallow-clone, refspec, or limited-ref mechanisms since these all operate
+at the DAG level whereas partial clone and fetch works *within* the set
+of commits already chosen for download.


It probably is not a huge deal (simply because it is about
"Non-Goals") but I have no idea what "refspec" and "limited-ref
mechanism" refer to in the above sentence, and I suspect many others
share the same puzzlement.


I'll reword this.  There was a question on the list earlier about
having a filter for commits in addition to ones for blobs and trees.

I just wanted to emphasize that we already have ways to filter or
limit commits using --shallow-* or --single-branch in clone and 1 or
more '' args in fetch.

 

+An object may be missing due to a partial clone or fetch, or missing due
+to repository corruption. To differentiate these cases, the local
+repository specially indicates packfiles obtained from the promisor
+remote. These "promisor packfiles" consist of a ".promisor" file
+with arbitrary contents (like the ".keep" files), in addition to
+their ".pack" and ".idx" files. (In the future, this ability
+may be extended to loose objects[a].)
+ ...
+Foot Notes
+--
+
+[a] Remembering that loose objects are promisor objects is mainly
+important for trees, since they may refer to promisor blobs that
+the user does not have.  We do not need to mark loose blobs as
+promisor because they do not refer to other objects.


I fail to see any logical link between the "loose" and "tree".
Putting it differently, I do not see why "tree" is so special.

A promisor pack that contains a tree but lacks blobs the tree refers
to would be sufficient to let us remember that these missing blobs
are not corruption.  A loose commit or a tag that is somehow marked
as obtained from a promisor, if it can serve just like a commit or a
tag in a promisor pack to promise its direct pointee, would equally
be useful (if very inefficient).

In any case, I suspect "since they may refer to promisor blobs" is a
typo of "since they may refer to promised blobs".


right. good point. i was only thinking about the tree==>blob
relationship.





+- Currently, dynamic object fetching invokes fetch-pack for each item
+  because most algorithms stumble upon a missing object and need to have
+  it resolved before continuing their work.  This may incur significant
+  overhead -- and multiple authentication requests -- if many objects are
+  needed.
+
+  We need to investigate use of a long-running process, such as proposed
+  in [5,6] to reduce process startup and overhead costs.


Also perhaps in some operations we can enumerate the objects we will
need upfront and ask for them in one go (e.g. "git log -p A..B" may
internally want to do "rev-list --objects A..B" to enumerate trees
and blobs that we may lack upfront).  I do not think having the
other side guess is a good idea, though.


right.




+- We currently only promisor packfiles.  We need to add support for
+  promisor loose objects as described earlier.


The earlier description was not convincing enough to feel the need
to me; at least not yet.


It seems like we need it if a promisor packfile gets unpacked for any
reason.  But right, I'm not sure how urgent it is.


Thanks
Jeff




Re: Apparent bug in 'git stash push ' loses untracked files

2017-12-13 Thread Igor Djordjevic
Hi Reid,

On 13/12/2017 18:32, Reid Price wrote:
> 
> When running 'git stash push ' if there are both tracked and
> untracked files in this subdirectory, the tracked files are stashed
> but the untracked files are discarded.

I can reproduce this as well (git version 2.15.1.windows.2).

For what it`s worth, using `git stash save ` instead seems to 
(still) work as expected... but on the other hand, `git-stash`[1] 
manpage seems not to mention this usage ("save" with "pathspec")?

Regards, Buga

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


Re: [PATCH 4/8] perf/run: use $default_value instead of $4

2017-12-13 Thread Junio C Hamano
Junio C Hamano  writes:

> If you want to be able to use this helper to specify a default value
> of an empty string (which the orignal that used $4 did), then the
> previous hunk must be corrected so that it does not unconditionally
> set default_value to $4.  Perhaps like
>
>   if test -n "${4+x}"
>   then
>   default_value=$4
>   else
>   unset default_value || :
>   fi
>
> or something.

And if you do not care about passing an empty string as the default
value, then the earlier hunk can stay the same

default_value=$4

but the eval part should become something like

test -n "$default_value" && eval ...

Given that you are planning to add yet another optional argument to
the helper, and when you pass that variable, you'd probably need to
pass "" as the "default" that is not exported, perhaps this "give up
ability to pass an empty string as the default" approach may be the
only thing you could do.



Re: [PATCH 4/8] perf/run: use $default_value instead of $4

2017-12-13 Thread Junio C Hamano
Christian Couder  writes:

> Signed-off-by: Christian Couder 
> ---
>  t/perf/run | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/perf/run b/t/perf/run
> index 43e4de49ef..bbd703dc4f 100755
> --- a/t/perf/run
> +++ b/t/perf/run
> @@ -105,7 +105,7 @@ get_var_from_env_or_config () {
>   env_var="$1"
>   conf_sec="$2"
>   conf_var="$3"
> - # $4 can be set to a default value
> + default_value="$4" # optional
>  
>   # Do nothing if the env variable is already set
>   eval "test -z \"\${$env_var+x}\"" || return
> @@ -123,7 +123,7 @@ get_var_from_env_or_config () {
>   conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") &&
>   eval "$env_var=\"$conf_value\"" && return
>  
> - test -n "${4+x}" && eval "$env_var=\"$4\""
> + test -n "${default_value+x}" && eval "$env_var=\"$default_value\""

This conversion changes the behaviour.  Because default_value is
always set by your change in the previous hunk, we end up always
doing this eval.

The original says "If $4 is set, then ${4+x} becomes x and if $4 is
not set, ${4+x} is empty, so let's check if ${4+x} is a non-empty
string to see if $4 is set.  If ${4+x} is a non-empty string, that
means $4 was set so we do the eval.

If you want to be able to use this helper to specify a default value
of an empty string (which the orignal that used $4 did), then the
previous hunk must be corrected so that it does not unconditionally
set default_value to $4.  Perhaps like

if test -n "${4+x}"
then
default_value=$4
else
unset default_value || :
fi

or something.

>  }
>  
>  run_subsection () {


Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output

2017-12-13 Thread Junio C Hamano
Christian Couder  writes:

>  my $resultsdir = "test-results";
> +my $results_section = "";
>  if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
>   $resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
> + $results_section = $ENV{GIT_PERF_SUBSECTION};
>  }

...

> + my $executable;
> + if ($results_section eq "") {
> + $executable = `uname -o -p`;
> + } else {
> + $executable = $results_section;
> + chomp $executable;
> + }

Aside from portability of 'uname -o' Eric raised, I wonder if the
platform information is still useful even when perf-subsection is
specified.  With the above code, we can identify that a single
result is for (say) MacOS only when we are not limiting to a single
subsection, but wouldn't it be equally a valid desire to be able to
track performance figures for a single subsection over time and
being able to say "On MacOS, subsection A's performance dropped
between release X and X+1 quite a bit, but on Linux x86-64, there
was no such change" or somesuch?

IOW, shouldn't the "executable" label always contain the platform
information, plus an optional subsection info when (and only when)
the result is limited to a subsection?

By the way, $results_section that is not an empty string at this
point must have come from $ENV{GIT_PERF_SUBSECTION}, and from the
way the environment variable is used in t/perf/run, e.g.

(
GIT_PERF_SUBSECTION="$subsec"
export GIT_PERF_SUBSECTION
echo " Run for subsection 
'$GIT_PERF_SUBSECTION' "
run_subsection "$@"
)

I do not think we expect it to have a trailing LF.  What's that
chomp doing there?


[PATCH] t/helper: ignore everything but sources

2017-12-13 Thread Stefan Beller
Compiled test helpers in t/helper are out of sync with the .gitignore
files quite frequently. This can happen when new test helpers are added,
but the explicit .gitignore file is not updated in the same commit, or
when you forget to 'make clean' before checking out a different version
of git, as the different version may have a different explicit list of
test helpers to ignore.

Fix this by having an overly broad ignore pattern in that directory:
Anything, except C and shell source, will be ignored.

Signed-off-by: Stefan Beller 
---

Thanks Todd and Junio for mentioning the .gitignore file.


 t/helper/.gitignore | 44 
 1 file changed, 4 insertions(+), 40 deletions(-)

diff --git a/t/helper/.gitignore b/t/helper/.gitignore
index d02f9b39ac..5b540625af 100644
--- a/t/helper/.gitignore
+++ b/t/helper/.gitignore
@@ -1,40 +1,4 @@
-/test-chmtime
-/test-ctype
-/test-config
-/test-date
-/test-delta
-/test-drop-caches
-/test-dump-cache-tree
-/test-dump-fsmonitor
-/test-dump-split-index
-/test-dump-untracked-cache
-/test-fake-ssh
-/test-scrap-cache-tree
-/test-genrandom
-/test-hashmap
-/test-index-version
-/test-lazy-init-name-hash
-/test-line-buffer
-/test-match-trees
-/test-mergesort
-/test-mktemp
-/test-online-cpus
-/test-parse-options
-/test-path-utils
-/test-prio-queue
-/test-read-cache
-/test-ref-store
-/test-regex
-/test-revision-walking
-/test-run-command
-/test-sha1
-/test-sha1-array
-/test-sigchain
-/test-strcmp-offset
-/test-string-list
-/test-submodule-config
-/test-subprocess
-/test-svn-fe
-/test-urlmatch-normalization
-/test-wildmatch
-/test-write-cache
+*
+!*.[ch]
+!*.sh
+!.gitignore
-- 
2.15.1.504.g5279b80103-goog



From Mr. Chow

2017-12-13 Thread kovacsne . dora


Good day

I emailed you earlier without any response from you, in my first email I 
mentioned about my late client, whose relatives I cannot get in touch with. He 
deposited a large sum of funds in our bank. I am in a position to front you as 
the next of kin to late client and move funds to you as a foreigner legally. If 
you are interested let me know so that i can send you a comprehensive details.

Sincerely,

Mr . Edward chow,
edwardchow1...@yahoo.com



Re: [PATCH] doc: clarify usage of XDG_CONFIG_HOME config file

2017-12-13 Thread Junio C Hamano
Yaroslav Halchenko  writes:

> It always read it for non--global
> ...
> and it doesn't read it for --global
> ...
> unless ~/.gitconfig is missing

Yes, this dates back to 21cf3227 ("config: read (but not write) from
$XDG_CONFIG_HOME/git/config file", 2012-06-22), around the time back
when we added support to use xdg locations and doing so without breaking
existing users.

Taken together with a later commit in the same series 0e8593dc
("config: write to $XDG_CONFIG_HOME/git/config file when
appropriate", 2012-06-22), which says:

config: write to $XDG_CONFIG_HOME/git/config file when appropriate

Teach git to write to $XDG_CONFIG_HOME/git/config if

 - it already exists,
 - $HOME/.gitconfig file doesn't, and
 - The --global option is used.

Otherwise, write to $HOME/.gitconfig when the --global option is
given, as before.

If the user doesn't create $XDG_CONFIG_HOME/git/config, there is
absolutely no change. Users can use this new file only if they want.

If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/config
will be used.

Advice for users who often come back to an old version of Git: you
shouldn't create this file.

the plan is to have either one of these and not both at the same
time.  A user who wants to live in xdg world (and wants to avoid
cluttering $HOME with .many-files) can do so by creating an empty
one there, and all writes from there on go to the xdg world; as
there will no $HOME/.gitconfig created, the read side attempting to
read from it does not matter.  Other users get $HOME/.gitconfig when
running "config --global" to write for the first time, or users who
have been using Git from olden days already have $HOME/.gitconfig,
and no write goes to xdg world, so again the read side attempting to
read from there does not matter, either.

Perhaps a doc update needs to clarify these.

Thanks.


Re: [PATCH] path: document path functions

2017-12-13 Thread Junio C Hamano
Brandon Williams  writes:

> Signed-off-by: Brandon Williams 
> ---
>
> As promised here is an update to the documentation for the path generating
> functions.
>
>  path.h | 133 
> ++---
>  1 file changed, 112 insertions(+), 21 deletions(-)
>
> diff --git a/path.h b/path.h
> index 9541620c7..1ccd0373c 100644
> --- a/path.h
> +++ b/path.h
> @@ -4,53 +4,144 @@
>  struct repository;
>  
>  /*
> - * Return a statically allocated filename, either generically (mkpath), in
> - * the repository directory (git_path), or in a submodule's repository
> - * directory (git_path_submodule). In all cases, note that the result
> - * may be overwritten by another call to _any_ of the functions. Consider
> - * using the safer "dup" or "strbuf" formats below (in some cases, the
> - * unsafe versions have already been removed).
> + * The result to all functions which return statically allocated memory may 
> be
> + * overwritten by another call to _any_ one of these functions. Consider 
> using
> + * the safer variants which operate on strbufs or return allocated memory.
>   */

The result of the update definitely is better, but stepping back a
bit, it still does not answer some questions the original did not:

 - is the resulting path absolute?  if relative, relative to what (cwd)?

 - what should a caller write in fmt?  can we have a couple of
   typical calling example?

 - does a caller need to know which hierarchies under $GIT_DIR are
   common and call git_common_path() for them, or does git_path()
   do the 'right' thing for the caller?  If latter, what's the use
   of git_common_path() for callers (outside the implementation of
   path.c API)?

Will queue.  Thanks.


Re: [PATCH v4 00/34] Add directory rename detection to git

2017-12-13 Thread Junio C Hamano
Ramsay Jones  writes:

> On 13/12/17 01:06, Junio C Hamano wrote:
>
>> We may probably want to redirect the output of underlying grep to
>> /dev/null in test_i18ngrep to make this kind of misuse easier to
>> spot.
>
> I have test-suite failures on the 'pu' branch for t4151-am-abort.sh
> (#3 and #6) and t5536-fetch-conflicts.sh (#3 and #6-7), which on a
> very quick inspection seem to be due to this (ie your SQUASH commit
> e5c5e24ad9).

Yup, sorry for leaving it broken this morning.  I re-pushed after
dropping the /dev/null experiment to fix it.  In the longer term, I
think we should make sure that output from test_i18ngrep is never
used (i.e. t4151 etc. must be fixed), though.



Re: [PATCH] doc: Modify git-add doc to say "staging area"

2017-12-13 Thread David A. Wheeler
On Wed, 13 Dec 2017 09:02:42 -0800, Junio C Hamano  wrote:
> .. But that is not the only thing the index does.  When "git merge"
> finds conflicting changes, it adds the contents for common, our and
> their variants to the index for the path.  This is quite different
> from how you use the index "as staging area"; the index is being
> used as the "merging area".  When "git clean" wants to see which
> paths it finds on the filesystem are not of interest, it consults
> the index, which acts as the list of paths that are of interest.

If the phrase "staging area" is consistently used *instead* of index,
there's no problem. E.g., "git clean consults the staging area"
conveys exactly the same information as "git clean consults the index"
when index == staging area.

The term "index" has too many *other* meanings.

--- David A. Wheeler


Re: [PATCH v6 3/3] rev-list: support --no-filter argument

2017-12-13 Thread Jeff Hostetler



On 12/11/2017 1:17 AM, Christian Couder wrote:

On Tue, Dec 5, 2017 at 5:50 PM, Jeff Hostetler  wrote:

From: Jeff Hostetler 

Teach rev-list to support --no-filter to override a
previous --filter= argument.  This is
to be consistent with commands that use OPT_PARSE
macros.

Signed-off-by: Jeff Hostetler 
---
  Documentation/rev-list-options.txt | 15 ++-
  builtin/rev-list.c |  4 
  2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 11bb87f..8d8b7f4 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -715,16 +715,21 @@ ifdef::git-rev-list[]
  The form '--filter=blob:none' omits all blobs.
  +
  The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes
-or units.  The value may be zero.
+or units.  n may be zero.  The suffixes k, m, and g can be used to name


"'' may be zero" would be more consistent with other parts of this file.
s/k, m, and g/'k', 'm', and 'g'/ could also help.


good catch. thanks.
jeff



[PATCH] path: document path functions

2017-12-13 Thread Brandon Williams
Signed-off-by: Brandon Williams 
---

As promised here is an update to the documentation for the path generating
functions.

 path.h | 133 ++---
 1 file changed, 112 insertions(+), 21 deletions(-)

diff --git a/path.h b/path.h
index 9541620c7..1ccd0373c 100644
--- a/path.h
+++ b/path.h
@@ -4,53 +4,144 @@
 struct repository;
 
 /*
- * Return a statically allocated filename, either generically (mkpath), in
- * the repository directory (git_path), or in a submodule's repository
- * directory (git_path_submodule). In all cases, note that the result
- * may be overwritten by another call to _any_ of the functions. Consider
- * using the safer "dup" or "strbuf" formats below (in some cases, the
- * unsafe versions have already been removed).
+ * The result to all functions which return statically allocated memory may be
+ * overwritten by another call to _any_ one of these functions. Consider using
+ * the safer variants which operate on strbufs or return allocated memory.
  */
-extern const char *mkpath(const char *fmt, ...) __attribute__((format (printf, 
1, 2)));
-extern const char *git_path(const char *fmt, ...) __attribute__((format 
(printf, 1, 2)));
-extern const char *git_common_path(const char *fmt, ...) __attribute__((format 
(printf, 1, 2)));
 
+/*
+ * Return a statically allocated path.
+ */
+extern const char *mkpath(const char *fmt, ...)
+   __attribute__((format (printf, 1, 2)));
+
+/*
+ * Return a path.
+ */
+extern char *mkpathdup(const char *fmt, ...)
+   __attribute__((format (printf, 1, 2)));
+
+/*
+ * Construct a path and place the result in the provided buffer `buf`.
+ */
 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
-extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
-   __attribute__((format (printf, 2, 3)));
+
+/*
+ * The `git_common_path` family of functions will construct a path into a
+ * repository's common git directory, which is shared by all worktrees.
+ */
+
+/*
+ * Constructs a path into the common git directory of repository `repo` and
+ * append it in the provided buffer `sb`.
+ */
 extern void strbuf_git_common_path(struct strbuf *sb,
   const struct repository *repo,
   const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
-extern char *git_path_buf(struct strbuf *buf, const char *fmt, ...)
-   __attribute__((format (printf, 2, 3)));
-extern int strbuf_git_path_submodule(struct strbuf *sb, const char *path,
-const char *fmt, ...)
-   __attribute__((format (printf, 3, 4)));
-extern char *git_pathdup(const char *fmt, ...)
-   __attribute__((format (printf, 1, 2)));
-extern char *mkpathdup(const char *fmt, ...)
+
+/*
+ * Return a statically allocated path into the main repository's
+ * (the_repository) common git directory.
+ */
+extern const char *git_common_path(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
-extern char *git_pathdup_submodule(const char *path, const char *fmt, ...)
-   __attribute__((format (printf, 2, 3)));
 
+
+/*
+ * The `git_path` family of functions will construct a path into a repository's
+ * git directory.
+ *
+ * These functions will perform adjustments to the resultant path to account
+ * for special paths which are either considered common among worktrees (e.g.
+ * paths into the object directory) or have been explicitly set via an
+ * environment variable or config (e.g. path to the index file).
+ *
+ * For an exhaustive list of the adjustments made look at `common_list` and
+ * `adjust_git_path` in path.c.
+ */
+
+/*
+ * Return a path into the git directory of repository `repo`.
+ */
 extern char *repo_git_path(const struct repository *repo,
   const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+
+/*
+ * Construct a path into the git directory of repository `repo` and append it
+ * to the provided buffer `sb`.
+ */
 extern void strbuf_repo_git_path(struct strbuf *sb,
 const struct repository *repo,
 const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
 
+/*
+ * Return a statically allocated path into the main repository's
+ * (the_repository) git directory.
+ */
+extern const char *git_path(const char *fmt, ...)
+   __attribute__((format (printf, 1, 2)));
+
+/*
+ * Return a path into the main repository's (the_repository) git directory.
+ */
+extern char *git_pathdup(const char *fmt, ...)
+   __attribute__((format (printf, 1, 2)));
+
+/*
+ * Construct a path into the main repository's (the_repository) git directory
+ * and place it in the provided buffer `buf`, the contents of the buffer will
+ * be overridden.
+ */
+extern char *git_path_buf(struct strbuf *buf, const char *fmt, ...)
+ 

Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output

2017-12-13 Thread Eric Sunshine
On Wed, Dec 13, 2017 at 10:13 AM, Christian Couder
 wrote:
> Codespeed (https://github.com/tobami/codespeed/) is an open source
> project that can be used to track how some software performs over
> time. It stores performance test results in a database and can show
> nice graphs and charts on a web interface.
>
> As it can be interesting to Codespeed to see how Git performance
> evolves over time and releases, let's implement a Codespeed output
> in "perf/aggregate.perl".
>
> Signed-off-by: Christian Couder 
> ---
> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
> @@ -174,6 +181,63 @@ sub print_default_results {
> +sub print_codespeed_results {
> +   my ($results_section) = @_;
> +
> +   my $project = "Git";
> +
> +   my $executable;
> +   if ($results_section eq "") {
> +   $executable = `uname -o -p`;

Option -o is not recognized on MacOS and, on at least a couple of my
Linux installations, -p returns only "unknown".

A combination, on the other hand, which seems to work nicely on MacOS,
Linux, and BSD is: uname -s -m


Re: [PATCH v4 00/34] Add directory rename detection to git

2017-12-13 Thread Ramsay Jones


On 13/12/17 01:06, Junio C Hamano wrote:
> Elijah Newren  writes:
> 
>> This patchset introduces directory rename detection to merge-recursive.
> 
> The use of negated form of test_i18ngrep in these patches are all
> wrong.  Because the helper must say "even though the string does not
> match (does match), the test expects it to match (does not match),
> and we know that expectation won't hold simply because we are under
> poison build", so negating the result of test_i18ngrep won't work.
> Instead, you would tell test_i18ngrep that we do not expect it to
> find matching lines.
> 
> Even with the attached, test #70 will still fail because you have a
> construct that greps in output of test_i18ngrep.  That won't work
> under poison build, because the output of test_i18ngrep won't have
> the string you are looking for under poison build.
> 
> We may probably want to redirect the output of underlying grep to
> /dev/null in test_i18ngrep to make this kind of misuse easier to
> spot.

I have test-suite failures on the 'pu' branch for t4151-am-abort.sh
(#3 and #6) and t5536-fetch-conflicts.sh (#3 and #6-7), which on a
very quick inspection seem to be due to this (ie your SQUASH commit
e5c5e24ad9).

For t4151 the test (both 3 & 6) is using test_i18ngrep on the output
of a previous test_i18ngrep, and t5536 is doing likewise in the
verify_stderr() helper function.

ATB,
Ramsay Jones




Re: [PATCH v1] convert: add support for 'encoding' attribute

2017-12-13 Thread Junio C Hamano
Lars Schneider  writes:

> ... In a perfect world I think I would store
> the encoding of a file in the tree object. I didn't pursue that solution
> as this would change the Git data model which would open a can of worms
> for a problem that not that many people have (almost everyone is on
> UTF-8 anyways).

Having that "encoding" trailt recorded in the tree that contains the
blob would mean that the same blob can be recorded with one
"encoding" trait in a tree, and in a different tree it can be
recorded with a different "encoding" trait.  I doubt it really makes
sense.



Re: [PATCH v1] convert: add support for 'encoding' attribute

2017-12-13 Thread Lars Schneider

> On 12 Dec 2017, at 20:31, Junio C Hamano  wrote:
> 
> Lars Schneider  writes:
> 
>> Our favorite is "treat-encoding-as". Do you consider this better
>> or worse than "checkout-encoding"?
> 
> I am afraid that "treat as" is not sufficiently specific and would
> invite a misinterpretation, e.g. "You record the bytes I throw at
> you as-is in the object store, but treat them appropriately as
> contents that are encoded in cp1252 when presenting".
> 
> what is missing is at what stage in the overall user experience does
> that "treating" happens.  That causes such a misinterpretation.
> 
> So from that point of view, "checkout-" or" working-tree-" would be
> a better phrase to accompany "encoding" to clarify what this attr is
> for than "treat-as".

Alright. I'll go with "checkout-encoding" then.


> Having said all that, this "feature" would need a moderate amount of
> clear description in the documentation, and between the perfect and
> a suboptimal name, the amount of explanation required would not be
> all that different, I suspect.  We need to say that those who wish
> to use this attribute are buying into recording their contents in
> UTF-8 and when the contents are externalized to the working tree,
> they are converted to the encoding the value of this attribute
> specifies and vice versa.

Absolutely! The docs will come in v2. I just wanted to send out a v1
to make people aware of what I am working on.

I think storing the encoding in gitattributes might not be the perfect
solution but probably a pragmatic one (because it works and the changes
to Git are rather small). In a perfect world I think I would store
the encoding of a file in the tree object. I didn't pursue that solution
as this would change the Git data model which would open a can of worms
for a problem that not that many people have (almost everyone is on
UTF-8 anyways).

- Lars


Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

2017-12-13 Thread Lars Schneider

> On 13 Dec 2017, at 18:38, Junio C Hamano  wrote:
> 
> Lars Schneider  writes:
> 
>> I think your solution points into the right direction.
>> Right now we have the following test matrix:
>> 
>> 1. Linux - clang
>> 2. Linux - gcc
>> 3. Mac - clang
>> 4. Mac - gcc
>> 5. Linux - gcc - GET_TEXT_POISION
>> 6. Linux - gcc - 32bit
>> 7. Windows
>> 
>> AFAIK your solution would run the split index test for 
>> 1, 2, 3, and 4. I think that is too much.
> 
> Not that it matters too much, but I meant to add it to 1. and
> 6. when I said "only one of 64-bit build plus 32-bit one".  

Ah. Sorry, I didn't get that.


>> 1 runs the fastest and I would like to keep it that way
>> to get a quick "general" result. I think only 2 should be
>> extended in the way you are suggesting. We could run
>> the tests with different env variables there. What else
>> do we have besides GIT_TEST_SPLIT_INDEX?
>> 
>> Would that work for everyone?
> 
> I am OK to make 2. use split-index (which unfortunately would mean
> we lose tests without split-index under gcc), or add 2.5 that is a
> copy of 2. plus split-index.


I think I experessed myself poorly. As far as I understand it,
GIT_TEST_SPLIT_INDEX is an environment variable that only needs
to be set at test time and not at compile time. Is this right?

If yes, my idea for 2. is as follows:
- build Git with gcc
- run tests with "make --quiet test"
- run tests with "GIT_TEST_SPLIT_INDEX=YesPlease make --quiet test"

This should be quicker than a new 2.5 target because we don't need to
spin up the machine and build Git. Plus, we could run the tests
a few more times with other test flags.

- Lars


Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

2017-12-13 Thread Junio C Hamano
Lars Schneider  writes:

> I think your solution points into the right direction.
> Right now we have the following test matrix:
>
> 1. Linux - clang
> 2. Linux - gcc
> 3. Mac - clang
> 4. Mac - gcc
> 5. Linux - gcc - GET_TEXT_POISION
> 6. Linux - gcc - 32bit
> 7. Windows
>
> AFAIK your solution would run the split index test for 
> 1, 2, 3, and 4. I think that is too much.

Not that it matters too much, but I meant to add it to 1. and
6. when I said "only one of 64-bit build plus 32-bit one".  

> 1 runs the fastest and I would like to keep it that way
> to get a quick "general" result. I think only 2 should be
> extended in the way you are suggesting. We could run
> the tests with different env variables there. What else
> do we have besides GIT_TEST_SPLIT_INDEX?
>
> Would that work for everyone?

I am OK to make 2. use split-index (which unfortunately would mean
we lose tests without split-index under gcc), or add 2.5 that is a
copy of 2. plus split-index.


Apparent bug in 'git stash push ' loses untracked files

2017-12-13 Thread Reid Price
When running 'git stash push ' if there are both tracked and
untracked files in this subdirectory, the tracked files are stashed
but the untracked files are discarded.

I can reproduce this on my system (OSX, git 2.14.1) by running the
below script as

bash -x ./stashbug.sh &> output.txt

I could not find this indicated anywhere as an existing issue by
performing generic searches, apologies if this is known.

  -Reid

Contents of stashbug.sh

#!/bin/sh

uname -a
git --version
mkdir -p stashbug
cd stashbug
git init
mkdir dir
touch dir/tracked
git add dir/tracked
git commit -m 'initial'
tree; git status
mkdir dir/untracked_dir
touch dir/untracked_dir/casualty1
touch dir/casualty2
echo 'contents' > dir/tracked
tree; git status
git stash push dir/
git stash show -v
tree; git status
git stash pop
tree; git status


Resulting output.txt
-
+ uname -a
Darwin Reids-MacBook-Pro.local 15.6.0 Darwin Kernel Version
15.6.0: Tue Apr 11 16:00:51 PDT 2017;
root:xnu-3248.60.11.5.3~1/RELEASE_X86_64 x86_64
+ git --version
git version 2.14.1
+ mkdir -p stashbug
+ cd stashbug
+ git init
Initialized empty Git repository in /Users/reid/git/stashbug/.git/
+ mkdir dir
+ touch dir/tracked
+ git add dir/tracked
+ git commit -m initial
[master (root-commit) 895197e] initial
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 dir/tracked
+ tree
.
└── dir
└── tracked

1 directory, 1 file
+ git status
On branch master
nothing to commit, working tree clean
+ mkdir dir/untracked_dir
+ touch dir/untracked_dir/casualty1
+ touch dir/casualty2
+ echo contents
+ tree
.
└── dir
├── casualty2
├── tracked
└── untracked_dir
└── casualty1

2 directories, 3 files
+ git status
On branch master
Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   dir/tracked

Untracked files:
  (use "git add ..." to include in what will be committed)

dir/casualty2
dir/untracked_dir/

no changes added to commit (use "git add" and/or "git commit -a")
+ git stash push dir/
Saved working directory and index state WIP on master: 895197e initial
+ git stash show -v
diff --git a/dir/tracked b/dir/tracked
index e69de29..12f00e9 100644
--- a/dir/tracked
+++ b/dir/tracked
@@ -0,0 +1 @@
+contents
+ tree
.
└── dir
└── tracked

1 directory, 1 file
+ git status
On branch master
nothing to commit, working tree clean
+ git stash pop
On branch master
Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   dir/tracked

no changes added to commit (use "git add" and/or "git commit -a")
Dropped refs/stash@{0} (93ceee344b947ecd8a27a672e3aedd2b2e1acc99)
+ tree
.
└── dir
└── tracked

1 directory, 1 file
+ git status
On branch master
Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   dir/tracked

no changes added to commit (use "git add" and/or "git commit -a")
-


Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

2017-12-13 Thread Lars Schneider

> On 12 Dec 2017, at 20:15, Junio C Hamano  wrote:
> 
> Lars Schneider  writes:
> 
>>> You're right, it's my first time using travis CI and I got confused
>>> about how the .travis.yml works, thanks for catching that.  Will
>>> re-phrase the commit message.
>> 
>> Szeder is spot on. If you fix up the message, then this patch looks
>> perfect! Pragmatic (= very good!) idea to combine GITTEXT_POISON with 
>> GIT_TEST_SPLIT_INDEX :-)
> 
> I am failing to guess the real intent of the smiley here.

No real reason. I was just happy to see that Travis CI seems to
be useful for the Git project.


> If split-index code is so easy to break, I do not think it is a good
> idea to combine it into the poison build.  In fact, the poison test
> is useless on a codebase where other/real breakages are expected to
> exist, because it is about seeing messages meant for non-humans are
> not passed to the _() mechanism by sloppy coding, and the way it
> does so is to corrupt all the messages that come through the _()
> mechanism.  If we do not even produce a message when a correct code
> _should_ produce one, poison test would catch nothing useful.
> 
> I wonder if it makes more sense to update ci/run-tests.sh so that
> its final step is run twice with different settings, like so?

Agreed - I didn't think it through. Let's keep it separate then.

I think your solution points into the right direction.
Right now we have the following test matrix:

1. Linux - clang
2. Linux - gcc
3. Mac - clang
4. Mac - gcc
5. Linux - gcc - GET_TEXT_POISION
6. Linux - gcc - 32bit
7. Windows

AFAIK your solution would run the split index test for 
1, 2, 3, and 4. I think that is too much.

1 runs the fastest and I would like to keep it that way
to get a quick "general" result. I think only 2 should be
extended in the way you are suggesting. We could run
the tests with different env variables there. What else
do we have besides GIT_TEST_SPLIT_INDEX?

Would that work for everyone?

- Lars


> ci/run-tests.sh | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/ci/run-tests.sh b/ci/run-tests.sh
> index f0c743de94..15a5f5a6cc 100755
> --- a/ci/run-tests.sh
> +++ b/ci/run-tests.sh
> @@ -8,3 +8,4 @@
> mkdir -p $HOME/travis-cache
> ln -s $HOME/travis-cache/.prove t/.prove
> make --quiet test
> +GIT_TEST_SPLIT_INDEX=LetsTryIt make --quiet test



RE: feature-request: git "cp" like there is git mv.

2017-12-13 Thread Randall S. Becker
-Original Message-
On December 13, 2017 11:40 AM Johannes Schindelin wrote:
>On Tue, 12 Dec 2017, Simon Doodkin wrote:
>> please develop a new feature, git "cp" like there is git mv 
>> tomovefile1 tofile2 (to save space).
>> there is a solution in https://stackoverflow.com/a/44036771/466363
>> however, it is not single easy command.
>This is not how this project works. The idea is that it is Open Source, so
that you can develop this feature yourself, and contribute a patch.

Agree with Johannes. Let's help though, to quantify the requirements so that
Simon can get this right. I'm putting my tyrannical repository manager hat
on here rather than developer so...

Are you looking to have git cp copy the entire history of tomovefile1 to
tofile2 or just copy the content of tomovefile1 to tofile2 and add and/or
commit the file?

In the latter, I see the convenience of this capability. Even so, a simple
cp would copy the content and then you can commit it fairly easily. In the
former, copying the entire history of a file inside the repository is going
to potentially cause tofile2 to appear in old commits where prior to the git
cp command the file was not present? In this situation, you are actually
rewriting history and potentially impacting signed commits (which would no
longer pass a signature check, I hope). Stitching repositories is sometimes
done when repairs or reorganization is required, but I'm concerned that this
is opening up a can of worms that breaks the atomicity of commits
(particularly signed ones). What I don't want, for my own teams, is for
members to think that git cp would be a harmless (unless it actually is)
command, rather than a repair/reorg mechanism used for splitting apart a
repository, or copying a file to a new project then splitting selectively.
So, I'm obviously a bit confused about the goal.

Simon: the stackoverflow post provides a few options on this command. Can
you clarify which particular direction you are interest it?

Cheers,
Randall

-- Brief whoami: NonStop developer since approximately
UNIX(421664400)/NonStop(2112884442) 
-- In my real life, I talk too much.





Re: [PATCH] doc: Modify git-add doc to say "staging area"

2017-12-13 Thread Junio C Hamano
Jacob Keller  writes:

> I know we've used various terms for this concept across a lot of the
> documentation. However, I was under the impression that we most
> explicitly used "index" rather than "staging area".
>
> Additionally, I think there are many other locations which
> consistently use "index" as the term already.

Another reason why we would want to standardize in the other
direction is because "X acts as Y" does not mean "X is Y".

It is true that we (and also "newbie friendly" tutorials) often
explain the index like so: "When working towards the next commit,
you improve the contents a bit in the working tree, you 'git add' it
to the index to make the contents of the index closer to what you
want to have the next commit.  The index works like the staging
area."

But that is not the only thing the index does.  When "git merge"
finds conflicting changes, it adds the contents for common, our and
their variants to the index for the path.  This is quite different
from how you use the index "as staging area"; the index is being
used as the "merging area".  When "git clean" wants to see which
paths it finds on the filesystem are not of interest, it consults
the index, which acts as the list of paths that are of interest.


Greetings MY Dearest In The Lord

2017-12-13 Thread Mrs. Alla Babayeve
Greetings MY Dearest In The Lord

Please I know this may come to you by surprise ,because you do not know me, I 
need your assistance that is why I am writing you, it is my desire of going 
into relationship with you. I am Mrs. Alla Babayeve i am a Farmer/gold and 
diamond dealer 

I am a merchant of Russia nationality but presently residing in Kenya. I have 
been diagnosed with Esophageal cancer. i am currently admitted in a privet 
hospital,and I have some funds I inherited from my late loving husband Mr. 
Johnson Adams Babayeve, the sum of US$3.500.000 which he deposited in BANK Here 
and I need a very honest and The Lord fearing Christian that can use this funds 
for The Lord work and 20% out of the total funds will be for your compensation 
for doing this work of The Lord

I found your email address from the internet and decide to contact you. Please 
if you would be able to use these funds for the Lord's work kindly reply me 
Thank you and May The Lord bless you.
Your Sister In The Lord

Mrs. Alla Babayeve



Re: feature-request: git "cp" like there is git mv.

2017-12-13 Thread Johannes Schindelin
Hi Simon,

On Tue, 12 Dec 2017, Simon Doodkin wrote:

> please develop a new feature, git "cp" like there is git mv tomovefile1
> tofile2 (to save space).
> 
> there is a solution in https://stackoverflow.com/a/44036771/466363
> however, it is not single easy command.

This is not how this project works. The idea is that it is Open Source, so
that you can develop this feature yourself, and contribute a patch.

Ciao,
Johannes


Re: [WIP 12/15] ls-refs: introduce ls-refs server command

2017-12-13 Thread Philip Oakley

From: "Brandon Williams" 
Sent: Monday, December 04, 2017 11:58 PM

Introduce the ls-refs server command.  In protocol v2, the ls-refs
command is used to request the ref advertisement from the server.  Since
it is a command which can be requested (as opposed to manditory in v1),
a clinet can sent a number of parameters in its request to limit the ref


s/clinet/client/


advertisement based on provided ref-patterns.

Signed-off-by: Brandon Williams 
---

Philip


Re: [PATCH 0/8] Codespeed perf results

2017-12-13 Thread Christian Couder
On Wed, Dec 13, 2017 at 5:02 PM, Philip Oakley  wrote:

>> The end goal would be to have such a server always available to track
>> how the different git commands perform over time on different kind of
>> repos (small, medium, large, ...) with different optimizations on and
>> off (split-index, libpcre2, BLK_SHA1, ...)
>
> Dumb question: is this expected to also be able to do a retrospective on the
> performance of appropriate past releases? That would allow immediate
> performance comparisons, rather than needing to wait for a few releases to
> see the trends.

Yeah, sure. For example in the perf.conf file above there is
"dirsOrRevs = v2.12.0 v2.13.0" which means that tests will be
performed on v2.12.0 and v2.13.0.


Re: [PATCH 0/8] Codespeed perf results

2017-12-13 Thread Philip Oakley

From: "Christian Couder" 

This patch series is built on top of cc/perf-run-config which recently
graduated to master.

It makes it possible to send perf results to a Codespeed server. See
https://github.com/tobami/codespeed/ and web sites like
http://speed.pypy.org/ which are using Codespeed.

The end goal would be to have such a server always available to track
how the different git commands perform over time on different kind of
repos (small, medium, large, ...) with different optimizations on and
off (split-index, libpcre2, BLK_SHA1, ...)


Dumb question: is this expected to also be able to do a retrospective on the 
performance of appropriate past releases? That would allow immediate 
performance comparisons, rather than needing to wait for a few releases to 
see the trends.


Philip



With this series and a config file like:

$ cat perf.conf
[perf]
   dirsOrRevs = v2.12.0 v2.13.0
   repeatCount = 10
sendToCodespeed = http://localhost:8000
repoName = Git repo
[perf "with libpcre"]
   makeOpts = "DEVELOPER=1 USE_LIBPCRE=YesPlease"
[perf "without libpcre"]
   makeOpts = "DEVELOPER=1"

One should be able to just launch:

$ ./run --config perf.conf p7810-grep.sh

and then get nice graphs in a Codespeed instance running on
http://localhost:8000.

Caveat
~~

For now one has to create the "Git repo" environment in the Codespeed
admin interface. (We send the perf.repoName config variable in the
"environment" Codespeed field.) This is because Codespeed requires the
environment fields to be created and does not provide a simple way to
create these fields programmatically.

I might try to work around this problem in the future.

Links
~

This patch series is available here:

https://github.com/chriscool/git/commits/codespeed

The cc/perf-run-config patch series was discussed here:

v1: 
https://public-inbox.org/git/20170713065050.19215-1-chrisc...@tuxfamily.org/
v2: 
https://public-inbox.org/git/cap8ufd2j-ufh+9awz91gtz-jusq7euoexmguro59vpf29jx...@mail.gmail.com/


Christian Couder (8):
 perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}
 perf/aggregate: refactor printing results
 perf/aggregate: implement codespeed JSON output
 perf/run: use $default_value instead of $4
 perf/run: add conf_opts argument to get_var_from_env_or_config()
 perf/run: learn about perf.codespeedOutput
 perf/run: learn to send output to codespeed server
 perf/run: read GIT_TEST_REPO_NAME from perf.repoName

t/perf/aggregate.perl | 164 
+++---

t/perf/run|  29 +++--
2 files changed, 140 insertions(+), 53 deletions(-)

--
2.15.1.361.g8b07d831d0





Re: What's cooking in git.git (Dec 2017, #02; Thu, 7)

2017-12-13 Thread Jeff Hostetler



On 12/12/2017 4:30 AM, Christian Couder wrote:

On Thu, Dec 7, 2017 at 7:04 PM, Junio C Hamano  wrote:


* jh/object-filtering (2017-12-05) 9 commits
   (merged to 'next' on 2017-12-05 at 3a56b51085)
  + rev-list: support --no-filter argument
  + list-objects-filter-options: support --no-filter
  + list-objects-filter-options: fix 'keword' typo in comment
   (merged to 'next' on 2017-11-27 at e5008c3b28)
  + pack-objects: add list-objects filtering
  + rev-list: add list-objects filtering support
  + list-objects: filter objects in traverse_commit_list
  + oidset: add iterator methods to oidset
  + oidmap: add oidmap iterator methods
  + dir: allow exclusions from blob in addition to file
  (this branch is used by jh/fsck-promisors and jh/partial-clone.)

  In preparation for implementing narrow/partial clone, the object
  walking machinery has been taught a way to tell it to "filter" some
  objects from enumeration.


* jh/fsck-promisors (2017-12-05) 12 commits
  - gc: do not repack promisor packfiles
  - rev-list: support termination at promisor objects
  - fixup: sha1_file: add TODO
  - fixup: sha1_file: convert gotos to break/continue
  - sha1_file: support lazily fetching missing objects
  - introduce fetch-object: fetch one promisor object
  - index-pack: refactor writing of .keep files
  - fsck: support promisor objects as CLI argument
  - fsck: support referenced promisor objects
  - fsck: support refs pointing to promisor objects
  - fsck: introduce partialclone extension
  - extension.partialclone: introduce partial clone extension
  (this branch is used by jh/partial-clone; uses jh/object-filtering.)

  In preparation for implementing narrow/partial clone, the machinery
  for checking object connectivity used by gc and fsck has been
  taught that a missing object is OK when it is referenced by a
  packfile specially marked as coming from trusted repository that
  promises to make them available on-demand and lazily.


I am currently working on integrating this series with my external odb
series 
(https://public-inbox.org/git/20170916080731.13925-1-chrisc...@tuxfamily.org/).

Instead of using an "extension.partialclone" config variable, an odb
will be configured like using an "odb..promisorRemote" (the
name might still change) config variable. Other odbs could still be
configured using "odb..scriptCommand" and
"odb..subprocessCommand".

The current work is still very much WIP and some tests fail, but you
can take a look there:

https://github.com/chriscool/git/tree/gl-promisor-external-odb440



In our current V6 patch series, Jonathan Tan and I are using the
extension.partialclone config variable for 2 purposes.  First, to
indicate a change in the repository format and stop non-aware clients
(older versions of git.exe) from operating on the repo -- since they
won't know how to handle missing objects.   Second, to name the remote
to help the client later demand load missing objects.  This is a current
limitation (we only support one promisor remote), so this second usage
may change.  I haven't had time to look at your branch yet, so I can't
comment on how it might help/solve our second usage, but we do need to
keep the first usage in mind.

Jeff


Re: [PATCH v4 00/34] Add directory rename detection to git

2017-12-13 Thread Elijah Newren
On Tue, Dec 12, 2017 at 6:01 PM, Junio C Hamano  wrote:
> OK, it seems that I managed to make this test pass under poison build
> (see https://travis-ci.org/git/git/jobs/315658242)
>
> Please check 
> https://github.com/git/git/commit/e5c5e24ad91a75b5a70c056fe6c6e3bfb55b56fc
> and sprinkle its fix to whichever original commits in the series that
> need fixing.

Will do; sorry for the problems.  Thanks for finding and fixing.


[PATCH 6/8] perf/run: learn about perf.codespeedOutput

2017-12-13 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 t/perf/run | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/perf/run b/t/perf/run
index 04ea5090f9..4454a2713d 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -144,10 +144,15 @@ run_subsection () {
set -- . "$@"
fi
 
+   codespeed_opt=
+   test "$GIT_PERF_CODESPEED_OUTPUT" = "true" && 
codespeed_opt="--codespeed"
+
run_dirs "$@"
-   ./aggregate.perl "$@"
+   ./aggregate.perl $codespeed_opt "$@"
 }
 
+get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf" 
"codespeedOutput" "" --bool
+
 cd "$(dirname $0)"
 . ../../GIT-BUILD-OPTIONS
 
-- 
2.15.1.361.g8b07d831d0



[PATCH 5/8] perf/run: add conf_opts argument to get_var_from_env_or_config()

2017-12-13 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 t/perf/run | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/perf/run b/t/perf/run
index bbd703dc4f..04ea5090f9 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -106,6 +106,7 @@ get_var_from_env_or_config () {
conf_sec="$2"
conf_var="$3"
default_value="$4" # optional
+   conf_opts="$5" # optional
 
# Do nothing if the env variable is already set
eval "test -z \"\${$env_var+x}\"" || return
@@ -116,11 +117,11 @@ get_var_from_env_or_config () {
if test -n "$GIT_PERF_SUBSECTION"
then
var="$conf_sec.$GIT_PERF_SUBSECTION.$conf_var"
-   conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") &&
+   conf_value=$(git config $conf_opts -f "$GIT_PERF_CONFIG_FILE" 
"$var") &&
eval "$env_var=\"$conf_value\"" && return
fi
var="$conf_sec.$conf_var"
-   conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") &&
+   conf_value=$(git config $conf_opts -f "$GIT_PERF_CONFIG_FILE" "$var") &&
eval "$env_var=\"$conf_value\"" && return
 
test -n "${default_value+x}" && eval "$env_var=\"$default_value\""
-- 
2.15.1.361.g8b07d831d0



[PATCH 2/8] perf/aggregate: refactor printing results

2017-12-13 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 t/perf/aggregate.perl | 96 +++
 1 file changed, 50 insertions(+), 46 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 769d418708..3609cb5dc3 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -100,13 +100,6 @@ sub read_descr {
return $line;
 }
 
-my %descrs;
-my $descrlen = 4; # "Test"
-for my $t (@subtests) {
-   $descrs{$t} = $shorttests{$t}.": ".read_descr("$resultsdir/$t.descr");
-   $descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;
-}
-
 sub have_duplicate {
my %seen;
for (@_) {
@@ -122,54 +115,65 @@ sub have_slash {
return 0;
 }
 
-my %newdirabbrevs = %dirabbrevs;
-while (!have_duplicate(values %newdirabbrevs)) {
-   %dirabbrevs = %newdirabbrevs;
-   last if !have_slash(values %dirabbrevs);
-   %newdirabbrevs = %dirabbrevs;
-   for (values %newdirabbrevs) {
-   s{^[^/]*/}{};
+sub print_default_results {
+   my %descrs;
+   my $descrlen = 4; # "Test"
+   for my $t (@subtests) {
+   $descrs{$t} = $shorttests{$t}.": 
".read_descr("$resultsdir/$t.descr");
+   $descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;
}
-}
 
-my %times;
-my @colwidth = ((0)x@dirs);
-for my $i (0..$#dirs) {
-   my $d = $dirs[$i];
-   my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : 
$dirnames{$d});
-   $colwidth[$i] = $w if $w > $colwidth[$i];
-}
-for my $t (@subtests) {
-   my $firstr;
+   my %newdirabbrevs = %dirabbrevs;
+   while (!have_duplicate(values %newdirabbrevs)) {
+   %dirabbrevs = %newdirabbrevs;
+   last if !have_slash(values %dirabbrevs);
+   %newdirabbrevs = %dirabbrevs;
+   for (values %newdirabbrevs) {
+   s{^[^/]*/}{};
+   }
+   }
+
+   my %times;
+   my @colwidth = ((0)x@dirs);
for my $i (0..$#dirs) {
my $d = $dirs[$i];
-   $times{$prefixes{$d}.$t} = 
[get_times("$resultsdir/$prefixes{$d}$t.times")];
-   my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
-   my $w = length format_times($r,$u,$s,$firstr);
+   my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : 
$dirnames{$d});
$colwidth[$i] = $w if $w > $colwidth[$i];
-   $firstr = $r unless defined $firstr;
}
-}
-my $totalwidth = 3*@dirs+$descrlen;
-$totalwidth += $_ for (@colwidth);
-
-binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
+   for my $t (@subtests) {
+   my $firstr;
+   for my $i (0..$#dirs) {
+   my $d = $dirs[$i];
+   $times{$prefixes{$d}.$t} = 
[get_times("$resultsdir/$prefixes{$d}$t.times")];
+   my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+   my $w = length format_times($r,$u,$s,$firstr);
+   $colwidth[$i] = $w if $w > $colwidth[$i];
+   $firstr = $r unless defined $firstr;
+   }
+   }
+   my $totalwidth = 3*@dirs+$descrlen;
+   $totalwidth += $_ for (@colwidth);
 
-printf "%-${descrlen}s", "Test";
-for my $i (0..$#dirs) {
-   my $d = $dirs[$i];
-   printf "   %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} 
: $dirnames{$d});
-}
-print "\n";
-print "-"x$totalwidth, "\n";
-for my $t (@subtests) {
-   printf "%-${descrlen}s", $descrs{$t};
-   my $firstr;
+   printf "%-${descrlen}s", "Test";
for my $i (0..$#dirs) {
my $d = $dirs[$i];
-   my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
-   printf "   %-$colwidth[$i]s", format_times($r,$u,$s,$firstr);
-   $firstr = $r unless defined $firstr;
+   printf "   %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? 
$dirabbrevs{$d} : $dirnames{$d});
}
print "\n";
+   print "-"x$totalwidth, "\n";
+   for my $t (@subtests) {
+   printf "%-${descrlen}s", $descrs{$t};
+   my $firstr;
+   for my $i (0..$#dirs) {
+   my $d = $dirs[$i];
+   my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+   printf "   %-$colwidth[$i]s", 
format_times($r,$u,$s,$firstr);
+   $firstr = $r unless defined $firstr;
+   }
+   print "\n";
+   }
 }
+
+binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
+
+print_default_results();
-- 
2.15.1.361.g8b07d831d0



[PATCH 1/8] perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}

2017-12-13 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 t/perf/aggregate.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index e401208488..769d418708 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -70,7 +70,7 @@ if (not @tests) {
 }
 
 my $resultsdir = "test-results";
-if ($ENV{GIT_PERF_SUBSECTION} ne "") {
+if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
$resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
 }
 
-- 
2.15.1.361.g8b07d831d0



[PATCH 3/8] perf/aggregate: implement codespeed JSON output

2017-12-13 Thread Christian Couder
Codespeed (https://github.com/tobami/codespeed/) is an open source
project that can be used to track how some software performs over
time. It stores performance test results in a database and can show
nice graphs and charts on a web interface.

As it can be interesting to Codespeed to see how Git performance
evolves over time and releases, let's implement a Codespeed output
in "perf/aggregate.perl".

Signed-off-by: Christian Couder 
---
 t/perf/aggregate.perl | 68 +--
 1 file changed, 66 insertions(+), 2 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 3609cb5dc3..6e15f62a9e 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -35,10 +35,15 @@ sub format_times {
return $out;
 }
 
-my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests);
+my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, $codespeed);
 while (scalar @ARGV) {
my $arg = $ARGV[0];
my $dir;
+   if ($arg eq "--codespeed") {
+   $codespeed = 1;
+   shift @ARGV;
+   next;
+   }
last if -f $arg or $arg eq "--";
if (! -d $arg) {
my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
@@ -70,8 +75,10 @@ if (not @tests) {
 }
 
 my $resultsdir = "test-results";
+my $results_section = "";
 if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
$resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
+   $results_section = $ENV{GIT_PERF_SUBSECTION};
 }
 
 my @subtests;
@@ -174,6 +181,63 @@ sub print_default_results {
}
 }
 
+sub print_codespeed_results {
+   my ($results_section) = @_;
+
+   my $project = "Git";
+
+   my $executable;
+   if ($results_section eq "") {
+   $executable = `uname -o -p`;
+   } else {
+   $executable = $results_section;
+   chomp $executable;
+   }
+
+   my $environment;
+   if (exists $ENV{GIT_TEST_REPO_NAME} and $ENV{GIT_TEST_REPO_NAME} ne "") 
{
+   $environment = $ENV{GIT_TEST_REPO_NAME};
+   } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} 
ne "") {
+   $environment = $ENV{GIT_TEST_INSTALLED};
+   $environment =~ s|/bin-wrappers$||;
+   } else {
+   $environment = `uname -r`;
+   chomp $environment;
+   }
+
+   my @data;
+
+   for my $t (@subtests) {
+   for my $d (@dirs) {
+   my $commitid = $prefixes{$d};
+   $commitid =~ s/^build_//;
+   $commitid =~ s/\.$//;
+   my ($result_value, $u, $s) = 
get_times("$resultsdir/$prefixes{$d}$t.times");
+
+   my %vals = (
+   "commitid" => $commitid,
+   "project" => $project,
+   "branch" => $dirnames{$d},
+   "executable" => $executable,
+   "benchmark" => $shorttests{$t} . " " . 
read_descr("$resultsdir/$t.descr"),
+   "environment" => $environment,
+   "result_value" => $result_value,
+   );
+   push @data, \%vals;
+   }
+   }
+
+   #use Data::Dumper qw/ Dumper /;
+   #print Dumper(\@data);
+
+   use JSON;
+   print to_json(\@data, {utf8 => 1, pretty => 1}), "\n";
+}
+
 binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
 
-print_default_results();
+if ($codespeed) {
+   print_codespeed_results($results_section);
+} else {
+   print_default_results();
+}
-- 
2.15.1.361.g8b07d831d0



[PATCH 7/8] perf/run: learn to send output to codespeed server

2017-12-13 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 t/perf/run | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/t/perf/run b/t/perf/run
index 4454a2713d..7b7011f19b 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -148,10 +148,20 @@ run_subsection () {
test "$GIT_PERF_CODESPEED_OUTPUT" = "true" && 
codespeed_opt="--codespeed"
 
run_dirs "$@"
-   ./aggregate.perl $codespeed_opt "$@"
+
+   if test -z "$GIT_PERF_SEND_TO_CODESPEED"
+   then
+   ./aggregate.perl $codespeed_opt "$@"
+   else
+   json_res_file="test-results/$GIT_PERF_SUBSECTION/aggregate.json"
+   ./aggregate.perl --codespeed "$@" | tee "$json_res_file"
+   send_data_url="$GIT_PERF_SEND_TO_CODESPEED/result/add/json/"
+   curl -v --request POST --data-urlencode "json=$(cat 
"$json_res_file")" "$send_data_url"
+   fi
 }
 
 get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf" 
"codespeedOutput" "" --bool
+get_var_from_env_or_config "GIT_PERF_SEND_TO_CODESPEED" "perf" 
"sendToCodespeed"
 
 cd "$(dirname $0)"
 . ../../GIT-BUILD-OPTIONS
-- 
2.15.1.361.g8b07d831d0



[PATCH 8/8] perf/run: read GIT_TEST_REPO_NAME from perf.repoName

2017-12-13 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 t/perf/run | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/perf/run b/t/perf/run
index 7b7011f19b..279c2d41f6 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -137,6 +137,9 @@ run_subsection () {
get_var_from_env_or_config "GIT_PERF_MAKE_COMMAND" "perf" "makeCommand"
get_var_from_env_or_config "GIT_PERF_MAKE_OPTS" "perf" "makeOpts"
 
+   get_var_from_env_or_config "GIT_TEST_REPO_NAME" "perf" "repoName"
+   export GIT_PERF_REPO_NAME
+
GIT_PERF_AGGREGATING_LATER=t
export GIT_PERF_AGGREGATING_LATER
 
-- 
2.15.1.361.g8b07d831d0



[PATCH 4/8] perf/run: use $default_value instead of $4

2017-12-13 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 t/perf/run | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/perf/run b/t/perf/run
index 43e4de49ef..bbd703dc4f 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -105,7 +105,7 @@ get_var_from_env_or_config () {
env_var="$1"
conf_sec="$2"
conf_var="$3"
-   # $4 can be set to a default value
+   default_value="$4" # optional
 
# Do nothing if the env variable is already set
eval "test -z \"\${$env_var+x}\"" || return
@@ -123,7 +123,7 @@ get_var_from_env_or_config () {
conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") &&
eval "$env_var=\"$conf_value\"" && return
 
-   test -n "${4+x}" && eval "$env_var=\"$4\""
+   test -n "${default_value+x}" && eval "$env_var=\"$default_value\""
 }
 
 run_subsection () {
-- 
2.15.1.361.g8b07d831d0



[PATCH 0/8] Codespeed perf results

2017-12-13 Thread Christian Couder
This patch series is built on top of cc/perf-run-config which recently
graduated to master.

It makes it possible to send perf results to a Codespeed server. See
https://github.com/tobami/codespeed/ and web sites like
http://speed.pypy.org/ which are using Codespeed.

The end goal would be to have such a server always available to track
how the different git commands perform over time on different kind of
repos (small, medium, large, ...) with different optimizations on and
off (split-index, libpcre2, BLK_SHA1, ...)

With this series and a config file like:

$ cat perf.conf
[perf]
dirsOrRevs = v2.12.0 v2.13.0
repeatCount = 10
sendToCodespeed = http://localhost:8000
repoName = Git repo
[perf "with libpcre"]
makeOpts = "DEVELOPER=1 USE_LIBPCRE=YesPlease"
[perf "without libpcre"]
makeOpts = "DEVELOPER=1"

One should be able to just launch:

$ ./run --config perf.conf p7810-grep.sh

and then get nice graphs in a Codespeed instance running on
http://localhost:8000.

Caveat
~~

For now one has to create the "Git repo" environment in the Codespeed
admin interface. (We send the perf.repoName config variable in the
"environment" Codespeed field.) This is because Codespeed requires the
environment fields to be created and does not provide a simple way to
create these fields programmatically.

I might try to work around this problem in the future.

Links
~

This patch series is available here:

https://github.com/chriscool/git/commits/codespeed

The cc/perf-run-config patch series was discussed here:

v1: https://public-inbox.org/git/20170713065050.19215-1-chrisc...@tuxfamily.org/
v2: 
https://public-inbox.org/git/cap8ufd2j-ufh+9awz91gtz-jusq7euoexmguro59vpf29jx...@mail.gmail.com/

Christian Couder (8):
  perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}
  perf/aggregate: refactor printing results
  perf/aggregate: implement codespeed JSON output
  perf/run: use $default_value instead of $4
  perf/run: add conf_opts argument to get_var_from_env_or_config()
  perf/run: learn about perf.codespeedOutput
  perf/run: learn to send output to codespeed server
  perf/run: read GIT_TEST_REPO_NAME from perf.repoName

 t/perf/aggregate.perl | 164 +++---
 t/perf/run|  29 +++--
 2 files changed, 140 insertions(+), 53 deletions(-)

-- 
2.15.1.361.g8b07d831d0



Re: [PATCH] doc: clarify usage of XDG_CONFIG_HOME config file

2017-12-13 Thread Yaroslav Halchenko

On Tue, 12 Dec 2017, Jacob Keller wrote:

> > And then "and the other files will not be read" can be dropped from
> > the first sentence of this paragraph?

> > Yaroslav on the original thread mentioned that reading codepath
> > without --file or --global does not limit to one of the three, and
> > this section is about "If not set explicitly with `--file`", so we'd
> > need to make sure if the above is what happens in reality (or update
> > the proposed clarification to match the reality).

> I'm pretty sure it does not read XDG_CONFIG_HOME unless ~/.gitconfig
> is missing. I tried a few things, but it was 2am for me, so I may be
> mis-remembering.

It always read it for non--global

$> ( HOME=/tmp/HOME; rm -rf $HOME; mkdir -p $HOME/.config/git; echo -e 
"[user]\n name=home" > $HOME/.gitconfig; echo -e "[user]\n name=xdg\n 
name2=xdg2" > $HOME/.config/git/config; git config user.name; git config 
user.name2; ) 
home
xdg2

and it doesn't read it for --global

$> ( HOME=/tmp/HOME; rm -rf $HOME; mkdir -p $HOME/.config/git; echo -e 
"[user]\n name=home" > $HOME/.gitconfig; echo -e "[user]\n name=xdg\n 
name2=xdg2" > $HOME/.config/git/config; git config --global user.name; git 
config --global user.name2; )   
 
home

unless ~/.gitconfig is missing

$> ( HOME=/tmp/HOME; rm -rf $HOME; mkdir -p $HOME/.config/git; echo -e 
"[user]\n name=xdg\n name2=xdg2" > $HOME/.config/git/config; git config 
--global user.name; git config --global user.name2; ) 
xdg
xdg2


-- 
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


Re: [PATCH] partial-clone: design doc

2017-12-13 Thread Philip Oakley

From: "Junio C Hamano" 

"Philip Oakley"  writes:


+  These filtered packfiles are incomplete in the traditional sense
because
+  they may contain trees that reference blobs that the client does
not have.


Is a comment needed here noting that currently, IIUC, the complete
trees are fetched in the packfiles, it's just the un-necessary blobs
that are omitted ?


I probably am misreading what you meant to say, but the above
statement with "currently" taken literally to mean the system
without JeffH's changes, is false.


I was meaning the current JeffH's V6 series, rather than the last Git 
release.


In one of the previous discussions Jeff had noted that (at that time) his 
partial design would provide a full set of trees for the selected commits 
(excluding the trees already available locally), but only a few of the file 
blobs (based on the filter spec).


So yes, I should have been clearer to avoid talking at cross purposes.



When the receiver says it has commit A and the sender wants to send
a commit B (because the receiver said it does not have it, and it
wants it), trees in A are not sent in the pack the sender sends to
give objects sufficient to complete B, which the receiver wanted to
have, even if B also has those trees.  If you fetch from me twice
and between that time Documentation/ directory did not change, the
second fetch will not have the tree object that corresponds to that
hierarchy (and of course no blobs and sub trees inside it).


Though, after the fetch has completed (v2.15 Git), the receiver will have 
the 'full set of trees and blobs'. In Jeff's design (V6) the reciever would 
still have a full set of trees, but only a partial set of the blobs. So my 
viewpoint was not of the pack file but of the receiver's object store after 
the fetch.




So "the complete trees are fetched" is not true.  What is true (and
what matters more in JeffH's document) is that fetching is done in
such a way that objects resulting in the receiving repository are
complete in the current system that does not allow promised objects.
If some objects resulting in the receiving repository are incomplete,
the current system considers that we corrupted the repository.

The promise mechanism says that it is fine for the receiving end to
lack blobs, trees or commits, as long as the promisor repository
tells it that these "missing" objects can be obtained from it later.


True. (though I'm not sure exactly how Jeff decides about commits - I 
thought theye were not part of this optimisation)



The way the receiving end which notices that it does not have an
otherwise required blob, tree or commit is one promised by the
promisor repository is to see if it is referenced by a pack that
came from such a promisor repository.


.. and marked as such with the ".promisor" extension.



Thanks. 



Re: [PATCH] doc: Modify git-add doc to say "staging area"

2017-12-13 Thread Ævar Arnfjörð Bjarmason
On Wed, Dec 13, 2017 at 6:46 AM, David A. Wheeler  wrote:
> On December 13, 2017 12:40:12 AM EST, Jacob Keller  
> wrote:
>>I know we've used various terms for this concept across a lot of the
>>documentation. However, I was under the impression that we most
>>explicitly used "index" rather than "staging area".
>
> I think "staging area" is the better term. It focuses on its purpose, and it 
> is also less confusing ("index" and "cache" have other meanings in many of 
> the repos managed by git).

After your patch the majority of the docs will still talk about
"index", is this part of some larger series, perhaps it would be good
to see it all at once...


[PATCH] sequencer: improve config handling

2017-12-13 Thread Phillip Wood
From: Phillip Wood 

The previous config handling relied on global variables, called
git_default_config() even when the key had already been handled by
git_sequencer_config() and did not initialize the diff configuration
variables. Improve this by: i) loading the default values for message
cleanup and gpg signing of commits into struct replay_opts;
ii) restructuring the code to return immediately once a key is
handled; and iii) calling git_diff_basic_config(). Note that
unfortunately it is not possible to return early if the key is handled
by git_gpg_config() as it does not indicate to the caller if the key
has been handled or not.

The sequencer should probably have been calling
git_diff_basic_config() before as it creates a patch when there are
conflicts. The shell version uses 'diff-tree' to create the patch so
calling git_diff_basic_config() should match that. Although 'git
commit' calls git_diff_ui_config() I don't think the output of
print_commit_summary() is affected by anything that is loaded by that
as print_commit_summary() always turns on rename detection so would
ignore the value in the user's configuration anyway. The other values
loaded by git_diff_ui_config() are about the formatting of patches so
are not relevant to print_commit_summary().

Signed-off-by: Phillip Wood 
---
 builtin/rebase--helper.c | 13 +---
 builtin/revert.c | 15 ++---
 sequencer.c  | 87 ++--
 sequencer.h  | 19 ++-
 4 files changed, 61 insertions(+), 73 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 
68194d3aed950f327a8bc624fa1991478dfea01e..decb8f7a09e42eb94bed264164985e54e13a32f6
 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -9,17 +9,6 @@ static const char * const builtin_rebase_helper_usage[] = {
NULL
 };
 
-static int git_rebase_helper_config(const char *k, const char *v, void *cb)
-{
-   int status;
-
-   status = git_sequencer_config(k, v, NULL);
-   if (status)
-   return status;
-
-   return git_default_config(k, v, NULL);
-}
-
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
@@ -50,7 +39,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_END()
};
 
-   git_config(git_rebase_helper_config, NULL);
+   sequencer_init_config();
 
opts.action = REPLAY_INTERACTIVE_REBASE;
opts.allow_ff = 1;
diff --git a/builtin/revert.c b/builtin/revert.c
index 
1938825efa6ad20ede5aba57f097863aeb33d1d5..76f0a35b074b858ab4cb3e3894bc7c877401b7e8
 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -31,17 +31,6 @@ static const char * const cherry_pick_usage[] = {
NULL
 };
 
-static int common_config(const char *k, const char *v, void *cb)
-{
-   int status;
-
-   status = git_sequencer_config(k, v, NULL);
-   if (status)
-   return status;
-
-   return git_default_config(k, v, NULL);
-}
-
 static const char *action_name(const struct replay_opts *opts)
 {
return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
@@ -219,7 +208,7 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
-   git_config(common_config, NULL);
+   sequencer_init_config();
res = run_sequencer(argc, argv, );
if (res < 0)
die(_("revert failed"));
@@ -232,7 +221,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char 
*prefix)
int res;
 
opts.action = REPLAY_PICK;
-   git_config(common_config, NULL);
+   sequencer_init_config();
res = run_sequencer(argc, argv, );
if (res < 0)
die(_("cherry-pick failed"));
diff --git a/sequencer.c b/sequencer.c
index 
0f17b4d32580aa637ddfeedfaec68468a9995e3d..c3035bb5f3d8bb1c6a6be8f3bc8d1bb29cf4383c
 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -132,6 +132,51 @@ static GIT_PATH_FUNC(rebase_path_strategy, 
"rebase-merge/strategy")
 static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
 static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, 
"rebase-merge/allow_rerere_autoupdate")
 
+static int git_sequencer_config(const char *k, const char *v, void *cb)
+{
+   struct replay_opts *opts = cb;
+   int status;
+
+   if (!strcmp(k, "commit.cleanup")) {
+   const char *s;
+
+   status = git_config_string(, k, v);
+   if (status)
+   return status;
+
+   if (!strcmp(s, "verbatim"))
+   opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
+   else if (!strcmp(s, "whitespace"))
+   opts->default_msg_cleanup =