Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator

[Matthieu Moy]

Johannes Schindelin  writes:

> This is the correct thing to do, really: we already specify LF as
> field separator.

I'm almost convinced that this is the right thing to do in the long run
("almost" because I'm not sure, not because I have arguments against). I
agree with Junio that the commit message should be more convincing, but
indeed, accepting LF and not CR is strange.

However, is this the right thing to do in the maintainance branch? It
does fix the issue, but does so in a rather intrusive way, so I'd need
more arguments to be convinced that this is safe to merge in maint. Or
have a local fix for rebase to be merged in maint, and apply this in
master for the next feature release.

Sorry for being negative, and especially sorry since I'm partly guilty
for the breakage. I just want to be sure that we don't break anything
while repairing it (we already introduced this breakage while repairing
another one...).

>  # Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and
>  # do not equate an unset IFS with IFS with the default, so here is
> -# an explicit SP HT LF.
> +# an explicit SP HT LF CR.
>  IFS='
> -'
> +'"$(printf '\r')"

While we're there, it may be better to have a single "printf ' \t\n\r'"
to avoid the whitespace magic in the source code.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git repack command on larger pack file

[Sivakumar Selvam]

Hi,
   I ran git repack on a single larger repository abc.git where the pack
file size 34 GB. Generally it used to take 20-25 minutes in my server to
complete the repacking. During repacking I noticed, disk usage was more, So
I thought of splitting the pack file into 4 GB chunks. I used the following
command to do repacking.
   git repack -A -b -d -q --depth=50 --window=10 abc.git

   After adding --max-pack-size=4g to the above command again I ran to split
pack files..
   git repack -A -b -d -q --depth=50 --window=10 --max-pack-size=4g abc.git
 
   When I finished running, I found 12 pack files with each 4 GB and the
size is 48 GB. Now my disk usage has increased by 14 GB. Again, I ran to
check the performance, but the size (48 GB) and time to repacking takes
another 35 minutes more. Why this issue? If we split a larger pack file,
repacking takes more time with more disk usage for storing pack files. Any
thoughts on this why this happens?

Thanks,
Sivakumar Selvam.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git repack command on larger pack file

[Junio C Hamano]

Sivakumar Selvam  writes:

>I ran git repack on a single larger repository abc.git where the pack
> file size 34 GB. Generally it used to take 20-25 minutes in my server to
> complete the repacking. During repacking I noticed, disk usage was more, So
> I thought of splitting the pack file into 4 GB chunks. I used the following
> command to do repacking.
>git repack -A -b -d -q --depth=50 --window=10 abc.git
>
>After adding --max-pack-size=4g to the above command again I ran to split
> pack files..
>git repack -A -b -d -q --depth=50 --window=10 --max-pack-size=4g abc.git
>  
>When I finished running, I found 12 pack files with each 4 GB and the
> size is 48 GB. Now my disk usage has increased by 14 GB. Again, I ran to
> check the performance, but the size (48 GB) and time to repacking takes
> another 35 minutes more. Why this issue?

Hmmm, what is "this issue"?  I do not see anything surprising.

If you have N objects and run repack with window=10, you would
(roughly speaking, without taking various optimization we have and
bootstrap conditions into account) check each of these N objects
against 10 other objects to find good delta base, no matter how big
your max pack-size is set.  And that takes the bulk of time in the
repack process.  Also it has to write more data to disk (see below),
it has to find a good place to split, it has to adjust bookkeeping
data at the pack boundary, in general it has to do more, not less,
to produce split packs.  It would be surprising if it took less
time.

Each pack by definition has to be self-sufficient; all delta in the
pack must have its base object in the same pack.  Now, imagine that
an object (call it X) would have been expressed as a delta derived
from another object (call it Y) if you were producing a single pack,
and imagine that the pack has grown to be 4 GB big just before you
write object X out.  The current pack (which contains the base
object Y already) needs to be closed and then a new pack is opened.
Imagine how you would write X now into that new pack.  You have to
discard the deltified representation of X (which by definition is
much smaller, because it is an instruction to reconstitute X given
an object Y whose contents is very similar to X) and write the base
representation of X to the pack, because X can no longer be
expressed as a delta derived from Y.  That is why you would need to
write more.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF

[Johannes Schindelin]

Hi Junio,

On Sun, 25 Oct 2015, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > Chad Boles reported that `git rebase -i` recently started producing
> > errors when the editor saves files with DOS line endings. The symptom
> > is:
> >
> > Warning: the command isn't recognized in the following line:
> >  -
> >
> > You can fix this with 'git rebase --edit-todo'.
> > Or you can abort the rebase with 'git rebase --abort'.
> >
> > The real bummer is that simply calling `git rebase --continue` "fixes"
> > it.
> 
> Two questions.
> 
>  * What does the DOS editor do to a file with ^M in the middle of a
>line, e.g. "A^MB^M^J"?

You mean mixed line endings? At least with this Windows 10 WordPad, *all*
line endings are normalized to CR/LF. That is, if you edit a file that
contains a stray CR, it is presented as a line break.

>  * Is your shell ported correctly to the platform?

I would think so. It is an MSys2 program, therefore it uses all the POSIX
niceties, of course.

A simple test with CR/LF line endings in a script reveals that it is
pretty solid:

x=a
case "$x" in a) echo b;; esac

prints out 'b', as expected.

Please also note that things apparently worked alright before the patch
removing the stripspace call was applied.

> The latter may need a bit of elaboration.  "read a b c" is supposed
> to read one line of text (where the definition of line is platform
> dependent, your platform may use CRLF to signal the end of an line),
> split the characters on the line (i.e. LF vs CRLF no longer matters
> at this point) at $IFS characters and give them to $a $b and $c. If
> the platform accepts CRLF as the EOL signal, should the program still
> see CR at the end of $c?
> 
> A solution that mucks with IFS smells like fixing a wrong problem
> without addressing the real cause.

Please note that it is a bit unsettling if you use funny language like
"smells" here and then accuse me of not having an argument when I point
that the same rationale applies to having CR in IFS as applies to having
LF in IFS. Yes, that was an implicit argument, but it is a strong one, so
I do not think you are well served ignoring it.

> Also IFS is used not only by "read", so munging it globally doubly
> feels wrong.
> 
> In addition, you do not want to split at CR; what you want is to
> treat CRLF (i.e. not a lone CR) as the end-of-line separator.
> Adding CR to IFS feels triply wrong.
> 
> By the way, saying "This is right, really" makes you sound as if you
> do not have a real argument.

Again. If CR has no place in IFS, why does LF have a place in IFS? It
makes *no* sense to argue for one and against the other.

In any case, I am not interested in arguing for arguing's sake. Tell me
what you want instead of the patch to IFS, I will implement it and test
whether it fixes the bug and we will move on.

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/3] Add test to describe expectation of blame --reverse with branched history

[Junio C Hamano]

Max Kirillov  writes:

> If history contains merges from feature branches, `blame --reverse`
> reports not the commit when the line was actually edited, but head of
> the last merged branch which was created before the edit.
>
> As a workaround, `blame --reverse --first-parent` could be used to find
> the merge of branch containing the edit, but it was disabled in
> 95a4fb0eac, because incorrectly specified range could produce in
> unexpected and meaningless result.
>
> Add tests which describe ideal functionality with and without
> `--first-parent`.
>
> Signed-off-by: Max Kirillov 

I _think_ I know why it would be useful to allow "--first-parent" to
the command; it is useful the same way why "git log --first-parent
$path" would be a good way to get an overview.

But I am puzzled by your complaints (I'd characterise the statement
as such, given your second paragraph calls the combination a
"workaround") in the first paragraph.  I honestly do not understand
where it comes from at all.

The reverse blame begins from an old state and shows the most recent
child in the history that each line survived to, and it does not
show what commit removed the line from the original state.  And that
does not have anything to do with the presence of any merges or
forks in the history.  The command will always report "not the
commit that edited the line."  There is nothing special about "If
the history contains merges".

If you have this history, for example:

D---E---F
   / \*
  O   X---Y
   \ /
A---B---C

where O had the original file, which was not touched by any commits
on the branch on the upper side, and commit B rewrote all lines of
the file, running blame in reverse may show A as the last point
where all lines survived up to, if the "reversed" history happened
to consider A as the earlier "parent" (in reality it is a child but
blame is about assigning blame for each line from child to parents
so in the reversed history, real children becomes parents).  Or it
may show F as the last point where all lines survived up to, if D
was picked as the earlier "parent".  Because there is no inherent
ordering between A and D, both of which are children of O, your
result is not necessarily "head of the last merged branch".

But I do not see how "first-parent" would be a workaround for that.
The option would be useful to force the assignment of blame (in
reverse) along the first-parent chain O---D---E---F---X---Y so that
you can get a bird's-eye view of the history, i.e. squashing all
that happened in A---B---C as if that happened at X.

The explanation of the first paragraph needs to be rewritten to make
it understandable, but I am not sure what relevance it has with this
change.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git repack command on larger pack file

[Junio C Hamano]

Junio C Hamano  writes:

> Sivakumar Selvam  writes:
>
>> ... So
>> I thought of splitting the pack file into 4 GB chunks.
> ...
> Hmmm, what is "this issue"?  I do not see anything surprising.

While the explanation might have been enlightening, the knowledge
conveyed by the explanation by itself would not be of much practical
use, and enlightment without practical use is never fun.

So let's do another tangent that may be more useful.

In many repositories, older parts of the history often hold the bulk
of objects that do not change, and it is wasteful to repack them
over and over.  If your project is at around v40.0 today, and it was
at around v36.0 6 months ago, for example, you may want to pack
everything that happened before v36.0 into a single pack just once,
pack them really well, and have your "repack" not touch that old
part of the history.

  $ git rev-list --objects v36.0 |
git pack-objects --window=200 --depth=128 pack

would produce such a pack [*1*]

The standard output from the above pipeline will give you a 40-hex
string (e.g. 51c472761b4690a331c02c90ec364e47cca1b3ac, call it
$HEX), and in the current directory you will find two files,
pack-$HEX.pack and pack-$HEX.idx.

You can then do this:

  $ echo "v36.0 with W/D 200/128" >pack-$HEX.keep
  $ mv pack-$HEX.* .git/objects/pack/.
  $ git repack -a -d

A pack that has an accompanying .keep file is excempt from
repacking, so once you do this, your future "git repack" will only
repack objects that are not in the kept packs.



[Footnote]

*1* I won't say 200/128 gives you a good pack; you would need to
experiment.  In general, larger depth will result in smaller pack
but it will result in bigger overhead while you use the repository
every day.  Larger window will spend a lot of cycles while packing,
but will result in a smaller pack.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC] receive-pack: allow for hiding refs outside the namespace

[Lukas Fleischer]

Right now, we always advertise all refs as ".have", even those outside
the current namespace. This leads to problems when trying to push to a
repository with a huge number of namespaces from a slow connection.

Add a configuration option receive.advertiseAllRefs that can be used to
determine whether refs outside the current namespace should be
advertised or not.

Signed-off-by: Lukas Fleischer 
---
We are using Git namespaces to store a huge number of (virtual)
repositories inside a shared repository. While the blobs in the virtual
repositories are fairly similar, they do not share any refs, so
advertising any refs outside the current namespace is undesirable. See
the discussion on [1] for details.

Note that this patch is just a draft: I didn't do any testing, apart
from checking that it compiles. I would like to hear some opinions
before sending a polished version.

Is our use case considered common enough to justify the inclusion of
such a configuration option in mainline?

Are there suggestions for a better name for the option? Ideally, it
should contain the word "namespace" but I could not come up with
something sensible that is short enough.

[1] https://lists.archlinux.org/pipermail/aur-general/2015-October/031596.html

 Documentation/config.txt |  6 ++
 builtin/receive-pack.c   | 31 +--
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 315f271..aa101a7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2201,6 +2201,12 @@ receive.advertiseAtomic::
capability to its clients. If you don't want to this capability
to be advertised, set this variable to false.
 
+receive.advertiseAllRefs::
+   By default, git-receive-pack will advertise all refs, even those
+   outside the current namespace, so that the client can use them to
+   minimize data transfer. If you only want to advertise refs from the
+   active namespace to be advertised, set this variable to false.
+
 receive.autogc::
By default, git-receive-pack will run "git-gc --auto" after
receiving data from git-push and updating refs.  You can stop
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e6b93d0..ea9a820 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -41,6 +41,7 @@ static struct strbuf fsck_msg_types = STRBUF_INIT;
 static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
 static int advertise_atomic_push = 1;
+static int advertise_all_refs = 1;
 static int unpack_limit = 100;
 static int report_status;
 static int use_sideband;
@@ -190,6 +191,11 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (strcmp(var, "receive.advertiseallrefs") == 0) {
+   advertise_all_refs = git_config_bool(var, value);
+   return 0;
+   }
+
return git_default_config(var, value, cb);
 }
 
@@ -222,16 +228,21 @@ static void show_ref(const char *path, const unsigned 
char *sha1)
 static int show_ref_cb(const char *path, const struct object_id *oid, int 
flag, void *unused)
 {
path = strip_namespace(path);
-   /*
-* Advertise refs outside our current namespace as ".have"
-* refs, so that the client can use them to minimize data
-* transfer but will otherwise ignore them. This happens to
-* cover ".have" that are thrown in by add_one_alternate_ref()
-* to mark histories that are complete in our alternates as
-* well.
-*/
-   if (!path)
-   path = ".have";
+   if (!path) {
+   if (advertise_all_refs) {
+   /*
+* Advertise refs outside our current namespace as
+* ".have" refs, so that the client can use them to
+* minimize data transfer but will otherwise ignore
+* them. This happens to cover ".have" that are thrown
+* in by add_one_alternate_ref() to mark histories that
+* are complete in our alternates as well.
+*/
+   path = ".have";
+   } else {
+   return 0;
+   }
+   }
show_ref(path, oid->hash);
return 0;
 }
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] blame: fix option name in error message

[Junio C Hamano]

Max Kirillov  writes:

> The option name used in blame's UI is `--reverse`.
>
> Signed-off-by: Max Kirillov 
> ---

;-)

It is surprising that nobody noticed this which was in the very
original of --reverse.  Thanks.

>  builtin/blame.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 98b1810..f89bc9e 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2697,7 +2697,7 @@ parse_done:
>   sb.commits.compare = compare_commits_by_commit_date;
>   }
>   else if (contents_from)
> - die("--contents and --children do not blend well.");
> + die("--contents and --reverse do not blend well.");
>   else {
>   final_commit_name = prepare_initial();
>   sb.commits.compare = compare_commits_by_reverse_commit_date;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] run-command: factor out child_process_clear()

[Stefan Beller]

On Mon, Oct 26, 2015 at 11:43 AM, Junio C Hamano  wrote:
> René Scharfe  writes:
>
>> Avoid duplication by moving the code to release allocated memory for
>> arguments and environment to its own function, child_process_clear().
>> Export it to provide a counterpart to child_process_init().
>>
>> Signed-off-by: Rene Scharfe 
>> ---
>
> Hmm, is this _deinit() Stefan added to his series recently?

Yes. Although you (Junio) take credit for actually using it in these
places, too. :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] Correct fscanf formatting string for I64u values

[Junio C Hamano]

Thanks; will queue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] Silence GCC's "cast of pointer to integer of a different size" warning

[Junio C Hamano]

OK.  Will queue.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] Facilitate debugging Git executables in tests with gdb

[Jonathan Nieder]

Johannes Schindelin wrote:

> When prefixing a Git call in the test suite with 'TEST_GDB_GIT=1 ', it
> will now be run with GDB, allowing the developer to debug test failures
> more conveniently.

Neat.

[...]
> --- a/wrap-for-bin.sh
> +++ b/wrap-for-bin.sh
> @@ -19,4 +19,11 @@ GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
>  PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
>  export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
>  
> +if test -n "$TEST_GDB_GIT"
> +then
> + exec gdb -args "${GIT_EXEC_PATH}/@@PROG@@" "$@"

Most TEST_ environment variables that git respects are under
GIT_TEST_* --- e.g., GIT_TEST_OPTS.  Should this match that pattern
as well, for easier debugging with commands like 'env | grep GIT_'?

What happens if the child in turn calls git again?  Should this
unset TEST_GDB_GIT in gdb's environment?

The gdb manual and --help output advertise "--args".  Has "-args"
(with a single dash) always worked?

> + echo "Could not run gdb -args ${GIT_EXEC_PATH}/@@PROG@@ $*" >&2
> + exit 1

Does the 'exec' after the fi need this as well?  exec is supposed to
itself print a message and exit when it runs into an error.  Would
including an 'else' with the if make the control flow clearer?  E.g.

if test -n "$TEST_GDB_GIT"
then
exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
else
exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
fi

Thanks,
Jonathan

diff --git i/wrap-for-bin.sh w/wrap-for-bin.sh
index a151c95..db0ec6a 100644
--- i/wrap-for-bin.sh
+++ w/wrap-for-bin.sh
@@ -19,11 +19,10 @@ GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
 PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
 export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
 
-if test -n "$TEST_GDB_GIT"
+if test -n "$GIT_TEST_GDB"
 then
-   exec gdb -args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
-   echo "Could not run gdb -args ${GIT_EXEC_PATH}/@@PROG@@ $*" >&2
-   exit 1
+   unset GIT_TEST_GDB
+   exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+else
+   exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
 fi
-
-exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Remove unused #include "sigchain.h"

[Michael Haggerty]

On 10/22/2015 02:43 PM, Tobias Klauser wrote:
> This series removes the #include of sigchain.h from several modules
> after they were changed to use the tempfile module and they thus no
> longer use any declarations from sigchain.h
> 
> Tobias Klauser (5):
>   gc: remove unused #include "sigchain.h"
>   credential-cache--daemon: remove unused #include "sigchain.h"
>   diff: remove unused #include "sigchain.h"
>   read-cache: remove unused #include "sigchain.h"
>   shallow: remove unused #include "sigchain.h"

Thanks for cleaning these up, Tobias. I totally forgot to look for
includes that were no longer needed.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] t6031: generalize for recursive and resolve strategies

[Jeff King]

This script tests the filemode handling of merge-recursive,
but we do not test the same thing for merge-resolve. Let's
generalize the script a little:

  1. Break out the setup steps for each test into a separate
 snippet.

  2. For each test, run it twice; once with "-s recursive"
 and once with "-s resolve". We can avoid repeating
 ourselves by adding a function.

  3. Since we have a nice abstracted function, we can make
 our tests more thorough by testing both directions
 (change on "ours" versus "theirs").

This improves our test coverage, and will make this the
place to add more tests related to merging mode changes.

Signed-off-by: Jeff King 
---
Sadly I changed too much for this to count as a rename (mostly due to
the new indentation). Oh well, the rename diff was not especially
readable, and it is probably just as well to read the new content as a
single blob.

 t/t6031-merge-filemode.sh  | 77 ++
 t/t6031-merge-recursive.sh | 56 -
 2 files changed, 77 insertions(+), 56 deletions(-)
 create mode 100755 t/t6031-merge-filemode.sh
 delete mode 100755 t/t6031-merge-recursive.sh

diff --git a/t/t6031-merge-filemode.sh b/t/t6031-merge-filemode.sh
new file mode 100755
index 000..c6896e6
--- /dev/null
+++ b/t/t6031-merge-filemode.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+
+test_description='merge: handle file mode'
+. ./test-lib.sh
+
+test_expect_success 'set up mode change in one branch' '
+   : >file1 &&
+   git add file1 &&
+   git commit -m initial &&
+   git checkout -b a1 master &&
+   : >dummy &&
+   git add dummy &&
+   git commit -m a &&
+   git checkout -b b1 master &&
+   test_chmod +x file1 &&
+   git add file1 &&
+   git commit -m b1
+'
+
+do_one_mode () {
+   strategy=$1
+   us=$2
+   them=$3
+   test_expect_success "resolve single mode change ($strategy, $us)" '
+   git checkout -f $us &&
+   git merge -s $strategy $them &&
+   git ls-files -s file1 | grep ^100755
+   '
+
+   test_expect_success FILEMODE "verify executable bit on file ($strategy, 
$us)" '
+   test -x file1
+   '
+}
+
+do_one_mode recursive a1 b1
+do_one_mode recursive b1 a1
+do_one_mode resolve a1 b1
+do_one_mode resolve b1 a1
+
+test_expect_success 'set up mode change in both branches' '
+   git reset --hard HEAD &&
+   git checkout -b a2 master &&
+   : >file2 &&
+   H=$(git hash-object file2) &&
+   test_chmod +x file2 &&
+   git commit -m a2 &&
+   git checkout -b b2 master &&
+   : >file2 &&
+   git add file2 &&
+   git commit -m b2 &&
+   {
+   echo "100755 $H 2   file2"
+   echo "100644 $H 3   file2"
+   } >expect
+'
+
+do_both_modes () {
+   strategy=$1
+   test_expect_success "detect conflict on double mode change ($strategy)" 
'
+   git reset --hard &&
+   git checkout -f a2 &&
+   test_must_fail git merge -s $strategy b2 &&
+   git ls-files -u >actual &&
+   test_cmp actual expect &&
+   git ls-files -s file2 | grep ^100755
+   '
+
+   test_expect_success FILEMODE "verify executable bit on file 
($strategy)" '
+   test -x file2
+   '
+}
+
+# both sides are equivalent, so no need to run both ways
+do_both_modes recursive
+do_both_modes resolve
+
+test_done
diff --git a/t/t6031-merge-recursive.sh b/t/t6031-merge-recursive.sh
deleted file mode 100755
index 4053bd9..000
--- a/t/t6031-merge-recursive.sh
+++ /dev/null
@@ -1,56 +0,0 @@
-#!/bin/sh
-
-test_description='merge-recursive: handle file mode'
-. ./test-lib.sh
-
-test_expect_success 'mode change in one branch: keep changed version' '
-   : >file1 &&
-   git add file1 &&
-   git commit -m initial &&
-   git checkout -b a1 master &&
-   : >dummy &&
-   git add dummy &&
-   git commit -m a &&
-   git checkout -b b1 master &&
-   test_chmod +x file1 &&
-   git add file1 &&
-   git commit -m b1 &&
-   git checkout a1 &&
-   git merge-recursive master -- a1 b1 &&
-   git ls-files -s file1 | grep ^100755
-'
-
-test_expect_success FILEMODE 'verify executable bit on file' '
-   test -x file1
-'
-
-test_expect_success 'mode change in both branches: expect conflict' '
-   git reset --hard HEAD &&
-   git checkout -b a2 master &&
-   : >file2 &&
-   H=$(git hash-object file2) &&
-   test_chmod +x file2 &&
-   git commit -m a2 &&
-   git checkout -b b2 master &&
-   : >file2 &&
-   git add file2 &&
-   git commit -m b2 &&
-   git checkout a2 &&
-   (
-   git merge-recursive master -- a2 b2
-   test $? = 1
-   ) &&
-   git ls-files -u >actual &&
-   (
-   echo "100755 $H 2   file2"
-   echo "100644 $H 

Re: [PATCH 0/3] detecting delete/modechange conflicts

[Jeff King]

On Mon, Oct 26, 2015 at 02:46:42PM -0700, Junio C Hamano wrote:

> > After looking through the history and the list archive, I don't _think_
> > this was intentional, and we simply missed the case in both places. But
> > maybe somebody else knows something I don't. It seems like we should be
> > punting to the user under the general principle of stupid and safe
> > merges.
> 
> Yes, I do not recall ever discussing and agreeing with Linus that we
> should resolve to deletion over mode change, and I agree that it
> would be very likely that this never came up in practice simply
> because in real life removal is already rare, mode change is rarer,
> and these happening to the same path in the same timeperiod to
> matter in merges is even more rare.
> 
> We should definitely signal a conflict.

Thanks, that matches my thinking exactly.

BTW, this came up because libgit2 does signal the conflict, and we are
regression-testing a switch from merge-resolve over to libgit2 to power
GitHub's "merge" button. Run it on enough test cases and you will find
one of everything. :)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] user-manual: fix the description of fast-forward

[Junio C Hamano]

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF

[Junio C Hamano]

Johannes Schindelin  writes:

> A simple test with CR/LF line endings in a script reveals that it is
> pretty solid:
>
>   x=a
>   case "$x" in a) echo b;; esac
>
> prints out 'b', as expected.

I do not see what this has to do with anything.

The shell language parser when parsing a script may do the right
thing, but the bug I was alluding to was that your 'read' does not
seem to be removing the terminating  (which is CRLF on your
platform) after reading a line before splitting the contents on the
line at IFS boundaries.

> Again. If CR has no place in IFS, why does LF have a place in IFS? It
> makes *no* sense to argue for one and against the other.

See my message to Matthieu.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator

[Junio C Hamano]

Junio C Hamano  writes:

> Something along the line of the following would be tolerable, even
> though I think in the longer term, not just in Git land but in the
> larger ecosystem to use POSIXy tools on Windows, the best solution
> is to fix the shell so that it matches the expectation of the users
> of its platform.
>
> I say "something along the line of" here because I do not know how
> the problematic shell behaves on the assignment command that stuffs
> a lone CR into a variable.  It _might_ need a similar protection
> against the shell feature "the last EOL is removed from the result
> of command expansion", as I did in the above example, depending on
> how incoherent the implementation is.  The implementation seems to
> accept CRLF and LF in shell scripts proper just fine, but apparently
> its implementation of "read" does not honor such platform EOL
> convention, which caused this issue, and I don't know what it does
> in the codepath that implements command expansion.

I still have to say "something along the lines of" as I do not have
(and I do not wish to have, to be honest) an access to test this
with the problematic shell implementation, but here is an updated
patch.

-- >8 --
Subject: [PATCH] rebase-i: work around Windows CRLF line endings

Editors on Windows can and do save text files with CRLF line
endings, which is the convention on the platform.  We are seeing
reports that the "read" command in a port of bash to the environment
however does not strip the CRLF at the end, not adjusting for the
same convention on the platform.

This breaks the recently added sanity checks for the insn sheet fed
to "rebase -i"; instead of an empty line (hence nothing in $command),
the script was getting a lone CR in there.

Special case a lone CR and treat it the same way as an empty line to
work this around.

The test was stolen from Dscho.

Signed-off-by: Junio C Hamano 
---
 git-rebase--interactive.sh| 13 +
 t/t3404-rebase-interactive.sh | 12 
 2 files changed, 25 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c42ba34..3911711 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -77,6 +77,10 @@ amend="$state_dir"/amend
 rewritten_list="$state_dir"/rewritten-list
 rewritten_pending="$state_dir"/rewritten-pending
 
+# Work around a Windows port of shell that does not strip
+# the newline at the end of a line correctly.
+cr=$(printf "\015")
+
 strategy_args=
 if test -n "$do_merge"
 then
@@ -518,6 +522,11 @@ do_next () {
"$comment_char"*|''|noop|drop|d)
mark_action_done
;;
+   "$cr")
+   # Windows port of shell not stripping the newline
+   # at the end of an empty line correctly.
+   mark_action_done
+   ;;
pick|p)
comment_for_reflog pick
 
@@ -888,6 +897,10 @@ check_bad_cmd_and_sha () {
"$comment_char"*|''|noop|x|exec)
# Doesn't expect a SHA-1
;;
+   "$cr")
+   # Windows port of shell not stripping the newline
+   # at the end of an empty line correctly.
+   ;;
pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
if ! check_commit_sha "${rest%%[]*}" "$lineno" 
"$1"
then
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 88d7d53..2f97a3d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1240,4 +1240,16 @@ test_expect_success 'static check of bad SHA-1' '
test E = $(git cat-file commit HEAD | sed -ne \$p)
 '
 
+test_expect_success 'editor saves as CR/LF' '
+   git checkout -b with-crlf &&
+   write_script add-crs.sh <<-\EOF &&
+   sed -e "s/\$/Q/" <"$1" | tr Q "\\015" >"$1".new &&
+   mv -f "$1".new "$1"
+   EOF
+   (
+   test_set_editor "$(pwd)/add-crs.sh" &&
+   git rebase -i HEAD^
+   )
+'
+
 test_done
-- 
2.6.2-405-g6877da6


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] http: allow selection of proxy authentication method

[Junio C Hamano]

Knut Franke  writes:

> CURLAUTH_ANY does not work with proxies which answer unauthenticated requests
> with a 307 redirect to an error page instead of a 407 listing supported
> authentication methods. Therefore, allow the authentication method to be set
> using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or configuration
> variables http.proxy-authmethod and remote..proxy-authmethod (in analogy
> to http.proxy and remote..proxy).

Please do not name configuration variable with dashes (or
underscores).  There still may be existing aberrations that are
deprecated and need to be removed over time that are misnamed, but
the core git variables are named without dashes and underscores.

Call yours "http.proxyAuthmethod" in the documentation, and use
strcmp("http.proxyauthmethod", var) in the options callback code.

> +static void init_curl_proxy_auth(CURL *result)
> +{
> +set_from_env(_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");

Strange indentation here...

> +
> + if (http_proxy_authmethod) {
> + if (!strcmp(http_proxy_authmethod, "basic"))
> + curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
> CURLAUTH_BASIC);
> + else if (!strcmp(http_proxy_authmethod, "digest"))
> + curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
> CURLAUTH_DIGEST);
> + else if (!strcmp(http_proxy_authmethod, "negotiate"))
> + curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
> CURLAUTH_GSSNEGOTIATE);
> + else if (!strcmp(http_proxy_authmethod, "ntlm"))
> + curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
> CURLAUTH_NTLM);
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> + else if (!strcmp(http_proxy_authmethod, "anyauth"))
> + curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
> CURLAUTH_ANY);
> +#endif
> + // CURLAUTH_DIGEST_IE has no corresponding command-line option 
> in
> + // curl(1) and is not included in CURLAUTH_ANY, so we leave it 
> out
> + // here, too
> + else
> + error("Unknown proxy authentication method: %s",
> +   http_proxy_authmethod);

Along the same line as how we do sslversions[] instead of a long
if/else if/ chain is preferrable.

Thansk.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] detecting delete/modechange conflicts

[Jeff King]

I was surprised to find that:

  # base commit
  echo base >file &&
  git add file &&
  git commit -m base &&

  # one side changes mode
  chmod +x file &&
  git commit -am executable &&

  # the other deletes the file
  git checkout -b other HEAD^ &&
  git rm file &&
  git commit -am removed &&

  git merge master

silently completes the merge, dropping the mode change. We detect
delete/modify conflicts, but not delete/modechange conflicts.  The
trivial index merge does catch it, but both the resolve and recursive
strategies resolve it silently in favor of the deletion.

After looking through the history and the list archive, I don't _think_
this was intentional, and we simply missed the case in both places. But
maybe somebody else knows something I don't. It seems like we should be
punting to the user under the general principle of stupid and safe
merges.

  [1/3]: t6031: move triple-rename test to t3030
  [2/3]: t6031: generalize for recursive and resolve strategies
  [3/3]: merge: detect delete/modechange conflict

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?

[Jeff King]

On Sun, Oct 25, 2015 at 09:58:56AM -0700, Junio C Hamano wrote:

> >>> I cannot speak for the person who was primarily responsible for
> >>> designing this behaviour, but I happen to agree with the current
> >>> behaviour in the situation where it was designed to be used.  Upon
> >>> the first use in your session, the "daemon" is auto-spawned, you can
> >>> keep talking with that same instance during your session, and you do
> >>> not have to do anything special to shut it down when you log out.
> >>> Isn't that what happens here?
> >>
> >> After looking at this some more, I've discovered this is NOT what
> >> actually happens here. If I "git push" from a shell and then log out
> >> and log in again, another "git push" does NOT ask me for a password.
> >> In other words, the daemon is NOT shut down automatically when I log
> >> out. Given that, does it make sense to change the daemon to ignore
> >> SIGHUP, or is there some way to change it so that it does exit on
> >> logout?
> 
> I have a feeling that it would be moving in a wrong direction to
> change the code to ignore HUP, as I do think "logout to shutdown"
> would be the desired behaviour.  If you are not seeing that happen,
> perhaps the first thing to do is to figure out why and fix the code
> so that it happens?
> 
> I dunno.  I'll cc Peff so that he can take a look when he comes
> back.

I could see it going both ways.

If SIGHUP means "I am logging out, my session is over", then I agree it
makes sense to drop any credentials. And that is what SIGHUP meant when
people logged in through hard-wired terminals.

But these days, people often have several simultaneous sessions open.
They may have multiple ssh sessions to a single machine, or they may
have a bunch of terminal windows open, each of which has a login shell
and will send HUP to its children when it exits. In that case, you have
a meta-session surrounding those individual terminal sessions, and you
probably do want to keep the cache going as long as the meta session[1].

This is all further complicated by bash's huponexit option, which I
think is off by default. So I, for example, have never noticed this
behavior even with multiple xterms, because my cache never actually gets
SIGHUP.  I don't know what shell Noam is using, but I wonder if tweaking
that option (or a similar one if not bash) might be helpful to signal
"let this stuff keep running even after I exit".

But I am also not opposed to making it configurable somehow in git, if
there really are two cases that cannot otherwise be distinguished.

-Peff

[1] Of course we have no idea when that meta-session is closed. But if
you have a script that runs on X logout, for instance, you could put
"git credential-cache exit" in it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] remote-http(s): Support SOCKS proxies

[Junio C Hamano]

Johannes Schindelin  writes:

> This patch was required to work behind a faulty AP and scraped from
> http://stackoverflow.com/questions/15227130/#15228479 and guarded with
> an appropriate cURL version check by Johannes Schindelin.
>
> Signed-off-by: Johannes Schindelin 

Thanks.

The code looks OK but the last paragraph makes _us_ worried.  What
is the licensing status of the original at SO?  I can see that you
are taking legal responsibility with the Signed-off-by: line; you
state that to the best of your knowledge the patch is covered under
an appropriate open source license and you ahve the right under that
license to submit it here to the project.

But it is my job to double check when in doubt, hence this question.

> ---
>  http.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/http.c b/http.c
> index 7da76ed..6b89dea 100644
> --- a/http.c
> +++ b/http.c
> @@ -465,6 +465,17 @@ static CURL *get_curl_handle(void)
>  
>   if (curl_http_proxy) {
>   curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +#if LIBCURL_VERSION_NUM >= 0x071800
> + if (starts_with(curl_http_proxy, "socks5"))
> + curl_easy_setopt(result,
> + CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
> + else if (starts_with(curl_http_proxy, "socks4a"))
> + curl_easy_setopt(result,
> + CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4A);
> + else if (starts_with(curl_http_proxy, "socks"))
> + curl_easy_setopt(result,
> + CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
> +#endif
>   }
>  #if LIBCURL_VERSION_NUM >= 0x070a07
>   curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] merge: detect delete/modechange conflict

[Jeff King]

If one side deletes a file and the other changes its
content, we notice and report a conflict. However, if
instead of changing the content, we change only the mode,
the merge does not notice (and the mode change is silently
dropped).

The trivial index merge notices the problem and correctly
leaves the conflict in the index, but both merge-recursive
and merge-one-file will silently resolve this in favor of
the deletion.  In many cases that is a sane resolution, but
we should be punting to the user whenever there is any
question. So let's detect and treat this as a conflict (in
both strategies).

Signed-off-by: Jeff King 
---
The text for merge-one-file looks really gross and unlike
our usual error messages, but it matches the similar add/add
case lower in the script. I don't know if its worth cleaning
those messages up, as I doubt many people use "-s resolve"
these days. But even if we do, that should be a separate
series.

 git-merge-one-file.sh |  8 
 merge-recursive.c |  8 ++--
 t/t6031-merge-filemode.sh | 23 +++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 07dfeb8..cdc02af 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -38,6 +38,14 @@ case "${1:-.}${2:-.}${3:-.}" in
 # Deleted in both or deleted in one and unchanged in the other
 #
 "$1.." | "$1.$1" | "$1$1.")
+   if { test -z "$6" && test "$5" != "$7"; } ||
+  { test -z "$7" && test "$5" != "$6"; }
+   then
+   echo "ERROR: File $4 deleted on one branch but had its" >&2
+   echo "ERROR: permissions changed on the other." >&2
+   exit 1
+   fi
+
if test -n "$2"
then
echo "Removing $4"
diff --git a/merge-recursive.c b/merge-recursive.c
index a5e74d8..21e680a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1530,13 +1530,17 @@ static int read_sha1_strbuf(const unsigned char *sha1, 
struct strbuf *dst)
 }
 
 static int blob_unchanged(const unsigned char *o_sha,
+ unsigned o_mode,
  const unsigned char *a_sha,
+ unsigned a_mode,
  int renormalize, const char *path)
 {
struct strbuf o = STRBUF_INIT;
struct strbuf a = STRBUF_INIT;
int ret = 0; /* assume changed for safety */
 
+   if (a_mode != o_mode)
+   return 0;
if (sha_eq(o_sha, a_sha))
return 1;
if (!renormalize)
@@ -1722,8 +1726,8 @@ static int process_entry(struct merge_options *o,
} else if (o_sha && (!a_sha || !b_sha)) {
/* Case A: Deleted in one */
if ((!a_sha && !b_sha) ||
-   (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) ||
-   (!a_sha && blob_unchanged(o_sha, b_sha, normalize, path))) {
+   (!b_sha && blob_unchanged(o_sha, o_mode, a_sha, a_mode, 
normalize, path)) ||
+   (!a_sha && blob_unchanged(o_sha, o_mode, b_sha, b_mode, 
normalize, path))) {
/* Deleted in both or deleted in one and
 * unchanged in the other */
if (a_sha)
diff --git a/t/t6031-merge-filemode.sh b/t/t6031-merge-filemode.sh
index c6896e6..7d06461 100755
--- a/t/t6031-merge-filemode.sh
+++ b/t/t6031-merge-filemode.sh
@@ -74,4 +74,27 @@ do_both_modes () {
 do_both_modes recursive
 do_both_modes resolve
 
+test_expect_success 'set up delete/modechange scenario' '
+   git reset --hard &&
+   git checkout -b deletion master &&
+   git rm file1 &&
+   git commit -m deletion
+'
+
+do_delete_modechange () {
+   strategy=$1
+   us=$2
+   them=$3
+   test_expect_success "detect delete/modechange conflict ($strategy, 
$us)" '
+   git reset --hard &&
+   git checkout $us &&
+   test_must_fail git merge -s $strategy $them
+   '
+}
+
+do_delete_modechange recursive b1 deletion
+do_delete_modechange recursive deletion b1
+do_delete_modechange resolve b1 deletion
+do_delete_modechange resolve deletion b1
+
 test_done
-- 
2.6.2.481.g6ca35c3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] detecting delete/modechange conflicts

[Junio C Hamano]

Jeff King  writes:

> After looking through the history and the list archive, I don't _think_
> this was intentional, and we simply missed the case in both places. But
> maybe somebody else knows something I don't. It seems like we should be
> punting to the user under the general principle of stupid and safe
> merges.

Yes, I do not recall ever discussing and agreeing with Linus that we
should resolve to deletion over mode change, and I agree that it
would be very likely that this never came up in practice simply
because in real life removal is already rare, mode change is rarer,
and these happening to the same path in the same timeperiod to
matter in merges is even more rare.

We should definitely signal a conflict.

>   [1/3]: t6031: move triple-rename test to t3030
>   [2/3]: t6031: generalize for recursive and resolve strategies
>   [3/3]: merge: detect delete/modechange conflict
>
> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Add git-grep threads param

[John Keeping]

On Mon, Oct 26, 2015 at 03:32:13PM +0300, Victor Leschuk wrote:
> Make number of git-grep worker threads a configuration parameter.
> According to several tests on systems with different number of CPU cores
> the hard-coded number of 8 threads is not optimal for all systems:
> tuning this parameter can significantly speed up grep performance.
> 
> Signed-off-by: Victor Leschuk 
> ---
>  Documentation/config.txt   |  4 
>  Documentation/git-grep.txt |  4 
>  builtin/grep.c | 34 
> ++
>  contrib/completion/git-completion.bash |  1 +
>  grep.c | 10 ++
>  grep.h |  2 ++
>  6 files changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 391a0c3..1c95587 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1447,6 +1447,10 @@ grep.extendedRegexp::
>   option is ignored when the 'grep.patternType' option is set to a value
>   other than 'default'.
>  
> +grep.threads::
> + Number of grep worker threads, use it to tune up performance on
> + multicore machines. Default value is 8.
> +
>  gpg.program::
>   Use this custom program instead of "gpg" found on $PATH when
>   making or verifying a PGP signature. The program must support the
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index 4a44d6d..fbd4f83 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -22,6 +22,7 @@ SYNOPSIS
>  [--color[=] | --no-color]
>  [--break] [--heading] [-p | --show-function]
>  [-A ] [-B ] [-C ]
> +[--threads ]

Is this the best place for this option?  I know the current list isn't
sorted in any particular way, but here you're splitting up the set of
context options (`-A`, `-B`, `-C` and `-W`).

>  [-W | --function-context]
>  [-f ] [-e] 
>  [--and|--or|--not|(|)|-e ...]
> @@ -220,6 +221,9 @@ OPTIONS
>   Show  leading lines, and place a line containing
>   `--` between contiguous groups of matches.
>  
> +--threads ::
> + Set number of worker threads to . Default is 8.

The same comment as above applies here.

>  -W::
>  --function-context::
>   Show the surrounding text from the previous line containing a
> diff --git a/builtin/grep.c b/builtin/grep.c
> index d04f440..5ef1b07 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -27,8 +27,7 @@ static char const * const grep_usage[] = {
>  static int use_threads = 1;
>  
>  #ifndef NO_PTHREADS
> -#define THREADS 8
> -static pthread_t threads[THREADS];
> +static pthread_t *threads;
>  
>  /* We use one producer thread and THREADS consumer
>   * threads. The producer adds struct work_items to 'todo' and the
> @@ -206,7 +205,8 @@ static void start_threads(struct grep_opt *opt)
>   strbuf_init([i].out, 0);
>   }
>  
> - for (i = 0; i < ARRAY_SIZE(threads); i++) {
> + threads = xcalloc(opt->num_threads, sizeof(pthread_t));
> + for (i = 0; i < opt->num_threads; i++) {
>   int err;
>   struct grep_opt *o = grep_opt_dup(opt);
>   o->output = strbuf_out;
> @@ -220,7 +220,7 @@ static void start_threads(struct grep_opt *opt)
>   }
>  }
>  
> -static int wait_all(void)
> +static int wait_all(struct grep_opt *opt)

I'm not sure passing a grep_opt in here is the cleanest way to do this.
Options are a UI concept and all we care about here is the number of
threads.

Since `threads` is a global, shouldn't the number of threads be a global
as well?  Could we reuse `use_threads` here (possibly renaming it
`num_threads`)?

>  {
>   int hit = 0;
>   int i;
> @@ -238,12 +238,14 @@ static int wait_all(void)
>   pthread_cond_broadcast(_add);
>   grep_unlock();
>  
> - for (i = 0; i < ARRAY_SIZE(threads); i++) {
> + for (i = 0; i < opt->num_threads; i++) {
>   void *h;
>   pthread_join(threads[i], );
>   hit |= (int) (intptr_t) h;
>   }
>  
> + free(threads);
> +
>   pthread_mutex_destroy(_mutex);
>   pthread_mutex_destroy(_read_mutex);
>   pthread_mutex_destroy(_attr_mutex);
> @@ -256,7 +258,7 @@ static int wait_all(void)
>  }
>  #else /* !NO_PTHREADS */
>  
> -static int wait_all(void)
> +static int wait_all(struct grep_opt *opt)
>  {
>   return 0;
>  }
> @@ -702,6 +704,8 @@ int cmd_grep(int argc, const char **argv, const char 
> *prefix)
>   N_("show  context lines before matches")),
>   OPT_INTEGER('A', "after-context", _context,
>   N_("show  context lines after matches")),
> + OPT_INTEGER(0, "threads", _threads,
> + N_("use  worker threads")),
>   OPT_NUMBER_CALLBACK(, N_("shortcut for -C NUM"),
>   context_callback),

Re: Why are submodules not automatically handled by default or at least configurable to do so?

[Junio C Hamano]

Stefan Beller  writes:

> IIUC at the time submodules were invented, there was need for lots of
> code to be written.
> Each command needed new code to deal with submodules. As there was not
> enough people/time
> to do it properly, the "do nothing" was the safest action which could
> be added fast.

That is quite different from how I remember.  Soon after Linus and I
added the Gitlink in early Apr 20007, an early subproject/gitlink
(thought) experiment was started with help from folks like Steven
Grimm, Jan Hudec, Petr Baudis, Alex Riesen etc.  The first principle
of the design throughout that era was "we admit that we do not know
all the use cases, so let's start small and solid and make sure that
small-and-solid thing can later be enhanced as people discover the
way how they want to work" (you can see me expressing that sentiment
in $gmane/48287, for example).

So it wasn't "not enough people to do it properly" at all.  It was
"we admit that we do not know what is proper, so we defer to actual
users to define what is proper for them".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] Only use CURLOPT_LOGIN_OPTIONS if it is actually available

[Junio C Hamano]

Thanks.  Makes sense.  It probably needs to have imap-send early on
the title, though (will locally amend, so no need to resend).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] use pop_commit() for consuming the first entry of a struct commit_list

[Junio C Hamano]

René Scharfe  writes:

> Instead of open-coding the function pop_commit() just call it.  This
> makes the intent clearer and reduces code size.
>
> Signed-off-by: Rene Scharfe 
> ---
>  builtin/fmt-merge-msg.c |  9 +++--
>  builtin/merge.c | 12 +---
>  builtin/reflog.c|  6 +-
>  builtin/rev-parse.c |  7 ++-
>  builtin/show-branch.c   | 17 +++--
>  commit.c| 27 +++
>  remote.c|  6 ++
>  revision.c  | 27 +--
>  shallow.c   |  6 +-
>  upload-pack.c   |  6 ++
>  10 files changed, 31 insertions(+), 92 deletions(-)

While I appreciate this kind of simplification very much, I also
hate to review this kind of simplification at the same time, as it
is very easy to make and miss simple mistakes.

Let me try to go through it very carefully.

> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index 4ba7f28..846004b 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -537,7 +537,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
>  static void find_merge_parents(struct merge_parents *result,
>  struct strbuf *in, unsigned char *head)
>  {
> - struct commit_list *parents, *next;
> + struct commit_list *parents;
>   struct commit *head_commit;
>   int pos = 0, i, j;
>  
> @@ -576,13 +576,10 @@ static void find_merge_parents(struct merge_parents 
> *result,
>   parents = reduce_heads(parents);
>  
>   while (parents) {
> + struct commit *cmit = pop_commit();
>   for (i = 0; i < result->nr; i++)
> - if (!hashcmp(result->item[i].commit,
> -  parents->item->object.sha1))
> + if (!hashcmp(result->item[i].commit, cmit->object.sha1))
>   result->item[i].used = 1;
> - next = parents->next;
> - free(parents);
> - parents = next;

OK, I would have called it "commit" not "cmit", but this is
trivially equivalent to the original.

> diff --git a/builtin/merge.c b/builtin/merge.c
> index a0a9328..31241b8 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1019,7 +1019,7 @@ static struct commit_list *reduce_parents(struct commit 
> *head_commit,
> int *head_subsumed,
> struct commit_list *remoteheads)
>  {
> - struct commit_list *parents, *next, **remotes = 
> + struct commit_list *parents, **remotes;

Interesting.  I viewed the result of applying this patch with "git
show -U32" to make sure that this initialization of remotes was
unnecessary.

> @@ -1033,16 +1033,14 @@ static struct commit_list *reduce_parents(struct 
> commit *head_commit,
>   /* Find what parents to record by checking independent ones. */
>   parents = reduce_heads(remoteheads);
>  
> - for (remoteheads = NULL, remotes = 
> -  parents;
> -  parents = next) {
> - struct commit *commit = parents->item;
> - next = parents->next;
> + remoteheads = NULL;
> + remotes = 
> + while (parents) {
> + struct commit *commit = pop_commit();
>   if (commit == head_commit)
>   *head_subsumed = 0;
>   else
>   remotes = _list_insert(commit, remotes)->next;
> - free(parents);
>   }
>   return remoteheads;
>  }

Trivially equivalent.  Good.  I'll stop saying this if there is
nothing noteworthy from now on.

> diff --git a/builtin/show-branch.c b/builtin/show-branch.c
> index 092b59b..ac5141d 100644
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -53,17 +53,6 @@ static struct commit *interesting(struct commit_list *list)
>   return NULL;
>  }
>  
> -static struct commit *pop_one_commit(struct commit_list **list_p)
> -{
> - struct commit *commit;
> - struct commit_list *list;
> - list = *list_p;
> - commit = list->item;
> - *list_p = list->next;
> - free(list);
> - return commit;
> -}
> -

Interesting.  We had our own implementation that is less lenient
than the global one.  And this one is newer by two months!  Good
riddance.

> @@ -2733,10 +2726,7 @@ static void simplify_merges(struct rev_info *revs)
>   yet_to_do = NULL;
>   tail = _to_do;
>   while (list) {
> - commit = list->item;
> - next = list->next;
> - free(list);
> - list = next;
> + commit = pop_commit();
>   tail = simplify_one(revs, commit, tail);
>   }
>   }

Any conversion in this set that does not eliminate the intermediate
variable "next" (or named similarly but differently in some
contexts) is 

Re: [PATCH 2/2] daemon: plug memory leak

[Jeff King]

On Sat, Oct 24, 2015 at 02:23:20PM +0200, René Scharfe wrote:

> Call child_process_clear() when a child ends to release the memory
> allocated for its environment.  This is necessary because unlike all
> other users of start_command() we don't call finish_command(), which
> would have taken care of that for us.
> 
> This leak was introduced by f063d38b (daemon: use cld->env_array
> when re-spawning).

I agree this is a leak we need to address, but I find the solution a
little unsatisfactory (but tl;dr, it's probably the best we can do).

It would be nice if we could call finish_command() here to do the
wait(). But it does not know about WNOHANG, so we would have to
introduce a new helper anyway.

However, the whole check_dead_children function is accidentally
quadratic, and should probably be refactored in general. It loops over
the list of children, calling `waitpid(pid, WNOHANG)` on each. So if `n`
children connect at one time, we make `O(n^2)` waitpid calls.

I have patches to instead loop over waitpid(-1, WNOHANG) until there are
no dead children left (and use a hash to go from pid to each "struct
child").

But the reason I haven't posted them is that I'm somewhat of the opinion
that this child-list can go away altogether. The main use is in
enforcing the max_connections variable. Just enforcing that can be done
with a counter, but we do something much weirder: when we get a new
connection, we try to kill an _existing_ child by finding the first
client with multiple connections, killing that, sleeping for a second
(!), and then if that killed something, giving the slot to the new
connection.

This has a few problems:

  1. Under non-malicious heavy load, you probably want to reject the new
 request rather than killing an existing one. If you kill a fetch
 that is 99% completed, that client is likely to come back and fetch
 again, and you've just thrown away all of the effort (both CPU and
 bandwidth) put into the failed clone. You haven't spent anything on
 the incoming connection, so you'd much rather they go away and come
 back in a moment.

  2. I think the kill_some_child algorithm was meant to enforce
 fairness so that a single IP cannot monopolize all of your slots.
 I'm not sure that's realistic for a few reasons:

   a. Real bad guys will just hit you with a DDoS from many IPs.

   b. Real sites have load-balancing in front of git-daemon, and the
  client IPs may not be meaningful.

   c. The duplicate selection is pretty naive. A bad guy with tons
  of duplicate connections is no more likely to be killed than a
  normal client with exactly two connections (it actually
  depends solely on when the latest connection from either came
  in). So I suspect a determine attacker with a single IP could
  still present a problem.

  3. Calling sleep() in the server parent process is a great way to kill
 performance. :)

Of course my view is somewhat skewed by running a really big site with
load balancers and such (and we have long set --max-connections high
enough that it has never been hit). Maybe it's more realistic protection
for a "normal" site.

But my inclination would be to drop kill_some_child entirely in favor of
simply rejecting new connections that are over the limit, and getting
rid of the child-list entirely (and calling waitpid only to reap the
zombie PIDs).

I guess we'd still need something like child_process_clear(), though. We
could just run it immediately after spawning the child, rather than when
we cleaned it up.  So in that sense this series is the first incremental
step toward that future, anyway.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] Squelch warning about an integer overflow

[Junio C Hamano]

Johannes Schindelin  writes:

> We cannot rely on long integers to have more than 32 bits...
>
> Signed-off-by: Johannes Schindelin 
> ---

Interesting.  8192 * 1024 * 1024 does not fit within 32-bit long, of
course.  Perhaps we can lose L after 1024 if we are explicitly
saying that the result ought to be size_t (which may be larger than
long)?

>  git-compat-util.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 805d0e2..610e8a5 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -568,7 +568,7 @@ extern int git_lstat(const char *, struct stat *);
>  #endif
>  
>  #define DEFAULT_PACKED_GIT_LIMIT \
> - ((1024L * 1024L) * (sizeof(void*) >= 8 ? 8192 : 256))
> + ((1024L * 1024L) * (size_t)(sizeof(void*) >= 8 ? 8192 : 256))
>  
>  #ifdef NO_PREAD
>  #define pread git_pread
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] git-p4: Add option to ignore empty commits

[Luke Diamand]

On 24/10/15 19:08, Lars Schneider wrote:


On 21 Oct 2015, at 08:32, Luke Diamand  wrote:


On 19/10/15 19:43, larsxschnei...@gmail.com wrote:

From: Lars Schneider 



This seems to be adding a new function in the middle of an existing function. 
Is that right?

That is true. I could move these functions into the P4Sync class if you don't 
like them here. I added them right there because it is the only place where 
they are used/useful.


It just seemed a bit confusing, but I'm ok with them here as well.







+if not self.clientSpecDirs:
+return True
+inClientSpec = self.clientSpecDirs.map_in_client(path)
+if not inClientSpec and self.verbose:
+print '\n  Ignoring file outside of client spec' % path
+return inClientSpec


Any particular reason for putting a \n at the start of the message?

I did this because "sys.stdout.write("\rImporting revision ..." (line 2724) does not add a newline. However, I agree that this 
looks stupid. I will remove the "\n" and fix the "Import revision" print. Speaking of that one: this is only printed if 
"not self.silent". Is there a particular reason to have "self.silent" and "self.verbose"? Should we merge the two?


self.silent and self.verbose seem like two slightly different things. I 
would expect silent to prevent any output at all. But actually it doesn't.


If we wanted to implement it properly, I think we'd need a new function 
(p4print?) which did the obvious right thing. Maybe something for a 
rainy day in the future.







Also, could you use python3 style print stmnts, print("whatever") ?

Sure. How do you prefer the formatting? Using "format" would be true Python 3 
style I think:
print('Ignoring file outside of client spec: {}'.format(path))


Will that breaker older versions of python? There's a statement 
somewhere about how far back we support.



OK?




+
+def hasBranchPrefix(path):
+if not self.branchPrefixes:
+return True
+hasPrefix = [p for p in self.branchPrefixes
+if p4PathStartsWith(path, p)]
+if hasPrefix and self.verbose:
+print '\n  Ignoring file outside of prefix: %s' % path
+return hasPrefix
+
+files = [f for f in files
+if inClientSpec(f['path']) and hasBranchPrefix(f['path'])]
+
+if not files and gitConfigBool('git-p4.ignoreEmptyCommits'):
+print '\n  Ignoring change %s as it would produce an empty commit.'
+return


As with Junio's comment elsewhere, I worry about deletion here.

I believe this is right. See my answer to Junio.



+
  self.gitStream.write("commit %s\n" % branch)
  #gitStream.write("mark :%s\n" % details["change"])
  self.committedChanges.add(int(details["change"]))
@@ -2403,7 +2412,7 @@ class P4Sync(Command, P4UserMap):
  print "parent %s" % parent
  self.gitStream.write("from %s\n" % parent)

-self.streamP4Files(new_files)
+self.streamP4Files(files)
  self.gitStream.write("\n")

  change = int(details["change"])
diff --git a/t/t9826-git-p4-ignore-empty-commits.sh 
b/t/t9826-git-p4-ignore-empty-commits.sh
new file mode 100755
index 000..5ddccde
--- /dev/null
+++ b/t/t9826-git-p4-ignore-empty-commits.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+
+test_description='Clone repositories and ignore empty commits'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'Create a repo' '
+   client_view "//depot/... //client/..." &&
+   (
+   cd "$cli" &&
+
+   mkdir -p subdir &&
+
+   >subdir/file1.txt &&
+   p4 add subdir/file1.txt &&
+   p4 submit -d "Add file 1" &&
+
+   >file2.txt &&
+   p4 add file2.txt &&
+   p4 submit -d "Add file 2" &&
+
+   >subdir/file3.txt &&
+   p4 add subdir/file3.txt &&
+   p4 submit -d "Add file 3"
+   )
+'
+
+test_expect_success 'Clone repo root path with all history' '
+   client_view "//depot/... //client/..." &&
+   test_when_finished cleanup_git &&
+   (
+   cd "$git" &&
+   git init . &&
+   git p4 clone --use-client-spec --destination="$git" //depot@all 
&&
+   cat >expect <<-\EOF &&
+Add file 3
+[git-p4: depot-paths = "//depot/": change = 3]
+
+Add file 2
+[git-p4: depot-paths = "//depot/": change = 2]
+
+Add file 1
+[git-p4: depot-paths = "//depot/": change = 1]


Could you not just test for existence of these files?

Well, I assume the right files are in there as this is covered with other 
tests. I want to check that these particular commits are mentioned in the logs 
(including the commit message and the change list number).



If the format of the magic comments that git-p4 

What's cooking in git.git (Oct 2015, #06; Mon, 26)

[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'.

At tinyurl.com/gitCal, I drew a 14-week schedule for this cycle.  I
plan to be offline during weeks #7-#9 myself; hopefully we'll have
capable interim maintainers to take care of the list traffic in the
meantime as in past years.

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

* dk/p4-import-ctypes (2015-10-20) 1 commit
  (merged to 'next' on 2015-10-22 at 5760144)
 + git-p4: import the ctypes module

 "git-p4" tried to use from ctypes module without first importing
 it.


* dt/t7063-fix-flaky-test (2015-10-19) 1 commit
  (merged to 'next' on 2015-10-20 at 156af72)
 + t7063: fix flaky untracked-cache test


* es/worktree-add (2015-10-18) 1 commit
  (merged to 'next' on 2015-10-20 at ccadb70)
 + worktree: usage: denote  as optional with 'add'


* jc/am-3-fallback-regression-fix (2015-10-09) 1 commit
  (merged to 'next' on 2015-10-15 at 7dde994)
 + am -3: do not let failed merge from completing the error codepath
 (this branch is used by js/am-3-merge-recursive-direct.)

 "git am -3" had a small regression where it is aborted in its error
 handling codepath when underlying merge-recursive failed in certain
 ways, as it assumed that the internal call to merge-recursive will
 never die, which is not the case (yet).


* jc/usage-stdin (2015-10-16) 1 commit
  (merged to 'next' on 2015-10-20 at 937d4aa)
 + usage: do not insist that standard input must come from a file

 The synopsis text and the usage string of subcommands that read
 list of things from the standard input are often shown as if they
 only take input from a file on a filesystem, which was misleading.


* jk/repository-extension (2015-06-24) 2 commits
  (merged to 'next' on 2015-10-22 at 116c8ce)
 + introduce "preciousObjects" repository extension
 + introduce "extensions" form of core.repositoryformatversion

 Prepare for Git on-disk repository representation to undergo
 backward incompatible changes by introducing a new repository
 format version "1", with an extension mechanism.


* kn/for-each-tag (2015-10-18) 1 commit
  (merged to 'next' on 2015-10-20 at 7afd374)
 + tag.c: use the correct algorithm for the '--contains' option

 Recent update to "git tag --contains" caused a performance
 regression.


* mr/worktree-list (2015-10-08) 5 commits
  (merged to 'next' on 2015-10-20 at 7cb272d)
 + worktree: add 'list' command
 + worktree: add details to the worktree struct
 + worktree: add a function to get worktree details
 + worktree: refactor find_linked_symref function
 + worktree: add top-level worktree.c

 Add the "list" subcommand to "git worktree".


* rt/placeholder-in-usage (2015-10-16) 1 commit
  (merged to 'next' on 2015-10-20 at 5189b23)
 + am, credential-cache: add angle brackets to usage string

 A couple of commands still showed "[options]" in their usage string
 to note where options should come on their command line, but we
 spell that "[]" in most places these days.


* tk/stripspace (2015-10-16) 2 commits
  (merged to 'next' on 2015-10-20 at 327a997)
 + stripspace: use parse-options for command-line parsing
 + strbuf: make stripspace() part of strbuf

 The internal stripspace() function has been moved to where it
 logically belongs to, i.e. strbuf API, and the command line parser
 of "git stripspace" has been updated to use the parse_options API.

--
[New Topics]

* gr/rebase-i-drop-warn (2015-10-26) 1 commit
 - rebase-i: work around Windows CRLF line endings

 Recent update to "rebase -i" that tries to sanity check the edited
 insn sheet before it uses it has become too picky on Windows where
 CRLF left by the editor is turned into a trailing CR on the line
 read via the "read" built-in command.

 Waiting for comments.


* jc/add-u-A-default-to-top (2015-10-24) 1 commit
 - add: simplify -u/-A without pathspec

 "git --literal-pathspecs add -u/-A" without any command line
 argument misbehaved ever since Git 2.0.

 Waiting for comments.


* jk/delete-modechange-conflict (2015-10-26) 3 commits
 - merge: detect delete/modechange conflict
 - t6031: generalize for recursive and resolve strategies
 - t6031: move triple-rename test to t3030

 Merging a branch that removes a path and another that changes the
 mode bits on the same path should have conflicted at the path, but
 it didn't and silently favoured the removal.

 Will merge to 'next'.


* jk/war-on-sprintf (2015-10-23) 2 commits
  (merged to 'next' on 2015-10-23 at 3a94851)
 + compat/mingw.c: remove printf format warning
 + read_branches_file: plug a FILE* leak

 Will merge to 'master'.


* js/imap-send-curl-compilation-fix (2015-10-26) 1 commit
 - imap-send: only use CURLOPT_LOGIN_OPTIONS 

Re: [PATCH 2/6] remote-http(s): Support SOCKS proxies

[James McCoy]

On Mon, Oct 26, 2015 at 01:15:17PM -0700, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
> > This patch was required to work behind a faulty AP and scraped from
> > http://stackoverflow.com/questions/15227130/#15228479 and guarded with
> > an appropriate cURL version check by Johannes Schindelin.
> >
> > Signed-off-by: Johannes Schindelin 
> 
> Thanks.
> 
> The code looks OK but the last paragraph makes _us_ worried.  What
> is the licensing status of the original at SO?

According to Stackoverflow[0],

  As noted in the Stack Exchange Terms of Service[1] and in the footer of
  every page, all user contributions are licensed under Creative Commons
  Attribution-Share Alike[2]. Proper attribution[3] is required if you
  republish any Stack Exchange content.

[0]: https://stackoverflow.com/help/licensing
[1]: http://stackexchange.com/legal
[2]: http://creativecommons.org/licenses/by-sa/3.0/
[3]: http://blog.stackoverflow.com/2009/06/attribution-required/

Cheers,
-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace

[Junio C Hamano]

Is there a reason why transfer.hiderefs is not sufficient?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] t6031: move triple-rename test to t3030

[Jeff King]

The t6031 test was introduced to check filemode handling of
merge-recursive. Much later, an unrelated test was tacked on
to look at renames and d/f conflicts. This test does not
depend on anything that happened before (it actually blows
away any existing content in the test repo). Let's move it
to t3030, where there are more related tests.

Signed-off-by: Jeff King 
---
I didn't actually look all that closely at what it does and what t3030
does to see if there is overlap, and we could simply get rid of this.
But it _definitely_ doesn't belong in t6031, so this is at least a step
forward.

 t/t3030-merge-recursive.sh | 30 ++
 t/t6031-merge-recursive.sh | 31 ---
 2 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 82e1854..6224187 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -629,5 +629,35 @@ test_expect_failure 'merge-recursive rename vs. 
rename/symlink' '
test_cmp expected actual
 '
 
+test_expect_success 'merging with triple rename across D/F conflict' '
+   git reset --hard HEAD &&
+   git checkout -b main &&
+   git rm -rf . &&
+
+   echo "just a file" >sub1 &&
+   mkdir -p sub2 &&
+   echo content1 >sub2/file1 &&
+   echo content2 >sub2/file2 &&
+   echo content3 >sub2/file3 &&
+   mkdir simple &&
+   echo base >simple/bar &&
+   git add -A &&
+   test_tick &&
+   git commit -m base &&
+
+   git checkout -b other &&
+   echo more >>simple/bar &&
+   test_tick &&
+   git commit -a -m changesimplefile &&
+
+   git checkout main &&
+   git rm sub1 &&
+   git mv sub2 sub1 &&
+   test_tick &&
+   git commit -m changefiletodir &&
+
+   test_tick &&
+   git merge other
+'
 
 test_done
diff --git a/t/t6031-merge-recursive.sh b/t/t6031-merge-recursive.sh
index 6464a16..4053bd9 100755
--- a/t/t6031-merge-recursive.sh
+++ b/t/t6031-merge-recursive.sh
@@ -53,35 +53,4 @@ test_expect_success FILEMODE 'verify executable bit on file' 
'
test -x file2
 '
 
-test_expect_success 'merging with triple rename across D/F conflict' '
-   git reset --hard HEAD &&
-   git checkout -b main &&
-   git rm -rf . &&
-
-   echo "just a file" >sub1 &&
-   mkdir -p sub2 &&
-   echo content1 >sub2/file1 &&
-   echo content2 >sub2/file2 &&
-   echo content3 >sub2/file3 &&
-   mkdir simple &&
-   echo base >simple/bar &&
-   git add -A &&
-   test_tick &&
-   git commit -m base &&
-
-   git checkout -b other &&
-   echo more >>simple/bar &&
-   test_tick &&
-   git commit -a -m changesimplefile &&
-
-   git checkout main &&
-   git rm sub1 &&
-   git mv sub2 sub1 &&
-   test_tick &&
-   git commit -m changefiletodir &&
-
-   test_tick &&
-   git merge other
-'
-
 test_done
-- 
2.6.2.481.g6ca35c3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Fix missing accelerators causing crashes with old translations

[Mickaël Thomas]

From: mickael9 

Signed-off-by: Mickaël Thomas 
---
 gitk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index 2028b55..fcc606e 100755
--- a/gitk
+++ b/gitk
@@ -12452,8 +12452,8 @@ if {$cmdline_files ne {} || $revtreeargs ne {} || 
$revtreeargscmd ne {}} {
 set viewchanged(1) 0
 set vdatemode(1) 0
 addviewmenu 1
-.bar.view entryconf [mca "Edit view..."] -state normal
-.bar.view entryconf [mca "Delete view"] -state normal
+.bar.view entryconf [mca " view..."] -state normal
+.bar.view entryconf [mca " view"] -state normal
 }
 
 if {[info exists permviews]} {
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[gmane.comp.version-control.git] [PATCH 0/4] gitk crash fix and locale updates

[Junio C Hamano]

Ping.  What do you think of these?  It appears that quite a many
people are getting bitten by the issues this series addresses.

Thanks.

From: Takashi Iwai 
Newsgroups: gmane.comp.version-control.git
Subject: [PATCH 0/4] gitk crash fix and locale updates
Date: Tue, 20 Oct 2015 14:33:00 +0200
Message-ID: <1445344384-12762-1-git-send-email-ti...@suse.de>
Xref: news.gmane.org gmane.comp.version-control.git:279910
Archived-At: 

Hi,

the recent change in gitk to support the menu accelerator broke the
invocation with --all option in non-English locales.  Also, the whole
menu translations are gone by this, too.  This patchset tries to
address these issues.


Takashi

===

Takashi Iwai (4):
  gitk: Fix crash with --all in non-English locales
  gitk: Update msgid's for menu items with accelerator
  gitk: Add accelerators to Japanese locale
  gitk: Add accelerator to German locale

 gitk-git/gitk|  4 +--
 gitk-git/po/bg.po| 34 -
 gitk-git/po/ca.po| 34 -
 gitk-git/po/de.po| 70 ++--
 gitk-git/po/es.po| 34 -
 gitk-git/po/fr.po| 34 -
 gitk-git/po/hu.po| 34 -
 gitk-git/po/it.po| 36 +--
 gitk-git/po/ja.po| 68 +-
 gitk-git/po/pt_br.po | 34 -
 gitk-git/po/ru.po| 34 -
 gitk-git/po/sv.po| 34 -
 gitk-git/po/vi.po| 34 -
 13 files changed, 242 insertions(+), 242 deletions(-)

-- 
2.6.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?

[Noam Postavsky]

On Mon, Oct 26, 2015 at 5:50 PM, Jeff King wrote:
> But these days, people often have several simultaneous sessions open.
> They may have multiple ssh sessions to a single machine, or they may
> have a bunch of terminal windows open, each of which has a login shell
> and will send HUP to its children when it exits.

Yes, except that as far as I can tell the shell never sends HUP.

> This is all further complicated by bash's huponexit option, which I
> think is off by default. So I, for example, have never noticed this
> behavior even with multiple xterms, because my cache never actually gets
> SIGHUP.

Interesting, I was not aware of this option. I tried enabling it, but
I see no difference, the daemon still receives no SIGHUP. Could it be
a bug in my version of bash?

GNU bash, version 4.3.42(1)-release (x86_64-pc-linux-gnu)

Ah, it seems I'm not the only one: (Raphael Ahrens at
http://unix.stackexchange.com/a/85296/47926)

Bash seems to send the SIGHUP only if it self received a SIGHUP[...]
So if you type exit or press Ctrl+D all background process will
remain, since this does not send a hang up signal to the Bash.
[...]
There is an option to send the SIGHUP on exit, but it does not
work on my Bash 4.2.25. Maybe it works for you

> I don't know what shell Noam is using, but I wonder if tweaking
> that option (or a similar one if not bash) might be helpful to signal
> "let this stuff keep running even after I exit".

My normal login shell is zsh, but I've been testing with bash and I
see the same behaviour with both. But the original reason for this
whole thread is that when running from Emacs (not via shell), the
daemon *always* get a SIGHUP as soon as "git push" finishes, which
makes the caching thing not so useful. We do have a workaround for
this by now though (start the daemon independently before calling "git
push").
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Add main menu accelerators to po files

[Mickaël Thomas]

This adds accelerators introduced in previous commit to the po files so
that the old translations without accelerators are used instead of the
untranslated strings with accelerators.

Translations will have to be updated manually to add the new
accelerators in the right place.

Signed-off-by: Mickaël Thomas 
---
 po/bg.po| 30 +++---
 po/ca.po| 30 +++---
 po/de.po| 30 +++---
 po/es.po| 30 +++---
 po/fr.po| 30 +++---
 po/hu.po| 30 +++---
 po/it.po| 30 +++---
 po/ja.po| 30 +++---
 po/pt_br.po | 30 +++---
 po/ru.po| 30 +++---
 po/sv.po| 30 +++---
 po/vi.po| 30 +++---
 12 files changed, 180 insertions(+), 180 deletions(-)

diff --git a/po/bg.po b/po/bg.po
index 61073eb..1822d56 100644
--- a/po/bg.po
+++ b/po/bg.po
@@ -89,39 +89,39 @@ msgid "Cancel"
 msgstr "Отказ"
 
 #: gitk:2069
-msgid "Update"
+msgid ""
 msgstr "Обновяване"
 
 #: gitk:2070
-msgid "Reload"
+msgid ""
 msgstr "Презареждане"
 
 #: gitk:2071
-msgid "Reread references"
+msgid "Reread re"
 msgstr "Наново прочитане на настройките"
 
 #: gitk:2072
-msgid "List references"
+msgid " references"
 msgstr "Изброяване на указателите"
 
 #: gitk:2074
-msgid "Start git gui"
+msgid "Start git "
 msgstr "Стартиране на „git gui“"
 
 #: gitk:2076
-msgid "Quit"
+msgid ""
 msgstr "Спиране на програмата"
 
 #: gitk:2068
-msgid "File"
+msgid ""
 msgstr "Файл"
 
 #: gitk:2080
-msgid "Preferences"
+msgid ""
 msgstr "Настройки"
 
 #: gitk:2079
-msgid "Edit"
+msgid ""
 msgstr "Редактиране"
 
 #: gitk:2084
@@ -133,27 +133,27 @@ msgid "Edit view..."
 msgstr "Редактиране на изгледа…"
 
 #: gitk:2086
-msgid "Delete view"
+msgid " view"
 msgstr "Изтриване на изгледа"
 
 #: gitk:2088 gitk:4043
-msgid "All files"
+msgid " files"
 msgstr "Всички файлове"
 
 #: gitk:2083 gitk:4067
-msgid "View"
+msgid ""
 msgstr "Изглед"
 
 #: gitk:2093 gitk:2103 gitk:3012
-msgid "About gitk"
+msgid " gitk"
 msgstr "Относно gitk"
 
 #: gitk:2094 gitk:2108
-msgid "Key bindings"
+msgid " bindings"
 msgstr "Клавишни комбинации"
 
 #: gitk:2092 gitk:2107
-msgid "Help"
+msgid ""
 msgstr "Помощ"
 
 #: gitk:2185 gitk:8652
diff --git a/po/ca.po b/po/ca.po
index 976037a..d3ce6da 100644
--- a/po/ca.po
+++ b/po/ca.po
@@ -91,39 +91,39 @@ msgid "Cancel"
 msgstr "Cancel·la"
 
 #: gitk:2069
-msgid "Update"
+msgid ""
 msgstr "Actualitza"
 
 #: gitk:2070
-msgid "Reload"
+msgid ""
 msgstr "Recarrega"
 
 #: gitk:2071
-msgid "Reread references"
+msgid "Reread re"
 msgstr "Rellegeix les referències"
 
 #: gitk:2072
-msgid "List references"
+msgid " references"
 msgstr "Llista les referències"
 
 #: gitk:2074
-msgid "Start git gui"
+msgid "Start git "
 msgstr "Inicia el git gui"
 
 #: gitk:2076
-msgid "Quit"
+msgid ""
 msgstr "Surt"
 
 #: gitk:2068
-msgid "File"
+msgid ""
 msgstr "Fitxer"
 
 #: gitk:2080
-msgid "Preferences"
+msgid ""
 msgstr "Preferències"
 
 #: gitk:2079
-msgid "Edit"
+msgid ""
 msgstr "Edita"
 
 #: gitk:2084
@@ -135,27 +135,27 @@ msgid "Edit view..."
 msgstr "Edita la vista..."
 
 #: gitk:2086
-msgid "Delete view"
+msgid " view"
 msgstr "Suprimeix vista"
 
 #: gitk:2088 gitk:4043
-msgid "All files"
+msgid " files"
 msgstr "Tots els fitxers"
 
 #: gitk:2083 gitk:4067
-msgid "View"
+msgid ""
 msgstr "Vista"
 
 #: gitk:2093 gitk:2103 gitk:3012
-msgid "About gitk"
+msgid " gitk"
 msgstr "Quant al gitk"
 
 #: gitk:2094 gitk:2108
-msgid "Key bindings"
+msgid " bindings"
 msgstr "Associacions de tecles"
 
 #: gitk:2092 gitk:2107
-msgid "Help"
+msgid ""
 msgstr "Ajuda"
 
 #: gitk:2185 gitk:8652
diff --git a/po/de.po b/po/de.po
index 1a3264b..c2cad6c 100644
--- a/po/de.po
+++ b/po/de.po
@@ -89,39 +89,39 @@ msgid "Cancel"
 msgstr "Abbrechen"
 
 #: gitk:2069
-msgid "Update"
+msgid ""
 msgstr "Aktualisieren"
 
 #: gitk:2070
-msgid "Reload"
+msgid ""
 msgstr "Neu laden"
 
 #: gitk:2071
-msgid "Reread references"
+msgid "Reread re"
 msgstr "Zweige neu laden"
 
 #: gitk:2072
-msgid "List references"
+msgid " references"
 msgstr "Zweige/Markierungen auflisten"
 
 #: gitk:2074
-msgid "Start git gui"
+msgid "Start git "
 msgstr "»git gui« starten"
 
 #: gitk:2076
-msgid "Quit"
+msgid ""
 msgstr "Beenden"
 
 #: gitk:2068
-msgid "File"
+msgid ""
 msgstr "Datei"
 
 #: gitk:2080
-msgid "Preferences"
+msgid ""
 msgstr "Einstellungen"
 
 #: gitk:2079
-msgid "Edit"
+msgid ""
 msgstr "Bearbeiten"
 
 #: gitk:2084
@@ -133,27 +133,27 @@ msgid "Edit view..."
 msgstr "Ansicht bearbeiten ..."
 
 #: gitk:2086
-msgid "Delete view"
+msgid " view"
 msgstr "Ansicht entfernen"
 
 #: gitk:2088 gitk:4043
-msgid "All files"
+msgid " files"
 msgstr "Alle Dateien"
 
 #: gitk:2083 gitk:4067
-msgid "View"
+msgid ""
 msgstr "Ansicht"
 
 #: gitk:2093 gitk:2103 gitk:3012
-msgid "About gitk"
+msgid " 

Re: [PATCH 2/6] remote-http(s): Support SOCKS proxies

[Junio C Hamano]

James McCoy  writes:

>> The code looks OK but the last paragraph makes _us_ worried.  What
>> is the licensing status of the original at SO?
>
> According to Stackoverflow[0],
>
>   As noted in the Stack Exchange Terms of Service[1] and in the footer of
>   every page, all user contributions are licensed under Creative Commons
>   Attribution-Share Alike[2]. Proper attribution[3] is required if you
>   republish any Stack Exchange content.
>
> [0]: https://stackoverflow.com/help/licensing

Yes, and (please correct me if I am wrong--this is one of the times
I hope I am wrong!) I thought BY-SA does not mesh well with GPLv2,
in which case we cannot use this patch (instead somebody has to
reimplement the same without copying).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/3] Add test to describe expectation of blame --reverse with branched history

[Max Kirillov]

On Sun, Oct 25, 2015 at 11:27:32PM -0700, Junio C Hamano wrote:
> Max Kirillov  writes:
> 
>> If history contains merges from feature branches, `blame --reverse`
>> reports not the commit when the line was actually edited, but head of
>> the last merged branch which was created before the edit.
>>
>> As a workaround, `blame --reverse --first-parent` could be used to find
>> the merge of branch containing the edit, but it was disabled in
>> 95a4fb0eac, because incorrectly specified range could produce in
>> unexpected and meaningless result.
>>
>> Add tests which describe ideal functionality with and without
>> `--first-parent`.
>>
>> Signed-off-by: Max Kirillov 
>
> I _think_ I know why it would be useful to allow "--first-parent" to
> the command; it is useful the same way why "git log --first-parent
> $path" would be a good way to get an overview.
> 
> But I am puzzled by your complaints (I'd characterise the statement
> as such, given your second paragraph calls the combination a
> "workaround") in the first paragraph.  I honestly do not understand
> where it comes from at all.
> 
> The reverse blame begins from an old state and shows the most recent
> child in the history that each line survived to, and it does not
> show what commit removed the line from the original state.  And that
> does not have anything to do with the presence of any merges or
> forks in the history.  The command will always report "not the
> commit that edited the line."  There is nothing special about "If
> the history contains merges".
> 
> If you have this history, for example:
> 
> D---E---F
>/ \*
>   O   X---Y
>\ /
> A---B---C
> 
> where O had the original file, which was not touched by any commits
> on the branch on the upper side, and commit B rewrote all lines of
> the file, running blame in reverse may show A as the last point
> where all lines survived up to, if the "reversed" history happened
> to consider A as the earlier "parent" (in reality it is a child but
> blame is about assigning blame for each line from child to parents
> so in the reversed history, real children becomes parents).  Or it
> may show F as the last point where all lines survived up to, if D
> was picked as the earlier "parent".  Because there is no inherent
> ordering between A and D, both of which are children of O, your
> result is not necessarily "head of the last merged branch".
> 
> But I do not see how "first-parent" would be a workaround for that.
> The option would be useful to force the assignment of blame (in
> reverse) along the first-parent chain O---D---E---F---X---Y so that
> you can get a bird's-eye view of the history, i.e. squashing all
> that happened in A---B---C as if that happened at X.
> 
> The explanation of the first paragraph needs to be rewritten to make
> it understandable, but I am not sure what relevance it has with this
> change.

I understand how the blame works and why does it produce the
result which it used to produce. In one of my letter I
called it "technically correct, but absolutely useless", and
let me explain why I think so.

In a big project which uses the nowadays conventional topic
branches aka pull-requests aka however it's named workflow,
the history is a straigh first-parent chain with
short-living branches, which are forked from it, exists for
several days, then merged back and closed. When there are
many people working on a project, there can be tens of
merges during day, and average pull-request exists for
several days. So the history looks rather like (the
interesting line is removed in B1, line removal is
practically more interesting case because edits can be found
with normal forward blame):


 a0--a1-*a2-*a3-a4...-*a100
 |\ /   / /
 | b0-B1..bN   / /
 |\   / /
 | c0..   ..cN /
 \/
  z0....zN


...where many of the c-z branches started before a1 and
contain the older version of line. And, what I usually need
is the change b0->B1, because I expect to find there the
person who did it and explanation why that was done.

Now the git blame --reverse a0..a100 may return me zN, and in
practice it often does return some quite late commit wN
which was merged to some a90. Then, I continue search with
range a0..a89, and so on. So, to find the commit B1 I might
have to perform many blames. 

(I realize that this behavior is correct, and it's even not
obvious how to formally specify the b0 commit as something
different than zM commit, so we could discuss the
implementation of its search. But it does not make me want
being able to find it less)

In contrast, git blame --reverse --first-parent gives me a1,
and then I need only one more step:
git blame --reverse --first-parent a0..bN (--first-parent
for case there are synchronizing merges from master). And,
moreover, in the commit message of a2 I can often find the
information which I expect to find in 

Re: git repack command on larger pack file

[Sivakumar Selvam]

Junio C Hamano  pobox.com> writes:

> 
> Junio C Hamano  pobox.com> writes:
> 
> > Sivakumar Selvam  gmail.com> writes:
> >
> >> ... So
> >> I thought of splitting the pack file into 4 GB chunks.
> > ...
> > Hmmm, what is "this issue"?  I do not see anything surprising.
> 
> While the explanation might have been enlightening, the knowledge
> conveyed by the explanation by itself would not be of much practical
> use, and enlightment without practical use is never fun.
> 
> So let's do another tangent that may be more useful.
> 
> In many repositories, older parts of the history often hold the bulk
> of objects that do not change, and it is wasteful to repack them
> over and over.  If your project is at around v40.0 today, and it was
> at around v36.0 6 months ago, for example, you may want to pack
> everything that happened before v36.0 into a single pack just once,
> pack them really well, and have your "repack" not touch that old
> part of the history.
> 
>   $ git rev-list --objects v36.0 |
> git pack-objects --window=200 --depth=128 pack
> 
> would produce such a pack [*1*]
> 
> The standard output from the above pipeline will give you a 40-hex
> string (e.g. 51c472761b4690a331c02c90ec364e47cca1b3ac, call it
> $HEX), and in the current directory you will find two files,
> pack-$HEX.pack and pack-$HEX.idx.
> 
> You can then do this:
> 
>   $ echo "v36.0 with W/D 200/128" >pack-$HEX.keep
>   $ mv pack-$HEX.* .git/objects/pack/.
>   $ git repack -a -d
> 
> A pack that has an accompanying .keep file is excempt from
> repacking, so once you do this, your future "git repack" will only
> repack objects that are not in the kept packs.
> 
> [Footnote]
> 
> *1* I won't say 200/128 gives you a good pack; you would need to
> experiment.  In general, larger depth will result in smaller pack
> but it will result in bigger overhead while you use the repository
> every day.  Larger window will spend a lot of cycles while packing,
> but will result in a smaller pack.
> 


Hi Junio,

   When I finished git repacking, I found 12 pack files with each 4 GB and
the total size is 48 GB. Again I ran the same git repack command by just
removing only --max-pack-size= parameter, the size of the single pack file
is 66 GB.

git repack -A -b -d -q --depth=50 --window=10 abc.git

Now, I see the total size of the single abc.git has become 66 GB. Initially
it was 34 GB, After using  --max-pack-size=4g it become 48 GB. When we
remove the --max-pack-size=4g parameter and tried to create a single pack
file now it become 66 GB.
   
Looks like once we do git repack with multiple pack files, we can't revert
back to the original size.  

Thanks,
Sivakumar Selvam.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3] Add git-grep threads param

[Victor Leschuk]

Hello John,

see comments inline.

>> @@ -22,6 +22,7 @@ SYNOPSIS
>>  [--color[=] | --no-color]
>>  [--break] [--heading] [-p | --show-function]
>>  [-A ] [-B ] [-C ]
>> +[--threads ]

> Is this the best place for this option?  I know the current list isn't
> sorted in any particular way, but here you're splitting up the set of
> context options (`-A`, `-B`, `-C` and `-W`).

Agree, I'll move the option both here and in documentation.

>> -static int wait_all(void)
>> +static int wait_all(struct grep_opt *opt)

> I'm not sure passing a grep_opt in here is the cleanest way to do this.
> Options are a UI concept and all we care about here is the number of
> threads.

> Since `threads` is a global, shouldn't the number of threads be a global
> as well?  Could we reuse `use_threads` here (possibly renaming it
> `num_threads`)?

This thought also crossed my mind, however we already pass grep_opt to 
start_threads() function,
so I think passing it to wait_all() is not that ugly, and kind of symmetric. 
And I do not like the idea
of duplicating same information in different places. What do you think?

--
Best Regards,
Victor
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[BUG] diff --word-diff inconsistently places removed words

[Lucian Poston]

In the output generated by diff --word-diff, if the first word of a line was
removed, it may appear at the end of the previous line. For example, compare
the following two diffs on the same files, the first diff without --word-diff
and the second with it.


$ git diff

diff --git a/smallest-test b/smallest-test
index bc5f07a..d9116b1 100644
--- a/smallest-test
+++ b/smallest-test
@@ -1,7 +1,7 @@
 
-111 aaa
-111 aaa
+ aaa
+ aaa
 
 111 aaa
-111 aaa
+ aaa
 


$ git diff --word-diff=plain

diff --git a/smallest-test b/smallest-test
index bc5f07a..d9116b1 100644
--- a/smallest-test
+++ b/smallest-test
@@ -1,7 +1,7 @@

[-111-] aaa[-111-]
 aaa

111 aaa
[-111-] aaa


I would expect every [-111-] to be on the line where it was removed. Instead
it appears on the previous line in the case where the previous line also has a
removal.

Is this expected behavior?

There is also a patch in this email thread that contains a test to further
illustrate the issue. Running the test fails with the following diff between
the expected result and actual result.

--- expect  2015-10-27 01:08:05.415252713 +
+++ output.decrypted2015-10-27 01:08:05.418252766 +
@@ -6,5 +6,5 @@
 11aa
 11aa
 
-11aa
-11aa
+11aa11
+aa


Lucian Poston (1):
  t4034: Test parsing words following newline

 t/t4034-diff-words.sh | 30 ++
 1 file changed, 30 insertions(+)

-- 
2.4.10

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] t4034: Test parsing words following newline

[Lucian Poston]

Add test that shows diff --word-diff's inconsistent placement of removed
words. This occurs when both the removed word is first on its line and the
previous line has a removed word.

Signed-off-by: Lucian Poston 
---
 t/t4034-diff-words.sh | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index f2f55fc..8d9f1b7 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -265,6 +265,36 @@ test_expect_success 'test parsing words for newline' '
word_diff --color-words="a+"
 '
 
+test_expect_success 'test parsing words following newline' '
+   cat >pre <<-\EOF &&
+   11aa
+   11aa
+   
+   11aa
+   11aa
+   EOF
+   cat >post <<-\EOF &&
+   11aa
+   aa
+   
+   aa
+   aa
+   EOF
+   cat >expect <<-\EOF &&
+   diff --git a/pre b/post
+   index 2e0acd4..f6e6721 100644
+   --- a/pre
+   +++ b/post
+   @@ -1,5 +1,5 @@
+   11aa
+   11aa
+   
+   11aa
+   11aa
+   EOF
+   word_diff --color-words="a+|1+"
+'
+
 test_expect_success 'test when words are only removed at the end' '
echo "(:" >pre &&
echo "(" >post &&
-- 
2.4.10

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 00/34] libify mailinfo and call it directly from am

[Junio C Hamano]

Johannes Schindelin  writes:

>> > The corresponding times for me were:
>> >
>> >  (master)   (with the series)
>> >real0m9.760s  real  0m5.744s
>> >user0m0.531s  user  0m0.656s
>> >sys 0m5.726s  sys   0m3.520s
>> >
>> > So, yes, a noticeable improvement! :)
>> 
>> Same here, on Windows built with the old msysgit environment:
>> 
>> (master) (with the series)
>> real0m3.147s  real0m1.947s
>> user0m0.016s  user0m0.000s
>> sys 0m0.015s  sys 0m0.031s
>> 
>> Although I tested 'git am patches/*' where the patches/* are the result of
>> 'git-format-patch v2.6.1..github/jc/mailinfo-lib'.
>
> 2.548s vs 2.068s here.
>
> Ciao,
> Johannes

Thanks.  Let's start thinking about moving it forward.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] run-command: factor out child_process_clear()

[Junio C Hamano]

René Scharfe  writes:

> Avoid duplication by moving the code to release allocated memory for
> arguments and environment to its own function, child_process_clear().
> Export it to provide a counterpart to child_process_init().
>
> Signed-off-by: Rene Scharfe 
> ---

Hmm, is this _deinit() Stefan added to his series recently?

>  Documentation/technical/api-run-command.txt |  7 +++
>  run-command.c   | 15 +--
>  run-command.h   |  1 +
>  3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/technical/api-run-command.txt 
> b/Documentation/technical/api-run-command.txt
> index a9fdb45..8bf3e37 100644
> --- a/Documentation/technical/api-run-command.txt
> +++ b/Documentation/technical/api-run-command.txt
> @@ -46,6 +46,13 @@ Functions
>   The argument dir corresponds the member .dir. The argument env
>   corresponds to the member .env.
>  
> +`child_process_clear`::
> +
> + Release the memory associated with the struct child_process.
> + Most users of the run-command API don't need to call this
> + function explicitly because `start_command` invokes it on
> + failure and `finish_command` calls it automatically already.
> +
>  The functions above do the following:
>  
>  . If a system call failed, errno is set and -1 is returned. A diagnostic
> diff --git a/run-command.c b/run-command.c
> index e17e456..13fa452 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -11,6 +11,12 @@ void child_process_init(struct child_process *child)
>   argv_array_init(>env_array);
>  }
>  
> +void child_process_clear(struct child_process *child)
> +{
> + argv_array_clear(>args);
> + argv_array_clear(>env_array);
> +}
> +
>  struct child_to_clean {
>   pid_t pid;
>   struct child_to_clean *next;
> @@ -327,8 +333,7 @@ int start_command(struct child_process *cmd)
>  fail_pipe:
>   error("cannot create %s pipe for %s: %s",
>   str, cmd->argv[0], strerror(failed_errno));
> - argv_array_clear(>args);
> - argv_array_clear(>env_array);
> + child_process_clear(cmd);
>   errno = failed_errno;
>   return -1;
>   }
> @@ -513,8 +518,7 @@ fail_pipe:
>   close_pair(fderr);
>   else if (cmd->err)
>   close(cmd->err);
> - argv_array_clear(>args);
> - argv_array_clear(>env_array);
> + child_process_clear(cmd);
>   errno = failed_errno;
>   return -1;
>   }
> @@ -540,8 +544,7 @@ fail_pipe:
>  int finish_command(struct child_process *cmd)
>  {
>   int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);
> - argv_array_clear(>args);
> - argv_array_clear(>env_array);
> + child_process_clear(cmd);
>   return ret;
>  }
>  
> diff --git a/run-command.h b/run-command.h
> index 5428b04..12bb26c 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -47,6 +47,7 @@ struct child_process {
>  
>  #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
>  void child_process_init(struct child_process *);
> +void child_process_clear(struct child_process *);
>  
>  int start_command(struct child_process *);
>  int finish_command(struct child_process *);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] Facilitate debugging Git executables in tests with gdb

[Johannes Schindelin]

When prefixing a Git call in the test suite with 'TEST_GDB_GIT=1 ', it
will now be run with GDB, allowing the developer to debug test failures
more conveniently.

Signed-off-by: Johannes Schindelin 
---
 wrap-for-bin.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
index 701d233..a151c95 100644
--- a/wrap-for-bin.sh
+++ b/wrap-for-bin.sh
@@ -19,4 +19,11 @@ GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
 PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
 export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
 
+if test -n "$TEST_GDB_GIT"
+then
+   exec gdb -args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+   echo "Could not run gdb -args ${GIT_EXEC_PATH}/@@PROG@@ $*" >&2
+   exit 1
+fi
+
 exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] Squelch warning about an integer overflow

[Johannes Schindelin]

We cannot rely on long integers to have more than 32 bits...

Signed-off-by: Johannes Schindelin 
---
 git-compat-util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 805d0e2..610e8a5 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -568,7 +568,7 @@ extern int git_lstat(const char *, struct stat *);
 #endif
 
 #define DEFAULT_PACKED_GIT_LIMIT \
-   ((1024L * 1024L) * (sizeof(void*) >= 8 ? 8192 : 256))
+   ((1024L * 1024L) * (size_t)(sizeof(void*) >= 8 ? 8192 : 256))
 
 #ifdef NO_PREAD
 #define pread git_pread
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] Silence GCC's "cast of pointer to integer of a different size" warning

[Johannes Schindelin]

When calculating hashes from pointers, it actually makes sense to cut
off the most significant bits. In that case, said warning does not make
a whole lot of sense.

So let's just work around it.

Signed-off-by: Johannes Schindelin 
---
 compat/regex/regcomp.c | 6 --
 pack-revindex.c| 2 +-
 sha1_file.c| 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c
index 06f3088..fba5986 100644
--- a/compat/regex/regcomp.c
+++ b/compat/regex/regcomp.c
@@ -18,6 +18,8 @@
Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA.  */
 
+#include 
+
 static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern,
  size_t length, reg_syntax_t syntax);
 static void re_compile_fastmap_iter (regex_t *bufp,
@@ -2577,7 +2579,7 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, 
re_dfa_t *dfa,
 old_tree = NULL;
 
   if (elem->token.type == SUBEXP)
-postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx);
+postorder (elem, mark_opt_subexp, (void *) (intptr_t) elem->token.opr.idx);
 
   tree = create_tree (dfa, elem, NULL, (end == -1 ? OP_DUP_ASTERISK : OP_ALT));
   if (BE (tree == NULL, 0))
@@ -3806,7 +3808,7 @@ create_token_tree (re_dfa_t *dfa, bin_tree_t *left, 
bin_tree_t *right,
 static reg_errcode_t
 mark_opt_subexp (void *extra, bin_tree_t *node)
 {
-  int idx = (int) (long) extra;
+  int idx = (int) (intptr_t) extra;
   if (node->token.type == SUBEXP && node->token.opr.idx == idx)
 node->token.opt_subexp = 1;
 
diff --git a/pack-revindex.c b/pack-revindex.c
index 5c8376e..e542ea7 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -21,7 +21,7 @@ static int pack_revindex_hashsz;
 
 static int pack_revindex_ix(struct packed_git *p)
 {
-   unsigned long ui = (unsigned long)p;
+   unsigned long ui = (unsigned long)(intptr_t)p;
int i;
 
ui = ui ^ (ui >> 16); /* defeat structure alignment */
diff --git a/sha1_file.c b/sha1_file.c
index 50896ff..c5b31de 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2126,7 +2126,7 @@ static unsigned long pack_entry_hash(struct packed_git 
*p, off_t base_offset)
 {
unsigned long hash;
 
-   hash = (unsigned long)p + (unsigned long)base_offset;
+   hash = (unsigned long)(intptr_t)p + (unsigned long)base_offset;
hash += (hash >> 8) + (hash >> 16);
return hash % MAX_DELTA_CACHE;
 }
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] Correct fscanf formatting string for I64u values

[Johannes Schindelin]

From: Waldek Maleska 

This fix is probably purely cosmetic because PRIuMAX is likely identical
to SCNuMAX. Nevertheless, when using a function of the scanf() family,
the correct interpolation to use is the latter, not the former.

Signed-off-by: Waldek Maleska 
Signed-off-by: Johannes Schindelin 
---
 builtin/gc.c  | 2 +-
 git-compat-util.h | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index b677923..df3e454 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -240,7 +240,7 @@ static const char *lock_repo_for_gc(int force, pid_t* 
ret_pid)
 * running.
 */
time(NULL) - st.st_mtime <= 12 * 3600 &&
-   fscanf(fp, "%"PRIuMAX" %127c", , locking_host) == 2 
&&
+   fscanf(fp, "%"SCNuMAX" %127c", , locking_host) == 2 
&&
/* be gentle to concurrent "gc" on remote hosts */
(strcmp(locking_host, my_host) || !kill(pid, 0) || 
errno == EPERM);
if (fp != NULL)
diff --git a/git-compat-util.h b/git-compat-util.h
index 610e8a5..87456a3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -296,6 +296,10 @@ extern char *gitbasename(char *);
 #define PRIuMAX "llu"
 #endif
 
+#ifndef SCNuMAX
+#define SCNuMAX PRIuMAX
+#endif
+
 #ifndef PRIu32
 #define PRIu32 "u"
 #endif
-- 
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] remote-http(s): Support SOCKS proxies

[Johannes Schindelin]

From: Pat Thoyts 

With this patch we properly support SOCKS proxies, configured e.g. like
this:

git config http.proxy socks5://192.168.67.1:32767

Without this patch, Git mistakenly tries to use SOCKS proxies as if they
were HTTP proxies, resulting in a error message like:

fatal: unable to access 'http://.../': Proxy CONNECT aborted

This patch was required to work behind a faulty AP and scraped from
http://stackoverflow.com/questions/15227130/#15228479 and guarded with
an appropriate cURL version check by Johannes Schindelin.

Signed-off-by: Johannes Schindelin 
---
 http.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/http.c b/http.c
index 7da76ed..6b89dea 100644
--- a/http.c
+++ b/http.c
@@ -465,6 +465,17 @@ static CURL *get_curl_handle(void)
 
if (curl_http_proxy) {
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+#if LIBCURL_VERSION_NUM >= 0x071800
+   if (starts_with(curl_http_proxy, "socks5"))
+   curl_easy_setopt(result,
+   CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
+   else if (starts_with(curl_http_proxy, "socks4a"))
+   curl_easy_setopt(result,
+   CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4A);
+   else if (starts_with(curl_http_proxy, "socks"))
+   curl_easy_setopt(result,
+   CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
+#endif
}
 #if LIBCURL_VERSION_NUM >= 0x070a07
curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 00/34] libify mailinfo and call it directly from am

[Johannes Schindelin]

Hi,

On Wed, 21 Oct 2015, Johannes Sixt wrote:

> Am 21.10.2015 um 17:51 schrieb Ramsay Jones:
> > On 20/10/15 22:24, Junio C Hamano wrote:
> > > Junio C Hamano  writes:
> > > some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
> > > running Ubuntu),
> >
> > I suspect that I haven't tested exactly the same version as you, but I had
> > a quick look at testing this on Cygwin today. I have included a complete
> > transcript (below), so you can see what I did wrong! :-P
> >
> > >
> > > Between 'master' and the version with this series (on 'jch'),
> > > applying this 34-patch series itself on top of 'master' using "git
> > > am", best of 5 numbers for running:
> > >
> > >  time git am mbox >/dev/null
> > >
> > > are
> > >
> > >(master) (with the series)
> > >  real0m0.648sreal0m0.537s
> > >  user0m0.358suser0m0.338s
> > >  sys 0m0.172ssys 0m0.154s
> > >
> >
> > The corresponding times for me were:
> >
> >  (master)   (with the series)
> >real 0m9.760s  real  0m5.744s
> >user 0m0.531s  user  0m0.656s
> >sys  0m5.726s  sys   0m3.520s
> >
> > So, yes, a noticeable improvement! :)
> 
> Same here, on Windows built with the old msysgit environment:
> 
> (master) (with the series)
> real0m3.147s  real0m1.947s
> user0m0.016s  user0m0.000s
> sys 0m0.015s  sys 0m0.031s
> 
> Although I tested 'git am patches/*' where the patches/* are the result of
> 'git-format-patch v2.6.1..github/jc/mailinfo-lib'.

2.548s vs 2.068s here.

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Filters and diff vs. status

[Pietro Battiston]

Dear mailing list,

I am using, on specific files, a filter which I created.¹ Often, files
being tracked are modified in a way not meant to be reflected in the
git history to result in changes to be committed. And sometimes, the
following happens:

pietro@debiousci:~/path/to/repo$ git status a_file.ipynb
On branch master
Your branch is up-to-date with 'origin/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:   a_file.ipynb

no changes added to commit (use "git add" and/or "git commit -a")
pietro@debiousci:~/path/to/repo$ git diff a_file.ipynb
pietro@debiousci:~/path/to/repo$


(notice that no diff shown). Then I wonder what is happening (checking
out the file doesn't help, resetting --hard neither), and I try to
commit the (apparently unchanged) file . And I get the following:


pietro@debiousci:~/path_to_repo$ git commit a_file.ipynb -m '?!'
[master
c76125a] ?!
 1 file changed, 1 insertion(+), 1 deletion(-)
pietro@debiousci:~/path/to/repo$ git show HEAD
commit c76125a537f88db4ff5d13c97b92e1f01c13bb47
Author: Pietro Battiston 
Date:   Mon Oct 26 13:37:24 2015 +0100

?!

diff --git a/a_file.ipynb b/a_file.ipynb
index dfdbd79..61663fb 100644
--- a/notebook/a_file.ipynb
+++ b/notebook/a_file.ipynb
@@ -349,4 +349,4 @@
"metadata": {}
   }
  ]
-}
\ No newline at end of file
+}


Now, I don't particularly care about a newline being present or not at
the end of the file, but the fact that the working tree looks dirty
(forbidding me from doing merges) - and that I don't understand why,
and how to fix this without adding bogus commits, annoys me.
I found an analogous behaviour reported some years ago,² and the
conclusion was "I think that there is a bug. I have observed this as
well with my own clean filter sometimes, but not always. I haven't
found a recipe that reliably exhibits the problem." Apart from that, I
have found no clue of why "git diff" and "git status" do not agree.

Does anybody have any pointer to solve this? I could try to play with
my filter imposing that it adds/doesn't add a newline at the end of its
output, but I am really missing the logic of what is going on on the
git side.

Thanks in advance,

Pietro



¹ I don't want to waste anybody's time (I guess the answer to my
problem is pretty general), but for more details see
 https://github.com/toobaz/ipynb_output_filter , presenting both the
script and the configuration I use.

² 
http://thread.gmane.org/gmane.comp.version-control.git/125378/focus=125
684
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/6] Miscellaneous platform-independent patches from Git for Windows

[Johannes Schindelin]

While working on Git for Windows 2.x, a couple of fixes were necessary
that are not actually specific to Windows.

For example, when stuck behind a faulty Access Point that somehow worked
with an Android phone, but not with this developer's MacBook, it was
necessary to use a SOCKS proxy via the phone to be able to continue
developing Git for Windows.

This is the first patch series attempting to lift the patches from Git
for Windows' friendly fork into upstream Git.


Johannes Schindelin (4):
  Only use CURLOPT_LOGIN_OPTIONS if it is actually available
  Facilitate debugging Git executables in tests with gdb
  Squelch warning about an integer overflow
  Silence GCC's "cast of pointer to integer of a different size" warning

Pat Thoyts (1):
  remote-http(s): Support SOCKS proxies

Waldek Maleska (1):
  Correct fscanf formatting string for I64u values

 builtin/gc.c   |  2 +-
 compat/regex/regcomp.c |  6 --
 git-compat-util.h  |  6 +-
 http.c | 11 +++
 imap-send.c|  4 
 pack-revindex.c|  2 +-
 sha1_file.c|  2 +-
 wrap-for-bin.sh|  7 +++
 8 files changed, 34 insertions(+), 6 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] Add git-grep threads param

[Victor Leschuk]

Make number of git-grep worker threads a configuration parameter.
According to several tests on systems with different number of CPU cores
the hard-coded number of 8 threads is not optimal for all systems:
tuning this parameter can significantly speed up grep performance.

Signed-off-by: Victor Leschuk 
---
 Documentation/config.txt   |  4 
 Documentation/git-grep.txt |  4 
 builtin/grep.c | 34 ++
 contrib/completion/git-completion.bash |  1 +
 grep.c | 10 ++
 grep.h |  2 ++
 6 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..1c95587 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1447,6 +1447,10 @@ grep.extendedRegexp::
option is ignored when the 'grep.patternType' option is set to a value
other than 'default'.
 
+grep.threads::
+   Number of grep worker threads, use it to tune up performance on
+   multicore machines. Default value is 8.
+
 gpg.program::
Use this custom program instead of "gpg" found on $PATH when
making or verifying a PGP signature. The program must support the
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 4a44d6d..fbd4f83 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -22,6 +22,7 @@ SYNOPSIS
   [--color[=] | --no-color]
   [--break] [--heading] [-p | --show-function]
   [-A ] [-B ] [-C ]
+  [--threads ]
   [-W | --function-context]
   [-f ] [-e] 
   [--and|--or|--not|(|)|-e ...]
@@ -220,6 +221,9 @@ OPTIONS
Show  leading lines, and place a line containing
`--` between contiguous groups of matches.
 
+--threads ::
+   Set number of worker threads to . Default is 8.
+
 -W::
 --function-context::
Show the surrounding text from the previous line containing a
diff --git a/builtin/grep.c b/builtin/grep.c
index d04f440..5ef1b07 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -27,8 +27,7 @@ static char const * const grep_usage[] = {
 static int use_threads = 1;
 
 #ifndef NO_PTHREADS
-#define THREADS 8
-static pthread_t threads[THREADS];
+static pthread_t *threads;
 
 /* We use one producer thread and THREADS consumer
  * threads. The producer adds struct work_items to 'todo' and the
@@ -206,7 +205,8 @@ static void start_threads(struct grep_opt *opt)
strbuf_init([i].out, 0);
}
 
-   for (i = 0; i < ARRAY_SIZE(threads); i++) {
+   threads = xcalloc(opt->num_threads, sizeof(pthread_t));
+   for (i = 0; i < opt->num_threads; i++) {
int err;
struct grep_opt *o = grep_opt_dup(opt);
o->output = strbuf_out;
@@ -220,7 +220,7 @@ static void start_threads(struct grep_opt *opt)
}
 }
 
-static int wait_all(void)
+static int wait_all(struct grep_opt *opt)
 {
int hit = 0;
int i;
@@ -238,12 +238,14 @@ static int wait_all(void)
pthread_cond_broadcast(_add);
grep_unlock();
 
-   for (i = 0; i < ARRAY_SIZE(threads); i++) {
+   for (i = 0; i < opt->num_threads; i++) {
void *h;
pthread_join(threads[i], );
hit |= (int) (intptr_t) h;
}
 
+   free(threads);
+
pthread_mutex_destroy(_mutex);
pthread_mutex_destroy(_read_mutex);
pthread_mutex_destroy(_attr_mutex);
@@ -256,7 +258,7 @@ static int wait_all(void)
 }
 #else /* !NO_PTHREADS */
 
-static int wait_all(void)
+static int wait_all(struct grep_opt *opt)
 {
return 0;
 }
@@ -702,6 +704,8 @@ int cmd_grep(int argc, const char **argv, const char 
*prefix)
N_("show  context lines before matches")),
OPT_INTEGER('A', "after-context", _context,
N_("show  context lines after matches")),
+   OPT_INTEGER(0, "threads", _threads,
+   N_("use  worker threads")),
OPT_NUMBER_CALLBACK(, N_("shortcut for -C NUM"),
context_callback),
OPT_BOOL('p', "show-function", ,
@@ -832,8 +836,22 @@ int cmd_grep(int argc, const char **argv, const char 
*prefix)
}
 
 #ifndef NO_PTHREADS
-   if (list.nr || cached || online_cpus() == 1)
+   if (!opt.num_threads) {
+   use_threads = 0; /* User explicitely told not to use threads */
+   }
+   else if (list.nr || cached) {
+   use_threads = 0; /* Can not multi-thread object lookup */
+   }
+   else if (opt.num_threads >= 0) {
+   use_threads = 1; /* User explicitely set the number of threads 
*/
+   }
+   else if (online_cpus() <= 1) {
use_threads = 0;
+   }
+   else {
+   use_threads = 1;
+   

[PATCH 1/6] Only use CURLOPT_LOGIN_OPTIONS if it is actually available

[Johannes Schindelin]

This fixes the compilation on an older Linux that was used to debug
test failures when upgrading Git for Windows to Git v2.3.0.

Signed-off-by: Johannes Schindelin 
---
 imap-send.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/imap-send.c b/imap-send.c
index e9faaea..4d3b773 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1421,11 +1421,15 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
curl_easy_setopt(curl, CURLOPT_PORT, server.port);
 
if (server.auth_method) {
+#if LIBCURL_VERSION_NUM < 0x072200
+   warning("No LOGIN_OPTIONS support in this cURL version");
+#else
struct strbuf auth = STRBUF_INIT;
strbuf_addstr(, "AUTH=");
strbuf_addstr(, server.auth_method);
curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
strbuf_release();
+#endif
}
 
if (!server.use_ssl)
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] URL rewrite in .gitmodules

[Stefan Beller]

On Sun, Oct 25, 2015 at 8:12 AM, Lars Schneider
 wrote:
>
> On 20 Oct 2015, at 19:33, Junio C Hamano  wrote:
>
>> Lars Schneider  writes:
>>
>>> If not, what do you think about a patch that adds a "url" section
>>> similar to the one in git config to a .gitmodules file?
>>>
>>> Example:
>>> --
>>> [submodule "git"]
>>>  path = git
>>>url=git://github.com/larsxschneider/git.git
>>>
>>> [url "mycompany.com"]
>>>insteadOf = outside.com
>>> --
>>
>> It is unclear to me if you are adding the last two (or three,
>> counting the blank before) lines to your company's private fork of
>> the opensource project, but if that is the case, then that would
>> defeat your earlier desire:
>>
>>> ... I also would prefer not to do this as I want to use the
>>> very same hashes as defined by the "upstream" ...
>>
>> wouldn't it?
> The last three lines are added to my companies closed source Git repo. In 
> this example the company repo references 
> git://github.com/larsxschneider/git.git as submodule. This submodule in turn 
> references another submodule with a URL "outside.com". This is the URL I want 
> to rewrite. Do you think this could be useful to others as well?
>
>
>> I do not think this topic is specific to use of submodules.  If you
>> want to encourage your engineers to fetch from nearby mirrors you
>> maintain, you would want a forest of url.mine.insteadof=theirs for
>> the external repositories that matter to you specified by
>> everybody's $HOME/.gitconfig, and one way to do so would be to have
>> them use the configuration inclusion.  An item in your engineer
>> orientation material could tell them to add
>>
>>   [include]
>>   path = /usr/local/etc/git/mycompany.urlrewrite
>>
>> when they set up their "[user] name/email" in there.
>>
>> And you can update /usr/local/etc/git/mycompany.urlrewrite as
>> needed.
> Oh nice, I didn't know about "include". However, as mentioned to Stefan in 
> this thread, I fear that our engineers will miss that. I would prefer a 
> solution that does not need any additional setup. Therefore the suggestion to 
> add rewrites in the .gitmodules file.

How do you distribute new copies of Git to your engineers?
Maybe you could ship them a version which has the "include" line
already builtin as default? So your distributed copy of Git
would not just check the default places for configs, but also
some complied in /net/share/mycompany.gitconfig

>
> Thanks,
> Lars
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Why are submodules not automatically handled by default or at least configurable to do so?

[Jens Lehmann]

Am 26.10.2015 um 05:48 schrieb Nazri Ramliy:

On Mon, Oct 26, 2015 at 7:10 AM, John Smith  wrote:

When would people routinely check out a branch and want to stay with the 
submodules as
the have been checked out for the old branch?


I do this a lot. At my $dayjob we have a super project with bunch of
sub projects.
Each subproject has its corresponding rpm spec file in the
superproject - it's quite
often that I work on a "git-merge-base--octopus" branch that updates only the
spec files and nothing else - so when changing between branches I
don't care what
states the submodules are in. When the fixes to the spec files are ready I just
checkout to the respective branches and merge in the changes - I don't actively
do "git submodule update" when switching to different branches.


Which seems a bit error prone as you could forget to update the submodules
and build incorrect rpms from them, or am I missing something?

I understand why you don't need to update the submodules every time, but
would it hurt your workflow if they did (but don't get me wrong, that will
always be configurable).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ref-filter: fallback on alphabetical comparison

[Karthik Nayak]

On Mon, Oct 26, 2015 at 6:43 AM, Eric Sunshine  wrote:
> On Sun, Oct 25, 2015 at 12:29 PM, Karthik Nayak  wrote:
>> In ref-filter.c the comparison of refs while sorting is handled by
>> cmp_ref_sorting() function. When sorting as per numerical values
>> (e.g. --sort=objectsize) there is no fallback comparison when both
>> refs hold the same value. This can cause unexpected results (i.e. the
>> order of listting refs with equal values cannot be pre-determined) as
>
> s/listting/listing/

Thanks for pointing out. Should I re-roll?

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Why are submodules not automatically handled by default or at least configurable to do so?

[Stefan Beller]

On Sun, Oct 25, 2015 at 5:56 PM, Chris Packham  wrote:
> On Mon, Oct 26, 2015 at 12:10 PM, John Smith  wrote:
>> I found that I use submodules much, much more often in my git projects than 
>> I used externals
>> in Subversion and the reason is that git encourages/forces to organize large 
>> projects into
>> smaller repositories, one reason for this being that subversion allows to 
>> check out parts of
>> a repository while git does not.
>>
>> But when I clone a git repository with subprojects, I (and everyone else) 
>> has to remember to
>> add the --recursive option. When switching between branches with different 
>> versions/commits of the
>> submodules everyone has to remember to update the submodules. When updating 
>> a submodule
>> everyone has to remember to recurse there too.
>
> The config option fetch.recurseSubmodules exists. It's not quite the
> same as what git clone --recurse-submodules does but it's a start.
>
>>
>> Basically, everything with submodules has to be done manually every time and 
>> there seems
>> to be no way to change that default.
>>
>> Why is that? Basically all the time I use submodules I would want automatic 
>> handling of
>> submodules to happen and I cannot  remember having had a single situation 
>> where I would
>> not have wanted it to happen. So  why does git default to doing nothing?

IIUC at the time submodules were invented, there was need for lots of
code to be written.
Each command needed new code to deal with submodules. As there was not
enough people/time
to do it properly, the "do nothing" was the safest action which could
be added fast.

>
> It's hard to pick a default that suits every workflow that submodules
> support. Also with submodules there is a chicken-and-egg scenario.
> While you can put things in ~/.gitconfig most of what you'd want to
> configure when using submodules would be in super/.git/config but that
> doesn't exist until you've cloned super.git.
>
>> Why does it not provide a way to enable automatic
>> pulling/updating of submodules e.g. when cloning or switching branches?
>
> I believe Jens and Stefan (Cc'd) have been doing some great work in
> this area. Jens even posted his todo list a few days ago
> (https://github.com/jlehmann/git-submod-enhancements/wiki).

Yeah I would also point at Jens' wiki today.

All I did up to now was rewriting parts of the submodule code in C
(git submodule update specifically), while the code/patches you find at Jens'
copy of Git includes already lots of useful stuff such as `git
checkout --recurse-submodules`
(IIRC you don't need to type --recurse-submodules if you configured
that to be the default)

>
>> When would people routinely check out a branch and want to stay with the 
>> submodules as
>> the have been checked out for the old branch?

As said above, it was a sane choice which could be implemented fast, IIUC.

I mean what would happen if you had commits made in the submodule, or
just a dirty working tree?

>>
>> I honestly do not understand it.
>>
>> John
>>

Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] URL rewrite in .gitmodules

[Jens Lehmann]

Am 26.10.2015 um 17:34 schrieb Stefan Beller:

On Sun, Oct 25, 2015 at 8:12 AM, Lars Schneider  
wrote:

On 20 Oct 2015, at 19:33, Junio C Hamano  wrote:

I do not think this topic is specific to use of submodules.  If you
want to encourage your engineers to fetch from nearby mirrors you
maintain, you would want a forest of url.mine.insteadof=theirs for
the external repositories that matter to you specified by
everybody's $HOME/.gitconfig, and one way to do so would be to have
them use the configuration inclusion.  An item in your engineer
orientation material could tell them to add

   [include]
   path = /usr/local/etc/git/mycompany.urlrewrite

when they set up their "[user] name/email" in there.

And you can update /usr/local/etc/git/mycompany.urlrewrite as
needed.

Oh nice, I didn't know about "include". However, as mentioned to Stefan in this 
thread, I fear that our engineers will miss that. I would prefer a solution that does not 
need any additional setup. Therefore the suggestion to add rewrites in the .gitmodules 
file.


How do you distribute new copies of Git to your engineers?
Maybe you could ship them a version which has the "include" line
already builtin as default? So your distributed copy of Git
would not just check the default places for configs, but also
some complied in /net/share/mycompany.gitconfig


Which is just what we do at $DAYJOB, that way you can easily
distribute all kinds of settings, customizations and hooks
company-wide.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator

[Junio C Hamano]

Matthieu Moy  writes:

> Johannes Schindelin  writes:
>
>> This is the correct thing to do, really: we already specify LF as
>> field separator.
>
> I'm almost convinced that this is the right thing to do in the long run
> ("almost" because I'm not sure, not because I have arguments against). I
> agree with Junio that the commit message should be more convincing, but
> indeed, accepting LF and not CR is strange.

If there were a single character that denotes CRLF, I'd say that
including such a character in IFS would make sense on a system with
CRLF EOL convention.  But that is not the case.

On a platform with LF EOL convention, having LF in IFS makes sense,
due to two reasons.

 * read is not the only user of IFS.  Expressing "list of things"
   (pre bashism "shell array" days) by concatenating elements into a
   single string variable, separated with LF, and later iterating
   over them is a very common use case, e.g.

LF='
'
list="$thing1"
list="$list$LF$thing2"
list="$list$LF$thing3"
...

IFS=$LF
for thing in $list
do
...

   And including LF by default in IFS, especially when "things" can
   contain SP/HT, is handy.

 * It does not hurt on a platform with LF EOL convention to include
   LF in IFS, because you cannot have a "lone LF" in the middle of a
   line.  Presence of a single LF alone will terminate the current
   line and the bytes that follow it will be a next line.

No similarity argument can be made for a lone CR on a platform that
uses CRLF EOL convention.  A "lone CR" can appear in the middle of a
line without terminating the current line, as only a CR that is
immediately followed by a LF is the end of line, so you cannot make
the "It does not hurt" argument for including CR to IFS.  If you
have a variable in which "A^MB" is there, "set $variable" would
split them into two and assign B to $2, which is not what the
scripts would expect.

> However, is this the right thing to do in the maintainance branch? It
> does fix the issue, but does so in a rather intrusive way, so I'd need
> more arguments to be convinced that this is safe to merge in maint.

If this were the "right" thing in general for shell scripts on
systems with CRLF EOL convention, the implementation of the shell
language on such a platform would be doing that upon startup (or
upon "unset IFS"), and we wouldn't be having this discussion.  Don't
you think it is strange that individual applications like Git have
to set IFS to include CR?  I see it as a strong sign that this is
not a correct thing to do.

Intrusive does not begin to describe it.

I think the "right" thing for a shell implementation on a CRLF
platform to do is twofold.

 * Read
   http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html
   and notice that the operation is defined as a two-step process.
   It is supposed to read an input line and then "The terminating
(if any) shall be removed from the input".  And then
   "the result shall be split into fields" (at IFS boundaries).

   As POSIX does not say  has to be a single octet with
   value 10 (the same way you are allowed to show CRLF when you call
   printf("\n")), a shell implementation would read CRLF terminated
   lines, and remove the terminating CRLF before doing the field
   splitting using IFS.

   This is why I said in my message to Dscho that IFS does not get
   into the picture.

 * Implement the "field splitting" (at IFS boundaries) with a small
   twist.  When the script/user included '\n' in IFS, split at LF
   but remove CR if there is one that immediately precedes the LF.

The latter would help if you did the previous "read is not the only
user of IFS" example like this instead:

LF=$(printf '\n.')
LF=${LF%.}
list="$thing1"
list="$list$LF$thing2"
list="$list$LF$thing3"
...

IFS=$LF
for thing in $list
do
...

and the platform 'printf' gave CRLF for "\n" (newline), resulting
the list to be concatenated with CRLF, not LF.

And this is why I asked Dscho if the shell is done correctly _for_
the platform.

> Sorry for being negative, and especially sorry since I'm partly guilty
> for the breakage. I just want to be sure that we don't break anything
> while repairing it (we already introduced this breakage while repairing
> another one...).

Something along the line of the following would be tolerable, even
though I think in the longer term, not just in Git land but in the
larger ecosystem to use POSIXy tools on Windows, the best solution
is to fix the shell so that it matches the expectation of the users
of its platform.

I say "something along the line of" here because I do not know how
the problematic shell behaves on the assignment command that stuffs
a lone CR into a variable.  It _might_ need a similar protection
against the shell feature "the last 

[PATCH 2/2] http: use credential API to handle proxy authentication

[Knut Franke]

Currently, the only way to pass proxy credentials to curl is by including them
in the proxy URL. Usually, this means they will end up on disk unencrypted, one
way or another (by inclusion in ~/.gitconfig, shell profile or history). Since
proxy authentication often uses a domain user, credentials can be security
sensitive; therefore, a safer way of passing credentials is desirable.

If the configured proxy contains a username but not a password, query the
credential API for one. Also, make sure we approve/reject proxy credentials
properly.

For consistency reasons, add parsing of http_proxy/https_proxy/all_proxy
environment variables, which would otherwise be evaluated as a fallback by curl.
Without this, we would have different semantics for git configuration and
environment variables.

Signed-off-by: Knut Franke 
---
 http.c | 63 ++-
 http.h |  1 +
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/http.c b/http.c
index f7366d0..6e20017 100644
--- a/http.c
+++ b/http.c
@@ -64,6 +64,7 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *http_proxy_authmethod = NULL;
+struct credential http_proxy_auth = CREDENTIAL_INIT;
 static const char *curl_cookie_file;
 static int curl_save_cookies;
 struct credential http_auth = CREDENTIAL_INIT;
@@ -161,6 +162,9 @@ static void finish_active_slot(struct active_request_slot 
*slot)
 #else
slot->results->auth_avail = 0;
 #endif
+
+   curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
+   >results->http_connectcode);
}
 
/* Run callback if appropriate */
@@ -318,6 +322,25 @@ static void set_from_env(const char **var, const char 
*envname)
 
 static void init_curl_proxy_auth(CURL *result)
 {
+   if (http_proxy_auth.username) {
+   if (!http_proxy_auth.password) {
+   credential_fill(_proxy_auth);
+   }
+#if LIBCURL_VERSION_NUM >= 0x071301
+   curl_easy_setopt(result, CURLOPT_PROXYUSERNAME,
+   http_proxy_auth.username);
+   curl_easy_setopt(result, CURLOPT_PROXYPASSWORD,
+   http_proxy_auth.password);
+#else
+   struct strbuf up = STRBUF_INIT;
+   strbuf_reset();
+   strbuf_addstr_urlencode(, http_proxy_auth.username, 1);
+   strbuf_addch(, ':');
+   strbuf_addstr_urlencode(, http_proxy_auth.password, 1);
+   curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, up.buf);
+#endif
+   }
+
 set_from_env(_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
 
if (http_proxy_authmethod) {
@@ -505,8 +528,36 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
 #endif
 
+   /*
+* curl also examines these variables as a fallback; but we need to 
query
+* them here in order to decide whether to prompt for missing password 
(cf.
+* init_curl_proxy_auth()).
+*/
+   if (!curl_http_proxy) {
+   if (!strcmp(http_auth.protocol, "https")) {
+   set_from_env(_http_proxy, "HTTPS_PROXY");
+   set_from_env(_http_proxy, "https_proxy");
+   } else {
+   set_from_env(_http_proxy, "http_proxy");
+   }
+   if (!curl_http_proxy) {
+   set_from_env(_http_proxy, "ALL_PROXY");
+   set_from_env(_http_proxy, "all_proxy");
+   }
+   }
+
if (curl_http_proxy) {
-   curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+   if (strstr(curl_http_proxy, "://"))
+   credential_from_url(_proxy_auth, curl_http_proxy);
+   else {
+   struct strbuf url = STRBUF_INIT;
+   strbuf_reset();
+   strbuf_addstr(, "http://;);
+   strbuf_addstr(, curl_http_proxy);
+   credential_from_url(_proxy_auth, url.buf);
+   }
+
+   curl_easy_setopt(result, CURLOPT_PROXY, http_proxy_auth.host);
}
init_curl_proxy_auth(result);
 
@@ -643,6 +694,12 @@ void http_cleanup(void)
curl_http_proxy = NULL;
}
 
+   if (http_proxy_auth.password) {
+   memset(http_proxy_auth.password, 0, 
strlen(http_proxy_auth.password));
+   free(http_proxy_auth.password);
+   http_proxy_auth.password = NULL;
+   }
+
if (http_proxy_authmethod) {
free((void *)http_proxy_authmethod);
http_proxy_authmethod = NULL;
@@ -976,6 +1033,8 @@ static int handle_curl_result(struct slot_results *results)
 
if (results->curl_result == CURLE_OK) {

[PATCH 1/2] http: allow selection of proxy authentication method

[Knut Franke]

CURLAUTH_ANY does not work with proxies which answer unauthenticated requests
with a 307 redirect to an error page instead of a 407 listing supported
authentication methods. Therefore, allow the authentication method to be set
using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or configuration
variables http.proxy-authmethod and remote..proxy-authmethod (in analogy
to http.proxy and remote..proxy).

The following values are supported:

* anyauth (default)
* basic
* digest
* negotiate
* ntlm

Signed-off-by: Knut Franke 
---
 Documentation/config.txt | 28 ++
 http.c   | 61 
 remote.c |  3 +++
 remote.h |  1 +
 4 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..9dff88d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1597,6 +1597,29 @@ http.proxy::
`curl(1)`).  This can be overridden on a per-remote basis; see
remote..proxy
 
+http.proxy-authmethod::
+   Set the method with which to authenticate against the HTTP proxy. This 
only
+takes effect if the configured proxy URI contains a user name part (i.e. is
+of the form 'user@host' or 'user@host:port'). This can be overridden on a
+per-remote basis; see `remote..proxy-authmethod`. Both can be
+overridden by the 'GIT_HTTP_PROXY_AUTHMETHOD' environment variable.
+Possible values are:
++
+--
+* `anyauth` - Automatically pick a suitable authentication method. It is
+  assumed that the proxy answers an unauthenticated request with a 407
+  status code and one or more Proxy-authenticate headers with supported
+  authentication methods. This is the default.
+* `basic` - HTTP Basic authentication
+* `digest` - HTTP Digest authentication; this prevents the password from being
+  transmitted to the proxy in clear text
+* `negotiate` - GSS-Negotiate authentication (compare the --negotiate option
+  of `curl(1)`)
+* `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
+--
++
+
+
 http.cookieFile::
File containing previously stored cookie lines which should be used
in the Git http session, if they match the server. The file format
@@ -2390,6 +2413,11 @@ remote..proxy::
the proxy to use for that remote.  Set to the empty string to
disable proxying for that remote.
 
+remote..proxy-authmethod::
+For remotes that require curl (http, https and ftp), the method to use for
+authenticating against the proxy in use (probably set in
+`remote..proxy`). See `http.proxy-authmethod`.
+
 remote..fetch::
The default set of "refspec" for linkgit:git-fetch[1]. See
linkgit:git-fetch[1].
diff --git a/http.c b/http.c
index 7da76ed..f7366d0 100644
--- a/http.c
+++ b/http.c
@@ -63,6 +63,7 @@ static long curl_low_speed_limit = -1;
 static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
+static const char *http_proxy_authmethod = NULL;
 static const char *curl_cookie_file;
 static int curl_save_cookies;
 struct credential http_auth = CREDENTIAL_INIT;
@@ -257,6 +258,9 @@ static int http_options(const char *var, const char *value, 
void *cb)
if (!strcmp("http.proxy", var))
return git_config_string(_http_proxy, var, value);
 
+   if (!strcmp("http.proxy-authmethod", var))
+   return git_config_string(_proxy_authmethod, var, value);
+
if (!strcmp("http.cookiefile", var))
return git_config_string(_cookie_file, var, value);
if (!strcmp("http.savecookies", var)) {
@@ -305,6 +309,43 @@ static void init_curl_http_auth(CURL *result)
 #endif
 }
 
+static void set_from_env(const char **var, const char *envname)
+{
+   const char *val = getenv(envname);
+   if (val)
+   *var = val;
+}
+
+static void init_curl_proxy_auth(CURL *result)
+{
+set_from_env(_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
+
+   if (http_proxy_authmethod) {
+   if (!strcmp(http_proxy_authmethod, "basic"))
+   curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
CURLAUTH_BASIC);
+   else if (!strcmp(http_proxy_authmethod, "digest"))
+   curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
CURLAUTH_DIGEST);
+   else if (!strcmp(http_proxy_authmethod, "negotiate"))
+   curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
CURLAUTH_GSSNEGOTIATE);
+   else if (!strcmp(http_proxy_authmethod, "ntlm"))
+   curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
CURLAUTH_NTLM);
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+   else if (!strcmp(http_proxy_authmethod, "anyauth"))
+   curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
CURLAUTH_ANY);
+#endif
+   // CURLAUTH_DIGEST_IE has no corresponding