[PATCH v4] ref-filter: fallback on alphabetical comparison

[Karthik Nayak]

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 listing refs with equal values cannot be pre-determined) as
pointed out by Johannes Sixt ($gmane/280117).

Hence, fallback to alphabetical comparison based on the refname
whenever the other criterion is equal.

A test in t3203 was expecting that branch-two sorts before HEAD, which
happened to be how qsort(3) on Linux sorted the array, but (1) that
outcome was not even guaranteed, and (2) once we start breaking ties
with the refname, "HEAD" should sort before "branch-two" so the
original expectation was inconsistent with the criterion we now use.

Update it to match the new world order, which we can now depend on
being stable.

Helped-by: Junio C Hamano 
Reported-by: Johannes Sixt 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 2 +-
 t/t3203-branch-output.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 046e73b..7b33cb8 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1698,7 +1698,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct 
ref_array_item *a, stru
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
-   cmp = 0;
+   cmp = strcmp(a->refname, b->refname);
else
cmp = 1;
}
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index f77971c..9f2d482 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -158,8 +158,8 @@ EOF
 
 test_expect_success 'git branch `--sort` option' '
cat >expect <<-\EOF &&
- branch-two
* (HEAD detached from fromtag)
+ branch-two
  branch-one
  master
EOF
-- 
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] difftool: avoid symlinks when reusing worktree files

[David Aguilar]

On Thu, Oct 29, 2015 at 11:19:01AM -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > So I think it is fine to return $use=0 for any symbolic link from
> > use_wt_file.  Anything you do there will be replaced by the loop
> > over %symlink that appears later in the caller.  The caller discards
> > $wt_sha1 when $use=0 is returned, so the second return value does
> > not matter.
> 
> So let me try to update your patch with the result of the study of
> the codeflow.
> 
> -- >8 --
> From: David Aguilar 
> Subject: difftool: ignore symbolic links in use_wt_file
> 
> The caller is preparing a narrowed-down copy of the working tree and
> this function is asked if the path should be included in that copy.
> If we say yes, the path from the working tree will be either symlinked
> or copied into the narrowed-down copy.
> 
> For any path that is a symbolic link, the caller later fixes up the
> narrowed-down copy by unlinking the path and replacing it with a
> regular file it writes out that mimics the way how "git diff"
> compares symbolic links.
> 
> Let's answer "no, you do not want to copy/symlink the working tree
> file" for all symbolic links from this function, as we know the
> result will not be used because it will be overwritten anyway.
> 
> Incidentally, this also stops the function from feeding a symbolic
> link in the working tree to hash-object, which is a wrong thing to
> do to begin with. The link may be pointing at a directory, or worse
> may be dangling (both would be noticed as an error).  Even if the
> link points at a regular file, hashing the contents of a file that
> is pointed at by the link is not correct (Git hashes the contents of
> the link itself, not the pointee).
> 
> Signed-off-by: David Aguilar 
> Signed-off-by: Junio C Hamano 
> ---

This is a very nicely worded commit message.  Thanks for the
thorough explanation.


>  git-difftool.perl   |  4 +---
>  t/t7800-difftool.sh | 19 +++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/git-difftool.perl b/git-difftool.perl
> index 7df7c8a..488d14b 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -70,9 +70,7 @@ sub use_wt_file
>   my ($repo, $workdir, $file, $sha1) = @_;
>   my $null_sha1 = '0' x 40;
>  
> - if (! -e "$workdir/$file") {
> - # If the file doesn't exist in the working tree, we cannot
> - # use it.
> + if (-l "$workdir/$file" || ! -e _) {
>   return (0, $null_sha1);
>   }

The "-e _" shorthand caught my eye ~ I didn't know perl could do that!
Nice.

Underline is barely mentioned in perlvar, but it's obvious what
(I think) it means, and since Perl is DWIM, it must be right. ;-)
-- 
David
--
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]

Jeff King wrote:

> Somebody suggested elsewhere that the name "gdb" be configurable. We
> could stick that in the same variable, like:
>
>   test "$GIT_TEST_GDB" = 1 && GIT_TEST_GDB=gdb
>   exec ${GIT_TEST_GDB} --args ...
>
> but that does not play well with the "debug" function, which does not
> know which value to set it to. I guess we would need GIT_TEST_GDB_PATH
> or something.

*nod* I think having a separate variable like GIT_TEST_GDB_COMMAND
would be fine.

An interested person could also replace 'gdb --args' with 'lldb --' if
they want to.
--
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] ref-filter: fallback on alphabetical comparison

[Johannes Sixt]

Am 30.10.2015 um 09:45 schrieb Karthik Nayak:

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 listing refs with equal values cannot be pre-determined) as
pointed out by Johannes Sixt ($gmane/280117).

Hence, fallback to alphabetical comparison based on the refname
whenever the other criterion is equal.

A test in t3203 was expecting that branch-two sorts before HEAD, which
happened to be how qsort(3) on Linux sorted the array, but (1) that
outcome was not even guaranteed, and (2) once we start breaking ties
with the refname, "HEAD" should sort before "branch-two" so the
original expectation was inconsistent with the criterion we now use.

Update it to match the new world order, which we can now depend on
being stable.


Needless to say that the patch fixes the test failure on Windows. (I 
tested v2 of the patch.)


-- Hannes

--
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] test: accept death by SIGPIPE as a valid failure mode

[Junio C Hamano]

On a local host, the object/history transport code often talks over
pipe with the other side.  The other side may notice some (expected)
failure, send the error message either to our process or to the
standard error and hung up.  In such codepaths, if timing were not
unfortunate, our side would receive the report of (expected) failure
from the other side over the pipe and die().  Otherwise, our side
may still be trying to talk to it and would die with a SIGPIPE.

This was observed as an intermittent breakage in t5516 by a few
people.

In the real-life scenario, either mode of death exits with a
non-zero status, and the user would learn that the command failed.
The test_must_fail helper should also know that dying with SIGPIPE
is one of the valid failure modes when we are expecting the tested
operation to notice problem and fail.

Signed-off-by: Junio C Hamano 
---

 * This time with a real commit log message.  As the goal is to
   treat death-by-sigpipe just like a call to die(), this version
   silently lets the test pass instead of giving a message to the
   standard error stream.

 t/test-lib-functions.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8f8858a..4f40ffe 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -556,6 +556,9 @@ test_must_fail () {
if test $exit_code = 0; then
echo >&2 "test_must_fail: command succeeded: $*"
return 1
+   elif test $exit_code -eq 141; then
+   # died with sigpipe
+   return 0
elif test $exit_code -gt 129 && test $exit_code -le 192; then
echo >&2 "test_must_fail: died by signal: $*"
return 1
-- 
2.6.2-456-gc12a6fe

--
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: [Savannah-users] Anonymous commit (empty Author and Committer)

[Kaz Kylheku]

On 30.10.2015 13:59, Lennart Sorensen wrote:

I don't have non-fast-forward rights. Does someone from savannah-users
have them? Could he just delete this commit?


If you do that, then anyone that already did a pull after it went in
will have a broken tree.  Rather annoying.


Nope. They will have a git in which that commit looks like their own
local work. *Someone* will inadvertently do a "git push" to blast out
their changes based on that deleted commit, thereby causing it to
reappear.

:)
--
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] Limit the size of the data block passed to SHA1_Update()

[Junio C Hamano]

Atousa Pahlevan Duprat  writes:

> Some implementations of SHA_Updates have inherent limits
> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
> to set the max chunk size supported, if required.  This is
> enabled for OSX CommonCrypto library and set to 1GiB.
> ---

Missing sign-off.

>  Makefile |  9 +
>  cache.h  |  7 ++-
>  compat/apple-common-crypto.h |  4 
>  compat/sha1_chunked.c| 20 
>  4 files changed, 39 insertions(+), 1 deletion(-)
>  create mode 100644 compat/sha1_chunked.c
>
> diff --git a/Makefile b/Makefile
> index 04c2231..5955542 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -141,6 +141,10 @@ all::
>  # Define PPC_SHA1 environment variable when running make to make use of
>  # a bundled SHA1 routine optimized for PowerPC.
>  #
> +# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
> +# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
> +# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
> +#
>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl 
> (Darwin).
>  #
>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto 
> (Darwin).
> @@ -1346,6 +1350,7 @@ else
>  ifdef APPLE_COMMON_CRYPTO
>   COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>   SHA1_HEADER = 
> + SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
>  else
>   SHA1_HEADER = 
>   EXTLIBS += $(LIB_4_CRYPTO)
> @@ -1353,6 +1358,10 @@ endif
>  endif
>  endif
>  
> +ifdef SHA1_MAX_BLOCK_SIZE
> + LIB_OBJS += compat/sha1_chunked.o
> + BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
> +endif
>  ifdef NO_PERL_MAKEMAKER
>   export NO_PERL_MAKEMAKER
>  endif
> diff --git a/cache.h b/cache.h
> index 79066e5..ec84b16 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -14,7 +14,12 @@
>  #ifndef git_SHA_CTX
>  #define git_SHA_CTX  SHA_CTX
>  #define git_SHA1_InitSHA1_Init
> -#define git_SHA1_Update  SHA1_Update
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
> +#define git_SHA1_Update SHA1_Update_Chunked
> +#else
> +#define git_SHA1_Update SHA1_Update
> +#endif
>  #define git_SHA1_Final   SHA1_Final
>  #endif
>  
> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
> index c8b9b0e..83668fd 100644
> --- a/compat/apple-common-crypto.h
> +++ b/compat/apple-common-crypto.h
> @@ -16,6 +16,10 @@
>  #undef TYPE_BOOL
>  #endif
>  
> +#ifndef SHA1_MAX_BLOCK_SIZE
> +#error "Using Apple Common Crypto library requires setting 
> SHA1_MAX_BLOCK_SIZE"
> +#endif
> +

It crossed my mind if this might be better to just define it to some
reasonable value instead of erroring out, but because we do give a
default value in the Makefile, it would be a sign that the user is
doing something _quite_ unusual if the symbol is not defined here,
so I agree with your decision to error it out here.

>  #ifdef APPLE_LION_OR_NEWER
>  #define git_CC_error_check(pattern, err) \
>   do { \
> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
> new file mode 100644
> index 000..4a8e4f7
> --- /dev/null
> +++ b/compat/sha1_chunked.c
> @@ -0,0 +1,20 @@
> +#include "cache.h"
> +
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
> +{
> + size_t nr;
> + size_t total = 0;
> + char *cdata = (char*)data;

Please have a single blank line between the decls at the beginning
of a function and its first statement.  I am not sure about the cast
here, though.  Doesn't the function SHA1_Update() you are going to
call in the body of the loop take "const void *" as its second
parameter?  That's how openssl/sha1.h and block-sha1/sha1.h declare
this function.

> + while(len > 0) {
> + nr = len;
> + if(nr > SHA1_MAX_BLOCK_SIZE)

Please have a SP around () for control statements, like while,
switch and if.

> + nr = SHA1_MAX_BLOCK_SIZE;
> + SHA1_Update(c, cdata, nr);
> + total += nr;
> + cdata += nr;
> + len -= nr;
> + }
> + return total;
> +}
> +#endif

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: [off topic] xkcd

[Jacob Keller]

On Fri, Oct 30, 2015 at 12:33 PM, Duy Nguyen  wrote:
> We are on xkcd! We're famous now! /me goes hide
>
> https://xkcd.com/1597/
> --
> Duy
> --

I wonder how many of us are the phone number in git.txt ;)
--
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: Anonymous commit (empty Author and Committer)

[Lennart Sorensen]

On Fri, Oct 30, 2015 at 09:19:19PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> On 30.10.2015 21:09, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >> On 30.10.2015 15:26, Andrei Borzenkov wrote:
> >>> See
> >>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> >>>
> >>>
> >>> I was not even aware that this is possible. Is there anything on server
> >>> side that can prevent it?
> >>>
> >>> Would be good if commit were amended and force pushed to fix it.
> >>>
> >> It is a bug in SGit. I'll investigate how it happened
> 
> I don't have non-fast-forward rights. Does someone from savannah-users
> have them? Could he just delete this commit?

If you do that, then anyone that already did a pull after it went in
will have a broken tree.  Rather annoying.

-- 
Len Sorensen
--
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]

Lukas Fleischer  writes:

> 1. There does not seem to be a way to pass configuration parameters to
>git-shell commands. Right now, the only way to work around this seems
>to write a wrapper script around git-shell that catches
>git-receive-pack commands and executes something like
>
>git -c receive.hideRefs=[...] receive-pack [...]
>
>instead of forwarding those commands to git-shell.

This part we have never discussed in the thread, I think.  Why do
you need to override, instead of having these in the repository's
config files?

Is it because a repository may host multiple pseudo repositories in
the form of "namespaces" but they must share the same config file,
and you would want to customize per "namespace"?

For that we may want to enhance the [include] mechanism.  Something
like

[include "namespace=foo"]
path = /path/to/foo/specific/config.txt

[include "namespace=bar"]
path = /path/to/bar/specific/config.txt

Cc'ing Peff as we have discussed this kind of conditional inclusion
in the past...
--
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

[Junio C Hamano]

Jeff King  writes:

> At the risk of repeating what I just said elsewhere in the thread, I
> think this patch is the best of the proposed solutions.

OK, will queue.  I agree that more could be built on top, instead of
polishing this further in place.

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

[PATCH] Limit the size of the data block passed to SHA1_Update()

[Atousa Pahlevan Duprat]

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.
---
 Makefile |  9 +
 cache.h  |  7 ++-
 compat/apple-common-crypto.h |  4 
 compat/sha1_chunked.c| 20 
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..5955542 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1346,6 +1350,7 @@ else
 ifdef APPLE_COMMON_CRYPTO
COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
SHA1_HEADER = 
+   SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
SHA1_HEADER = 
EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1358,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+   LIB_OBJS += compat/sha1_chunked.o
+   BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index 79066e5..ec84b16 100644
--- a/cache.h
+++ b/cache.h
@@ -14,7 +14,12 @@
 #ifndef git_SHA_CTX
 #define git_SHA_CTXSHA_CTX
 #define git_SHA1_Init  SHA1_Init
-#define git_SHA1_UpdateSHA1_Update
+#ifdef SHA1_MAX_BLOCK_SIZE
+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
+#define git_SHA1_Update SHA1_Update_Chunked
+#else
+#define git_SHA1_Update SHA1_Update
+#endif
 #define git_SHA1_Final SHA1_Final
 #endif
 
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..83668fd 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error "Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE"
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 000..4a8e4f7
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,20 @@
+#include "cache.h"
+
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
+{
+   size_t nr;
+   size_t total = 0;
+   char *cdata = (char*)data;
+   while(len > 0) {
+   nr = len;
+   if(nr > SHA1_MAX_BLOCK_SIZE)
+   nr = SHA1_MAX_BLOCK_SIZE;
+   SHA1_Update(c, cdata, nr);
+   total += nr;
+   cdata += nr;
+   len -= nr;
+   }
+   return total;
+}
+#endif
-- 
2.4.9 (Apple Git-60)

--
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] ref-filter: fallback on alphabetical comparison

[Junio C Hamano]

Johannes Sixt  writes:

>> Update it to match the new world order, which we can now depend on
>> being stable.
>
> Needless to say that the patch fixes the test failure on Windows. (I
> tested v2 of the patch.)

Thanks, both.  Queued.
--
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 v2] http.c: use CURLOPT_RANGE for range requests

[David Turner]

A HTTP server is permitted to return a non-range response to a HTTP
range request (and Apache httpd in fact does this in some cases).
While libcurl knows how to correctly handle this (by skipping bytes
before and after the requested range), it only turns on this handling
if it is aware that a range request is being made.  By manually
setting the range header instead of using CURLOPT_RANGE, we were
hiding the fact that this was a range request from libcurl.  This
could cause corruption.

Signed-off-by: David Turner 
---

This version applies on top of pu.  It also catches all of the range
requests instead of just the one that I happened to notice.

---
 http.c | 24 
 http.h |  1 -
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/http.c b/http.c
index 6b89dea..16610b9 100644
--- a/http.c
+++ b/http.c
@@ -30,7 +30,7 @@ static CURL *curl_default;
 #endif
 
 #define PREV_BUF_SIZE 4096
-#define RANGE_HEADER_SIZE 30
+#define RANGE_HEADER_SIZE 17
 
 char curl_errorstr[CURL_ERROR_SIZE];
 
@@ -1213,8 +1213,9 @@ static int http_request(const char *url,
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
 fwrite);
if (posn > 0) {
-   strbuf_addf(, "Range: bytes=%ld-", posn);
-   headers = curl_slist_append(headers, buf.buf);
+   strbuf_addf(, "%ld-", posn);
+   curl_easy_setopt(slot->curl, CURLOPT_RANGE,
+);
strbuf_reset();
}
} else
@@ -1526,10 +1527,6 @@ void release_http_pack_request(struct http_pack_request 
*preq)
fclose(preq->packfile);
preq->packfile = NULL;
}
-   if (preq->range_header != NULL) {
-   curl_slist_free_all(preq->range_header);
-   preq->range_header = NULL;
-   }
preq->slot = NULL;
free(preq->url);
free(preq);
@@ -1631,10 +1628,8 @@ struct http_pack_request *new_http_pack_request(
fprintf(stderr,
"Resuming fetch of pack %s at byte %ld\n",
sha1_to_hex(target->sha1), prev_posn);
-   xsnprintf(range, sizeof(range), "Range: bytes=%ld-", prev_posn);
-   preq->range_header = curl_slist_append(NULL, range);
-   curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER,
-   preq->range_header);
+   xsnprintf(range, sizeof(range), "%ld-", prev_posn);
+   curl_easy_setopt(preq->slot->curl, CURLOPT_RANGE, range);
}
 
return preq;
@@ -1685,7 +1680,6 @@ struct http_object_request *new_http_object_request(const 
char *base_url,
ssize_t prev_read = 0;
long prev_posn = 0;
char range[RANGE_HEADER_SIZE];
-   struct curl_slist *range_header = NULL;
struct http_object_request *freq;
 
freq = xcalloc(1, sizeof(*freq));
@@ -1791,10 +1785,8 @@ struct http_object_request 
*new_http_object_request(const char *base_url,
fprintf(stderr,
"Resuming fetch of object %s at byte %ld\n",
hex, prev_posn);
-   xsnprintf(range, sizeof(range), "Range: bytes=%ld-", prev_posn);
-   range_header = curl_slist_append(range_header, range);
-   curl_easy_setopt(freq->slot->curl,
-CURLOPT_HTTPHEADER, range_header);
+   xsnprintf(range, sizeof(range), "%ld-", prev_posn);
+   curl_easy_setopt(freq->slot->curl, CURLOPT_RANGE, range);
}
 
return freq;
diff --git a/http.h b/http.h
index 49afe39..4f97b60 100644
--- a/http.h
+++ b/http.h
@@ -190,7 +190,6 @@ struct http_pack_request {
struct packed_git **lst;
FILE *packfile;
char tmpfile[PATH_MAX];
-   struct curl_slist *range_header;
struct active_request_slot *slot;
 };
 
-- 
2.4.2.691.g714732c-twtrsrc

--
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] http.c: use CURLOPT_RANGE for range requests

[Junio C Hamano]

David Turner  writes:

> A HTTP server is permitted to return a non-range response to a HTTP
> range request (and Apache httpd in fact does this in some cases).
> While libcurl knows how to correctly handle this (by skipping bytes
> before and after the requested range), it only turns on this handling
> if it is aware that a range request is being made.  By manually
> setting the range header instead of using CURLOPT_RANGE, we were
> hiding the fact that this was a range request from libcurl.  This
> could cause corruption.

Wow, that looks really bad.

> Signed-off-by: David Turner 
> ---
>
> This version applies on top of pu.  It also catches all of the range
> requests instead of just the one that I happened to notice.
>
> ---
>  http.c | 24 
>  http.h |  1 -
>  2 files changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/http.c b/http.c
> index 6b89dea..16610b9 100644
> --- a/http.c
> +++ b/http.c
> @@ -30,7 +30,7 @@ static CURL *curl_default;
>  #endif
>  
>  #define PREV_BUF_SIZE 4096
> -#define RANGE_HEADER_SIZE 30
> +#define RANGE_HEADER_SIZE 17

Was this change necessary and is 17-byte string sufficient for
64-bit longs?

> @@ -1213,8 +1213,9 @@ static int http_request(const char *url,
>   curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
>fwrite);
>   if (posn > 0) {
> - strbuf_addf(, "Range: bytes=%ld-", posn);
> - headers = curl_slist_append(headers, buf.buf);
> + strbuf_addf(, "%ld-", posn);
> + curl_easy_setopt(slot->curl, CURLOPT_RANGE,
> +  );
>   strbuf_reset();

Makes me wonder if all the callers have a CURL* and a long so that a
new helper range_opt_ask_remainder(CURL *, long) can make the
resulting code even simpler; we only say "we know what comes before
this position, give us everything from there" in all callers, right?
--
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

[Jeff King]

On Fri, Oct 30, 2015 at 02:31:28PM -0700, Junio C Hamano wrote:

> Lukas Fleischer  writes:
> 
> > 1. There does not seem to be a way to pass configuration parameters to
> >git-shell commands. Right now, the only way to work around this seems
> >to write a wrapper script around git-shell that catches
> >git-receive-pack commands and executes something like
> >
> >git -c receive.hideRefs=[...] receive-pack [...]
> >
> >instead of forwarding those commands to git-shell.
> 
> This part we have never discussed in the thread, I think.  Why do
> you need to override, instead of having these in the repository's
> config files?
> 
> Is it because a repository may host multiple pseudo repositories in
> the form of "namespaces" but they must share the same config file,
> and you would want to customize per "namespace"?
> 
> For that we may want to enhance the [include] mechanism.  Something
> like
> 
>   [include "namespace=foo"]
>   path = /path/to/foo/specific/config.txt
> 
>   [include "namespace=bar"]
>   path = /path/to/bar/specific/config.txt
> 
> Cc'ing Peff as we have discussed this kind of conditional inclusion
> in the past...

Yeah, that sort of conditional matching is exactly what I had intended
for the "subsection" of include to be. We just haven't come up with a
good condition to act as our first use case. :)

I am happy with any syntax that does not paint us into a corner (and
your example above looks fine, assuming we could later add other keys on
the left-hand of the "=").

I am slightly confused, though, where the namespace is set in such a
git-shell example. I have no really used ref namespaces myself, but my
understanding is that they have to come from the environment. You can
similarly set config through the environment. I don't think we've ever
publicized that, but it is how "git -c" works. E.g.:

  $ git -c alias.foo='!env' -c another.option=true foo | grep GIT_
  GIT_CONFIG_PARAMETERS='alias.foo='\!'env' 'another.option=true'

I think it is very particular that you single-quote each item, though:

  $ GIT_CONFIG_PARAMETERS=foo.bar=true git config foo.bar
  error: bogus format in GIT_CONFIG_PARAMETERS
  fatal: unable to parse command-line config

  $ GIT_CONFIG_PARAMETERS="'foo.bar=true'" git config foo.bar
  true

So we may want to make it a little more friendly before truly
recommending it as an interface, but I don't think there is any
conceptual problem with doing so.

-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

What's cooking in git.git (Oct 2015, #07; Fri, 30)

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

* ar/clone-dissociate (2015-10-22) 1 commit
  (merged to 'next' on 2015-10-23 at 6bf746f)
 + clone: allow "--dissociate" without reference

 "git clone --dissociate" used to require that "--reference" was
 used at the same time, but you can create a new repository that
 borrows objects from another without using "--reference", namely
 with "clone --local" from a repository that borrows objects from
 other repositories.


* dt/name-hash-dir-entry-fix (2015-10-21) 1 commit
  (merged to 'next' on 2015-10-22 at 15eb519)
 + name-hash: don't reuse cache_entry in dir_entry

 The name-hash subsystem that is used to cope with case insensitive
 filesystems keeps track of directories and their on-filesystem
 cases for all the paths in the index by holding a pointer to a
 randomly chosen cache entry that is inside the directory (for its
 ce->ce_name component).  This pointer was not updated even when the
 cache entry was removed from the index, leading to use after free.
 This was fixed by recording the path for each directory instead of
 borrowing cache entries and restructuring the API somewhat.


* gr/rebase-i-drop-warn (2015-10-28) 2 commits
  (merged to 'next' on 2015-10-29 at 4bfda25)
 + rebase-i: work around Windows CRLF line endings
 + t3404: "rebase -i" gets broken when insn sheet uses CR/LF 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.


* jc/add-u-A-default-to-top (2015-10-24) 1 commit
  (merged to 'next' on 2015-10-29 at 15aea9c)
 + add: simplify -u/-A without pathspec

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


* jc/am-mailinfo-direct (2015-10-21) 1 commit
  (merged to 'next' on 2015-10-22 at ca15014)
 + am: make direct call to mailinfo
 (this branch is used by jc/mailinfo; uses jc/mailinfo-lib.)

 "git am" used to spawn "git mailinfo" via run_command() API once
 per each patch, but learned to make a direct call to mailinfo()
 instead.


* jc/em-dash-in-doc (2015-10-22) 1 commit
  (merged to 'next' on 2015-10-23 at 31a08ce)
 + Documentation: AsciiDoc spells em-dash as double-dashes, not triple

 AsciiDoc markup fixes.


* jc/everyday-markup (2015-10-22) 1 commit
  (merged to 'next' on 2015-10-22 at 0a2702d)
 + Documentation/everyday: match undefline with the text

 AsciiDoc markup fixes.


* jc/mailinfo-lib (2015-10-21) 34 commits
  (merged to 'next' on 2015-10-22 at 405bd66)
 + mailinfo: remove calls to exit() and die() deep in the callchain
 + mailinfo: handle charset conversion errors in the caller
 + mailinfo: libify
 + mailinfo: keep the parsed log message in a strbuf
 + mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
 + mailinfo: move content/content_top to struct mailinfo
 + mailinfo: move [ps]_hdr_data to struct mailinfo
 + mailinfo: move cmitmsg and patchfile to struct mailinfo
 + mailinfo: move charset to struct mailinfo
 + mailinfo: move transfer_encoding to struct mailinfo
 + mailinfo: move check for metainfo_charset to convert_to_utf8()
 + mailinfo: move metainfo_charset to struct mailinfo
 + mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
 + mailinfo: move add_message_id and message_id to struct mailinfo
 + mailinfo: move patch_lines to struct mailinfo
 + mailinfo: move filter/header stage to struct mailinfo
 + mailinfo: move global "FILE *fin, *fout" to struct mailinfo
 + mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
 + mailinfo: introduce "struct mailinfo" to hold globals
 + mailinfo: move global "line" into mailinfo() function
 + mailinfo: do not let find_boundary() touch global "line" directly
 + mailinfo: do not let handle_boundary() touch global "line" directly
 + mailinfo: do not let handle_body() touch global "line" directly
 + mailinfo: get rid of function-local static states
 + mailinfo: move definition of MAX_HDR_PARSED closer to its use
 + mailinfo: move cleanup_space() before its users
 + mailinfo: move check_header() after the helpers it uses
 + mailinfo: move read_one_header_line() closer to its callers
 + mailinfo: move handle_boundary() lower
 + mailinfo: plug strbuf leak during continuation line handling
 + mailinfo: 

RE: [PATCH] Limit the size of the data block passed to SHA1_Update()

[Randall S. Becker]

On October-30-15 6:18 PM, Atousa Pahlevan Duprat wrote:
>Some implementations of SHA_Updates have inherent limits on the max chunk
size. >SHA1_MAX_BLOCK_SIZE can be defined to set the max chunk size
supported, if >required.  This is enabled for OSX CommonCrypto library and
set to 1GiB.
>---
> 

Didn't we have this same issue with NonStop port about a year ago and
decided to provision this through the configure script?

Cheers,
Randall


--
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] http.c: use CURLOPT_RANGE for range requests

[David Turner]

Please disregard this; I noticed I missed a few instances.  Will reroll
shortly.


On Fri, 2015-10-30 at 18:41 -0400, David Turner wrote:
> A HTTP server is permitted to return a non-range response to a HTTP
> range request (and Apache httpd in fact does this in some cases).
> While libcurl knows how to correctly handle this (by skipping bytes
> before and after the requested range), it only turns on this handling
> if it is aware that a range request is being made.  By manually
> setting the range header instead of using CURLOPT_RANGE, we were
> hiding the fact that this was a range request from libcurl.  This
> could cause corruption.
> 
> Signed-off-by: David Turner 
> ---
>  http.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/http.c b/http.c
> index 0f924a8..303b388 100644
> --- a/http.c
> +++ b/http.c
> @@ -1202,8 +1202,9 @@ static int http_request(const char *url,
>   curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
>fwrite);
>   if (posn > 0) {
> - strbuf_addf(, "Range: bytes=%ld-", posn);
> - headers = curl_slist_append(headers, buf.buf);
> + strbuf_addf(, "%ld-", posn);
> + curl_easy_setopt(slot->curl, CURLOPT_RANGE,
> +  );
>   strbuf_reset();
>   }
>   } else


--
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] Limit the size of the data block passed to SHA1_Update()

[Atousa Pahlevan Duprat]

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.
---
 Makefile |  9 +
 cache.h  |  7 ++-
 compat/apple-common-crypto.h |  4 
 compat/sha1_chunked.c| 20 
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..5955542 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1346,6 +1350,7 @@ else
 ifdef APPLE_COMMON_CRYPTO
COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
SHA1_HEADER = 
+   SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
SHA1_HEADER = 
EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1358,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+   LIB_OBJS += compat/sha1_chunked.o
+   BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index 79066e5..ec84b16 100644
--- a/cache.h
+++ b/cache.h
@@ -14,7 +14,12 @@
 #ifndef git_SHA_CTX
 #define git_SHA_CTXSHA_CTX
 #define git_SHA1_Init  SHA1_Init
-#define git_SHA1_UpdateSHA1_Update
+#ifdef SHA1_MAX_BLOCK_SIZE
+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
+#define git_SHA1_Update SHA1_Update_Chunked
+#else
+#define git_SHA1_Update SHA1_Update
+#endif
 #define git_SHA1_Final SHA1_Final
 #endif
 
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..83668fd 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error "Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE"
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 000..4a8e4f7
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,20 @@
+#include "cache.h"
+
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
+{
+   size_t nr;
+   size_t total = 0;
+   char *cdata = (char*)data;
+   while(len > 0) {
+   nr = len;
+   if(nr > SHA1_MAX_BLOCK_SIZE)
+   nr = SHA1_MAX_BLOCK_SIZE;
+   SHA1_Update(c, cdata, nr);
+   total += nr;
+   cdata += nr;
+   len -= nr;
+   }
+   return total;
+}
+#endif
-- 
2.4.9 (Apple Git-60)

--
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] git-p4: Handle p4 submit failure

[Junio C Hamano]

Luke Diamand  writes:

> On 30/10/15 17:57, Junio C Hamano wrote:
>> Etienne Girard  writes:
>>
>>> Yes, however if `p4 submit` fails the corresponding "Command failed"
>>> error message is displayed, and the p4 error message itself is
>>> displayed if any.
>>> Tthe script will also terminate successfully if self.edit_template
>>> returns false but it will exit with error code 1 if p4 submit fails.
>>>
>>> So the user will get "Command failed: [...]" followed by "Submission
>>> cancelled, undoing p4 changes", to let him know that the script failed
>>> because of p4 and that nothing was submitted.
>>
>> OK, then it sounds like all I have to do is to update the log
>> message with the "How about this" version and correct the authorship
>> to use your murex address, and then wait for reviews from real "git
>> p4" reviewers.
>>
>
> Looks good to me. Nice use of try...finally.
>
> One very small thing - test t9807-git-p4-submit.sh is now failing with
> this change.
>
> That's because it tries to delete one of the files it created, but you
> are now deleting it already! Could you just update that please?

I'll ask Etienne include that change in a reroll, as my environment
is not equipped to run any p4 tests.

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: Anonymous commit (empty Author and Committer)

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 21:09, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> On 30.10.2015 15:26, Andrei Borzenkov wrote:
>>> See
>>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
>>>
>>>
>>> I was not even aware that this is possible. Is there anything on server
>>> side that can prevent it?
>>>
>>> Would be good if commit were amended and force pushed to fix it.
>>>
>> It is a bug in SGit. I'll investigate how it happened

I don't have non-fast-forward rights. Does someone from savannah-users
have them? Could he just delete this commit?




signature.asc
Description: OpenPGP digital signature

Re: [RFC] t5516 "75 - deny fetch unreachable SHA1, allowtipsha1inwant=true" flaky?

[Junio C Hamano]

Junio C Hamano  writes:

> That leaves us to something along this line...
>
>> (3) Add a method "test_must_fail_or_die" to
>> "test-lib-functions.sh". This method accepts exit codes 129> too. Use the new method in t5516.
>
> ... but I have to wonder if 129 that the only error we expect, other than the orderly shutdown, is
> reception of sigpipe.

So, how about doing something like this as a starter.  All of the
object transport codepath share "we may notice that the other end
disconnected, or that the other end explicitly told us it found an
error, and die, or the other end may have died, perhaps after giving
a human-readable error message, and we end up dying when we try to
tell them more with SIGPIPE", and that by itself is not a bug in the
real life---we will exit with non-zero status and that is a sign
enough for the user to know that the command has failed.



 t/test-lib-functions.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6dffb8b..b1f950d 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -579,6 +579,9 @@ test_must_fail () {
if test $exit_code = 0; then
echo >&2 "test_must_fail: command succeeded: $*"
return 1
+   elif test $exit_code -eq 141; then
+   echo >&2 "test_must_fail: died with sigpipe: $*"
+   return 0
elif test $exit_code -gt 129 && test $exit_code -le 192; then
echo >&2 "test_must_fail: died by signal: $*"
return 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: [PATCH] gitk: l10n: Update Catalan translation

[Paul Mackerras]

On Mon, Oct 05, 2015 at 10:26:12PM -0600, Alex Henrie wrote:
> The gitk included in git 2.6.0 crashes if run from a Catalan locale.
> I'm hoping that a translation update will fix this.
> 
> Signed-off-by: Alex Henrie 

Should actually be fixed by a patch from Beat Bolli that I just applied,
but I applied your patch also.

Paul.
--
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] gitk: add missing accelerators

[Paul Mackerras]

On Wed, Sep 30, 2015 at 09:50:11PM +0200, Beat Bolli wrote:
> In d99b4b0de27a ("gitk: Accelerators for the main menu", 2015-09-09),
> accelerators were added to allow efficient keyboard navigation. One
> instance of the strings "Edit view..." and "Delete view" were left
> without the ampersand.
> 
> Add the missing ampersand characters to unbreak our international
> users.
> 
> Signed-off-by: Beat Bolli 
> Cc: Paul Mackerras 

Thanks, applied.

Paul.
--
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/4] gitk crash fix and locale updates

[Paul Mackerras]

On Tue, Oct 20, 2015 at 02:33:00PM +0200, Takashi Iwai wrote:
> 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.

Thanks for the series.  The patch 1/4 is the same as a patch from Beat
Bolli that I just applied, so I left out your 1/4 and applied 2, 3,
and 4.

Paul.
--
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: [gmane.comp.version-control.git] [PATCH 0/4] gitk crash fix and locale updates

[Paul Mackerras]

On Mon, Oct 26, 2015 at 05:16:20PM -0700, Junio C Hamano wrote:
> Ping.  What do you think of these?  It appears that quite a many
> people are getting bitten by the issues this series addresses.

Yes, sorry about that.  I have applied a patch from Beat Bolli fixing
the basic issue, since his patch was the same as Takashi's first patch
and was posted earlier.  I have also applied 2-4 of Takashi's series
plus some other translation updates.  I have pushed all that out to
git://ozlabs.org/~paulus/gitk.git.

Please pull whenever is convenient for you.

Regards,
Paul.
--
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] checkout: add --progress option

[Edmundo Carmona Antoranz]

One line got wrapped around, hope that's not a problem to understand
the whole concept.

> +/*
> + * Any of these conditions will make progress output be skipped:
> + * - selected --quiet
> + * - selected --no-progress
> + * - (didn't select --progress nor --no-progress) and not working on a 
> terminal
> + */
> +static int verbose_update(const struct checkout_opts *o)
> +{
> +   return !o->quiet && o->show_progress && (o->show_progress >= 0
> || isatty(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] checkout: add --progress option

[Edmundo Carmona Antoranz]

Ok this is like the previous patch (on top of my patch) but does
post_processing right after parse_options and so there's no need for
the function I had introduced before. Hope it can get your blessings
so I can send-email the _second_ patch for this feature (following the
one that has already being merged into pu, right?).

 builtin/checkout.c | 32 +++-
1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e28c36b..9e78835 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -27,8 +27,6 @@ static const char * const checkout_usage[] = {
   NULL,
};

-static int option_progress = -1;
-
struct checkout_opts {
   int patch_mode;
   int quiet;
@@ -39,6 +37,7 @@ struct checkout_opts {
   int overwrite_ignore;
   int ignore_skipworktree;
   int ignore_other_worktrees;
+   int show_progress;

   const char *new_branch;
   const char *new_branch_force;
@@ -419,19 +418,7 @@ static int reset_tree(struct tree *tree, const
struct checkout_opts *o,
   opts.reset = 1;
   opts.merge = 1;
   opts.fn = oneway_merge;
-   /**
-* Rules to display progress:
-* -q is selected
-*  no verbiage
-* -q is _not_ selected and --no-progress _is_ selected,
-*  progress will be skipped
-* -q is _not_ selected and --progress _is_ selected,
-*  progress will be printed to stderr
-* -q is _not_ selected and --progress is 'undefined'
-*  progress will be printed to stderr _if_ working on a terminal
-*/
-   opts.verbose_update = !o->quiet && (option_progress > 0 ||
-  (option_progress < 0 && isatty(2)));
+   opts.verbose_update = o->show_progress;
   opts.src_index = _index;
   opts.dst_index = _index;
   parse_tree(tree);
@@ -515,7 +502,7 @@ static int merge_working_tree(const struct
checkout_opts *opts,
   topts.update = 1;
   topts.merge = 1;
   topts.gently = opts->merge && old->commit;
-   topts.verbose_update = !opts->quiet && isatty(2);
+   topts.verbose_update = opts->show_progress;
   topts.fn = twoway_merge;
   if (opts->overwrite_ignore) {
   topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1170,7 +1157,7 @@ int cmd_checkout(int argc, const char **argv,
const char *prefix)
   N_("second guess 'git checkout
'")),
   OPT_BOOL(0, "ignore-other-worktrees",
_other_worktrees,
N_("do not check if another worktree is
holding the given ref")),
-   OPT_BOOL(0, "progress", _progress, N_("force
progress reporting")),
+   OPT_BOOL(0, "progress", _progress, N_("force
progress reporting")),
   OPT_END(),
   };

@@ -1178,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv,
const char *prefix)
   memset(, 0, sizeof(new));
   opts.overwrite_ignore = 1;
   opts.prefix = prefix;
+   opts.show_progress = -1;

   gitmodules_config();
   git_config(git_checkout_config, );
@@ -1187,6 +1175,16 @@ int cmd_checkout(int argc, const char **argv,
const char *prefix)
   argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);

+   /*
+* Final processing of show_progress
+* Any of these 3 conditions will make progress output be skipped:
+* - selected --quiet
+* - selected --no-progress
+* - didn't select --progress and not working on a terminal
+*/
+   opts.show_progress = !opts.quiet && opts.show_progress &&
+(opts.show_progress >= 0 || isatty(2));
+
   if (conflict_style) {
   opts.merge = 1; /* implied */
   git_xmerge_config("merge.conflictstyle", conflict_style, NULL);

On Fri, Oct 30, 2015 at 6:58 PM, Edmundo Carmona Antoranz
 wrote:
> On Fri, Oct 30, 2015 at 1:37 PM, Junio C Hamano  wrote:
>>
>> Actually, using a single variable is my preference.  In this case I
>> wanted to illustrate that the value parsed by parse_options() does
>> not have to be the one that is used in the final location deep in
>> the callchain for educational purposes, and I found it clearer to
>> use two separate variables in the illustration.
>
> From all your feedback, I think I got a working example not using the
> static variable, also reworked the wording to explain in what cases
> progress will be skipped.
>
> So... on top of my patch, I think this sums it up:
>
>  builtin/checkout.c | 33 -
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e28c36b..a2e5cd3 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -27,8 +27,6 @@ 

Re: [PATCH v2] http.c: use CURLOPT_RANGE for range requests

[Jeff King]

On Fri, Oct 30, 2015 at 06:54:42PM -0400, David Turner wrote:

> A HTTP server is permitted to return a non-range response to a HTTP
> range request (and Apache httpd in fact does this in some cases).
> While libcurl knows how to correctly handle this (by skipping bytes
> before and after the requested range), it only turns on this handling
> if it is aware that a range request is being made.  By manually
> setting the range header instead of using CURLOPT_RANGE, we were
> hiding the fact that this was a range request from libcurl.  This
> could cause corruption.

The goal makes sense. Why weren't we using CURLOPT_RANGE before? Did it
not exist (or otherwise have limitations) in 2005, and if so, when did
it become usable? Do we need to protect this with an #ifdef for the curl
version?

> @@ -1213,8 +1213,9 @@ static int http_request(const char *url,
>   curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
>fwrite);
>   if (posn > 0) {
> - strbuf_addf(, "Range: bytes=%ld-", posn);
> - headers = curl_slist_append(headers, buf.buf);
> + strbuf_addf(, "%ld-", posn);
> + curl_easy_setopt(slot->curl, CURLOPT_RANGE,
> +  );
>   strbuf_reset();
>   }
>   } else

We reuse curl slots from request to request, and curl options may
persist.  The old code appended to the header list, which then gets
added to the curl object with CURLOPT_HTTPHEADER. Subsequent requests,
even if they are not using ranges, would also set CURLOPT_HTTPHEADER,
possibly to NULL, so the range header would not accidentally creep into
the next request.

But with CURLOPT_RANGE, I think the value would persist to the next
request (unless curl automagically forgets it after making the request,
but I don't think it generally does so). I think the cleanest thing
would be to reset it back to NULL in get_active_slot (there are several
other fields that we reset there, too).

> @@ -1685,7 +1680,6 @@ struct http_object_request 
> *new_http_object_request(const char *base_url,
>   ssize_t prev_read = 0;
>   long prev_posn = 0;
>   char range[RANGE_HEADER_SIZE];
> - struct curl_slist *range_header = NULL;
>   struct http_object_request *freq;

Junio asked elsewhere if we really need to tweak RANGE_HEADER_SIZE. But
I think we should go even further, and bump the definition much closer
to the point of use[1], and not worry about an arbitrary constant. After
all, we already use sizeof(), so we do not even end up repeating the
constant.

We could even hide the whole thing away with something like:

  void http_set_range(CURL *curl, long lo, long hi)
  {
char buf[128];
int len = 0;

if (lo >= 0)
len += xsnprintf(buf + len, "%ld", lo);
len += xsnprintf(buf + len, "-");
if (hi >= 0)
len += xsnprintf(buf + len, "%ld", hi);

curl_easy_setopt(curl, CURLOPT_RANGE, buf);
  }

That would also make it easier to replace if we do need to keep an
#ifdef for older versions of curl. But maybe it is just
over-engineering.

-Peff

[1] Antique versions of curl needed the buffer to remain valid through
the whole request, but these days it makes its own copy (and even
under the old regime, I am not sure if the existing code would work
anyway, as we return the request from this function, and the buffer
is function-local).
--
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] checkout: add --progress option

[Edmundo Carmona Antoranz]

On Fri, Oct 30, 2015 at 1:37 PM, Junio C Hamano  wrote:
>
> Actually, using a single variable is my preference.  In this case I
> wanted to illustrate that the value parsed by parse_options() does
> not have to be the one that is used in the final location deep in
> the callchain for educational purposes, and I found it clearer to
> use two separate variables in the illustration.

>From all your feedback, I think I got a working example not using the
static variable, also reworked the wording to explain in what cases
progress will be skipped.

So... on top of my patch, I think this sums it up:

 builtin/checkout.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e28c36b..a2e5cd3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -27,8 +27,6 @@ static const char * const checkout_usage[] = {
   NULL,
};

-static int option_progress = -1;
-
struct checkout_opts {
   int patch_mode;
   int quiet;
@@ -39,6 +37,7 @@ struct checkout_opts {
   int overwrite_ignore;
   int ignore_skipworktree;
   int ignore_other_worktrees;
+   int show_progress;

   const char *new_branch;
   const char *new_branch_force;
@@ -406,6 +405,17 @@ static void describe_detached_head(const char
*msg, struct commit *commit)
   strbuf_release();
}

+/*
+ * Any of these conditions will make progress output be skipped:
+ * - selected --quiet
+ * - selected --no-progress
+ * - (didn't select --progress nor --no-progress) and not working on a terminal
+ */
+static int verbose_update(const struct checkout_opts *o)
+{
+   return !o->quiet && o->show_progress && (o->show_progress >= 0
|| isatty(2));
+}
+
static int reset_tree(struct tree *tree, const struct checkout_opts *o,
 int worktree, int *writeout_error)
{
@@ -419,19 +429,7 @@ static int reset_tree(struct tree *tree, const
struct checkout_opts *o,
   opts.reset = 1;
   opts.merge = 1;
   opts.fn = oneway_merge;
-   /**
-* Rules to display progress:
-* -q is selected
-*  no verbiage
-* -q is _not_ selected and --no-progress _is_ selected,
-*  progress will be skipped
-* -q is _not_ selected and --progress _is_ selected,
-*  progress will be printed to stderr
-* -q is _not_ selected and --progress is 'undefined'
-*  progress will be printed to stderr _if_ working on a terminal
-*/
-   opts.verbose_update = !o->quiet && (option_progress > 0 ||
-  (option_progress < 0 && isatty(2)));
+   opts.verbose_update = verbose_update(o);
   opts.src_index = _index;
   opts.dst_index = _index;
   parse_tree(tree);
@@ -515,7 +513,7 @@ static int merge_working_tree(const struct
checkout_opts *opts,
   topts.update = 1;
   topts.merge = 1;
   topts.gently = opts->merge && old->commit;
-   topts.verbose_update = !opts->quiet && isatty(2);
+   topts.verbose_update = verbose_update(opts);
   topts.fn = twoway_merge;
   if (opts->overwrite_ignore) {
   topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1170,7 +1168,7 @@ int cmd_checkout(int argc, const char **argv,
const char *prefix)
   N_("second guess 'git checkout
'")),
   OPT_BOOL(0, "ignore-other-worktrees",
_other_worktrees,
N_("do not check if another worktree is
holding the given ref")),
-   OPT_BOOL(0, "progress", _progress, N_("force
progress reporting")),
+   OPT_BOOL(0, "progress", _progress, N_("force
progress reporting")),
   OPT_END(),
   };

@@ -1178,6 +1176,7 @@ int cmd_checkout(int argc, const char **argv,
const char *prefix)
   memset(, 0, sizeof(new));
   opts.overwrite_ignore = 1;
   opts.prefix = prefix;
+   opts.show_progress = -1;

   gitmodules_config();
   git_config(git_checkout_config, );
--
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: Anonymous commit (empty Author and Committer)

[Andrei Borzenkov]

30.10.2015 23:59, Lennart Sorensen пишет:

On Fri, Oct 30, 2015 at 09:19:19PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:

On 30.10.2015 21:09, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

On 30.10.2015 15:26, Andrei Borzenkov wrote:

See
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac


I was not even aware that this is possible. Is there anything on server
side that can prevent it?

Would be good if commit were amended and force pushed to fix it.


It is a bug in SGit. I'll investigate how it happened


I don't have non-fast-forward rights. Does someone from savannah-users
have them? Could he just delete this commit?


If you do that, then anyone that already did a pull after it went in
will have a broken tree.  Rather annoying.



If we decide to fix this commit it is better done now, while it is the 
last one. It is annoying but do you have suggestion how it can be done 
differently?

--
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: Anonymous commit (empty Author and Committer)

[Konstantin Khomoutov]

On Fri, 30 Oct 2015 17:26:00 +0300
Andrei Borzenkov  wrote:

> See 
> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> 
> I was not even aware that this is possible. Is there anything on
> server side that can prevent it?

A hook running on "update" event could check the commits being pushed
and reject the update if some commit among those does not pass the
necessary checks.

Please see the githooks(5) manual page.
--
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] ident.c: add support for IPv6

[Elia Pinto]

Add IPv6 support by implementing name resolution with the
protocol agnostic getaddrinfo(3) API. The old gethostbyname(3)
code is still available when git is compiled with NO_IPV6.

Signed-off-by: Elia Pinto 
---
 ident.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/ident.c b/ident.c
index 5ff1aad..86b62be 100644
--- a/ident.c
+++ b/ident.c
@@ -69,6 +69,34 @@ static int add_mailname_host(struct strbuf *buf)
fclose(mailname);
return 0;
 }
+#ifndef NO_IPV6
+
+static void add_domainname(struct strbuf *out)
+{
+   char buf[1024];
+   struct addrinfo hints, *ai;
+   int gai;
+
+   if (gethostname(buf, sizeof(buf))) {
+   warning("cannot get host name: %s", strerror(errno));
+   strbuf_addstr(out, "(none)");
+   return;
+   }
+   if (strchr(buf, '.'))
+   strbuf_addstr(out, buf);
+   else{
+   memset (, '\0', sizeof (hints));
+   hints.ai_flags = AI_CANONNAME;
+   if (!(gai = getaddrinfo(buf, NULL, , )) && ai && 
strchr(ai->ai_canonname, '.')) {
+   strbuf_addstr(out, ai->ai_canonname);
+   freeaddrinfo(ai);
+   }
+   else
+   strbuf_addf(out, "%s.(none)", buf);
+   }
+}
+#else /* NO_IPV6 */
+
 
 static void add_domainname(struct strbuf *out)
 {
@@ -88,6 +116,8 @@ static void add_domainname(struct strbuf *out)
strbuf_addf(out, "%s.(none)", buf);
 }
 
+#endif /* NO_IPV6 */
+
 static void copy_email(const struct passwd *pw, struct strbuf *email)
 {
/*
-- 
2.3.3.GIT

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

Anonymous commit (empty Author and Committer)

[Andrei Borzenkov]

See 
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac


I was not even aware that this is possible. Is there anything on server 
side that can prevent it?


Would be good if commit were amended and force pushed to fix 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: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?

[Jeff King]

On Thu, Oct 29, 2015 at 09:20:01PM -0400, Noam Postavsky wrote:

> On Thu, Oct 29, 2015 at 8:50 PM, Jeff King  wrote:
> > workaround (the real inelegance is that you are assuming that "foo"
> > needs run in the first place).
> 
> Well, we currently check the output from "git config
> credential.helpers" to determine what's needed, so the inelegance here
> is that we reimplement git's checking of this option.

Right. And not only is that hard to get right (I doubt, for example, you
support the arbitrary "!" shell expressions that git does), but it's
impossible to know for sure that will be needed, because you cannot know
all possible helpers (I might even have a helper that is a shell snippet
that calls credential-cache).

As a workaround, I don't think looking for just "cache" is unreasonable.
But it is not a very robust solution. :)

> > I'm still not sure how the pre-helper would work. What git command kicks
> > off the pre-helper command? Wouldn't it also be subject to the SIGHUP
> > problem?
> 
> Ah, maybe the missing piece I forgot to mention is that we could make
> our pre/1st-helper be an emacsclient command, which would tell Emacs
> to startup the daemon. So the daemon would be a subprocess of Emacs,
> not "git push", thereby avoiding the SIGHUP. In our current workaround
> we startup the daemon (if it's not running) before git commands that
> we think are going to run credential helpers (i.e. "push", "pull",
> "fetch"), hence my thought that it would be nicer if we only did that
> before git is *actually* going to run the helpers.

I don't think even git knows it will need a helper until it is actually
ready to call one (e.g., it may depend on getting an HTTP 401 from the
server).

I am leaning more towards ignoring SIGHUP (configurably) being the only
really sane path forward. Do you want to try your hand at a patch?

-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 v5 1/3] blame: test to describe use of blame --reverse --first-parent

[Junio C Hamano]

Max Kirillov  writes:

> Reverse blame can be used to locate removal of lines which does not
> change adjacent lines. Such edits do not appear in non-reverse blame,
> because the adjacent lines last changed commit is older history, before
> the edit.
>
> For a big and active project which uses topic branches, or analogous
> feature, for example pull-requests, the history can contain many
> concurrent branches, and even after an edit merged into the target
> branch, there are still many (sometimes several tens or even hundreds)
> topic branch which do not contain it:
>
>  a0--a1-*a2-*a3-a4...-*a100
>  |\ /   / /
>  | b0-B1..bN   / /
>  |\   / /
>  | c0..   ..cN /
>  \/
>   z0....zN
>
> Here, the '*'s mark the first parent in merge, and uppercase B1 - the
> commit where the line being blamed for was removed. Since commits cN-zN
> do not contain the B1, the still have the line removed in B1, and
> reverce blame can report that the last commit for the line was zN
> (meaning that it was removed in a100). In fact it really does return
> some very late commit, and this makes it unusable for finding the B1
> commit.
>
> The search could be done by blame --reverse --first-parent. For range
> a0..a100 it would return a1, and then only one additional blame along
> the a0..bN will return the desired commit b0. But combining --reverse
> and --first-parent was forbidden in 95a4fb0eac, because incorrectly
> specified range could produce unexpected and meaningless result.
>
> Add test which describes the expected behavior of
> `blame --reverse --first-parent` in the case described above.
>
> Signed-off-by: Max Kirillov 
> ---

There were a few obvious typos I spotted but other than that a very
understandable description.  Will queue.

Thanks.

>  t/t8009-blame-vs-topicbranches.sh | 36 
>  1 file changed, 36 insertions(+)
>  create mode 100755 t/t8009-blame-vs-topicbranches.sh
>
> diff --git a/t/t8009-blame-vs-topicbranches.sh 
> b/t/t8009-blame-vs-topicbranches.sh
> new file mode 100755
> index 000..175ad37
> --- /dev/null
> +++ b/t/t8009-blame-vs-topicbranches.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +test_description='blaming trough history with topic branches'
> +. ./test-lib.sh
> +
> +# Creates the history shown below. '*'s mark the first parent in the merges.
> +# The only line of file.t is changed in commit B2
> +#
> +#+---C1
> +#   /  \
> +# A0--A1--*A2--*A3
> +#   \ /
> +#B1-B2
> +#
> +test_expect_success setup '
> + test_commit A0 file.t line0 &&
> + test_commit A1 &&
> + git reset --hard A0 &&
> + test_commit B1 &&
> + test_commit B2 file.t line0changed &&
> + git reset --hard A1 &&
> + test_merge A2 B2 &&
> + git reset --hard A1 &&
> + test_commit C1 &&
> + git reset --hard A2 &&
> + test_merge A3 C1
> + '
> +
> +test_expect_failure 'blame --reverse --first-parent finds A1' '
> + git blame --porcelain --reverse --first-parent A0..A3 -- file.t 
> >actual_full &&
> + head -n 1 actual &&
> + git rev-parse A1 >expect &&
> + test_cmp expect actual
> + '
> +
> +test_done
--
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] http.c: use CURLOPT_RANGE for range requests

[David Turner]

A HTTP server is permitted to return a non-range response to a HTTP
range request (and Apache httpd in fact does this in some cases).
While libcurl knows how to correctly handle this (by skipping bytes
before and after the requested range), it only turns on this handling
if it is aware that a range request is being made.  By manually
setting the range header instead of using CURLOPT_RANGE, we were
hiding the fact that this was a range request from libcurl.  This
could cause corruption.

Signed-off-by: David Turner 
---
 http.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 0f924a8..303b388 100644
--- a/http.c
+++ b/http.c
@@ -1202,8 +1202,9 @@ static int http_request(const char *url,
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
 fwrite);
if (posn > 0) {
-   strbuf_addf(, "Range: bytes=%ld-", posn);
-   headers = curl_slist_append(headers, buf.buf);
+   strbuf_addf(, "%ld-", posn);
+   curl_easy_setopt(slot->curl, CURLOPT_RANGE,
+);
strbuf_reset();
}
} else
-- 
2.4.2.691.g714732c-twtrsrc

--
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: [Savannah-users] Anonymous commit (empty Author and Committer)

[Balaco Baco]

> > See 
> > http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc3
> 19df14cd3398fbdfde665ac
> >
> >
> > 
> > 
> > I was not even aware that this is possible. Is there anything on 
> > server side that can prevent it?
> > 
> > Would be good if commit were amended and force pushed to fix it.
> > 
> > 
> 
> Is this even a problem? I'm pretty sure Git warns you if you try to
> commit something before user.name and user.email are defined, and if
> someone wants to do so, I don't see why you should try to stop them.
> It wouldn't work, anyway; they would just write some simple name like
> "anonymous" and some nonsense email if they really want to be anonymous.
> 

This commit just removed a line from the README:

"Please look at the GRUB Wiki  for
testing-procedures."

Is this change something that may justify that it's-not-me-there action?
I agree that it should not be prevented to avoid some pressure
situations that might be (eventually) present. Bogus random data there
would be harder to track. The empty fields are easy, and if it
guarantees some safety to whoever does it, should be the choice.


-- 
http://www.fastmail.com - mmm... Fastmail...

--
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: [gmane.comp.version-control.git] [PATCH 0/4] gitk crash fix and locale updates

[Junio C Hamano]

Paul Mackerras  writes:

> Yes, sorry about that.  I have applied a patch from Beat Bolli fixing
> the basic issue, since his patch was the same as Takashi's first patch
> and was posted earlier.  I have also applied 2-4 of Takashi's series
> plus some other translation updates.  I have pushed all that out to
> git://ozlabs.org/~paulus/gitk.git.
>
> Please pull whenever is convenient for you.

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] difftool: avoid symlinks when reusing worktree files

[Junio C Hamano]

David Aguilar  writes:

>> +if (-l "$workdir/$file" || ! -e _) {
>>  return (0, $null_sha1);
>>  }
>
> The "-e _" shorthand caught my eye ~ I didn't know perl could do that!
> Nice.
>
> Underline is barely mentioned in perlvar, but it's obvious what
> (I think) it means, and since Perl is DWIM, it must be right. ;-)

It is mentioned under -X (i.e. the headline for -f, -e, -d,
... tests) in perlfunc with an interesting example of doing
an explicit stat() first and then running many variants of -X
with _ as the target.

--
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: It(s) Service : Account Update.

[Chenoy, Rashna]

Attention: E-mail User,

You have exceeded your Outlook E-mail account limit quota of 250MB and you are 
requested to expand it within 24 hours or else your Outlook E-mail account will 
be disable from our database. Simply CLICK 
HERE with the complete information requested 
to expand your Outlook account quota to 1GB.

Thank you for using Outlook E-mail services.

©2015 Office Outlook Information Center
All rights reserved.














































































































































The information contained in this message is confidential and is intended for 
the addressee only. If you have received this message in error or there are any 
problems, please notify the originator immediately. The unauthorised use, 
disclosure, copying or alteration of this message is strictly forbidden. This 
mail and any attachments have been scanned for viruses prior to leaving the 
Barts Health NHS Trust network. Barts Health NHS Trust will not be liable for 
direct, special, indirect or consequential damages arising from alteration of 
the contents of this message by a third party or as a result of any virus being 
passed on.
--
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] git-p4: Handle p4 submit failure

[Junio C Hamano]

Etienne Girard  writes:

> Clean the workspace if p4_write_pipe raised SystemExit,
> so that the user don't have to do it themselves.
>
> Signed-off-by: GIRARD Etienne 
> ---

Thanks.  As I do not really do "p4", I am CC'ing this response to
the area experts.

First, one "procedural" comment.

You sent the patch from your gmail.com address, but signed it off
with another address (with name spelled differently).  If applied
as-is, the result will have your gmail.com address listed as the
author and that will be the name you would see in "git shortlog".

If you rather prefer to be known as the name and address you placed
on your sign-off, while sending the patch from another address, you
can do so by staring the _body_ of the message with what is called
an "in-body header", like this:

From: GIRARD Etienne 

Clean the workspace if p4_write_pipe raised SystemExit,
so that the user don't have to do it themselves.

Signed-off-by: GIRARD Etienne 
---
  ... here you have the usual patch ...

That "From:" line would sit on the first line of the body of your
message, separated by a blank line from the body of the log message.

This is not even a problem from the project's point of view, but I
thought you would want to be aware of it, and may want a chance to
change it.

Note: this time, you do not need to resend the patch; just
please let me know if you want me to do the equivalent of
the above while applying to make your murex address and name
appear as the author in "git log" and "git shortlog" output.

FYI, you could override the "Subject:" of your e-mail header with an
in-body header (this is often useful when replying to an existing
discussion thread with a patch that shows a solution), e.g.

From: GIRARD Etienne 
Subject: git-p4: clean up after p4 submit failure

Clean the workspace if p4_write_pipe raised SystemExit,
so that the user don't have to do it themselves.

Signed-off-by: GIRARD Etienne 
---
  ... here you have the usual patch ...

>  git-p4.py | 71 
> +--
>  1 file changed, 37 insertions(+), 34 deletions(-)
>
> The p4 submit command may fail, for example if the changelist contains
> a job that does not exist in the Jobs section. If this is the case,
> p4_write_pipe will exit the script. This change makes it so that the
> workspace is reverted before letting the script die.

Some of the information contained in this paragraph deserves to be
in the log message proper.  How about

From: GIRARD Etienne 
Subject: git-p4: clean up after p4 submit failure

When "p4 submit" command fails in P4Submit.applyCommit, the
workspace is left with the changes.  We already have a code
to revert the changes to the workspace when the user decides
to cancel submission by aborting the editor that edits the
change description, and we should treat the "p4 submit"
failure the same way.

Clean the workspace if p4_write_pipe raised SystemExit,
so that the user don't have to do it themselves.

Signed-off-by: GIRARD Etienne 

or something like that?

While trying to come up with the above description, I started
wondering if the error message wants to differentiate the two cases.

When self.edit_template() returns false, we know that the user
actively said "no I do not want to submit", and "Submission
cancelled" is perfectly fine, but when "p4 submit" fails because it
did not like the change description or if it found some other
issues, it is not necessarily that the user said "I want to cancel";
it is more like "Submission failed".

Hmm?
--
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] checkout: add --progress option

[Eric Sunshine]

On Fri, Oct 30, 2015 at 2:48 PM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
>> On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz
>>  wrote:
>>> +   opts.verbose_update = !o->quiet && (option_progress > 0 ||
>>> +  (option_progress < 0 && 
>>> isatty(2)));
>>
>> Does this logic also need to be applied to the other instance where
>> isatty() is consulted in merge_working_tree()?
>
> This makes me wonder if option_progress and o->quiet change once
> parse_options() finished doing its job.  If we know that these two
> will never change once parse_options() is done, then we can
> introduce a variable "show_progress", assign the value of that
> variable to opts.verbose_update in these places and then arrange
> that variable is set to the combination of quiet, option_progress
> and isatty(2) just once after parse_options().
>
> That is, something like this on top of Edmundo's patch.
>
> @@ -27,7 +27,7 @@ static const char * const checkout_usage[] = {
> -static int option_progress = -1;
> +static int show_progress = 1;
> @@ -430,8 +430,7 @@ static int reset_tree(struct tree *tree, const struct 
> checkout_opts *o,
> -   opts.verbose_update = !o->quiet && (option_progress > 0 ||
> -  (option_progress < 0 && 
> isatty(2)));
> +   opts.verbose_update = show_progress;
> @@ -515,7 +514,7 @@ static int merge_working_tree(const struct checkout_opts 
> *opts,
> -   topts.verbose_update = !opts->quiet && isatty(2);
> +   topts.verbose_update = show_progress;
> @@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const 
> char *prefix)
> +   int option_progress = -1;
> @@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const 
> char *prefix)
> argc = parse_options(argc, argv, prefix, options, checkout_usage,
>  PARSE_OPT_KEEP_DASHDASH);
>
> +   show_progress = (!opts.quiet &&
> +(0 < option_progress || (option_progress < 0 && 
> isatty(2;

Yep, I was thinking that too. Also, Edmundo's explanatory comment
about how --quiet, --progress, and isatty() interact would move down
to this location (if it's deemed worth keeping).
--
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/2] Fix Git GUI in Git for Windows 2.x

[Johannes Schindelin]

With the switch to 2.x, Git for Windows started relying on MSys2
instead of MSys. This also implies that we now have the `cygpath`
tool that was formerly used to determine whether we're running
Cygwin or MinGW/MSys.

Another issue: the code behind Repository>Create Desktop Icon
generated a `.lnk` file that fails to launch the Git GUI in Git
for Windows 2.x.

This topic branch fixes both bugs.


Johannes Schindelin (2):
  git-gui: fix detection of Cygwin
  git-gui (Windows): use git-gui.exe in `Create Desktop Shortcut`

 git-gui.sh   | 10 --
 lib/shortcut.tcl | 14 ++
 2 files changed, 14 insertions(+), 10 deletions(-)

-- 
2.5.3.windows.1.3.gc322723

--
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/2] git-gui: fix detection of Cygwin

[Johannes Schindelin]

MSys2 might *look* like Cygwin, but it is *not* Cygwin... Unless it
is run with `MSYSTEM=MSYS`, that is.

This change is backwards-compatible because MSys also used the
MSYSTEM variable to determine whether to run in MSys or in MinGW
mode. The only difference is that MSys does not support running
in 64-bit mode at all.

Signed-off-by: Johannes Schindelin 
---
 git-gui.sh | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 11048c7..9038a4c 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -270,12 +270,10 @@ proc is_Windows {} {
 proc is_Cygwin {} {
global _iscygwin
if {$_iscygwin eq {}} {
-   if {$::tcl_platform(platform) eq {windows}} {
-   if {[catch {set p [exec cygpath --windir]} err]} {
-   set _iscygwin 0
-   } else {
-   set _iscygwin 1
-   }
+   if {$::tcl_platform(platform) eq {windows} &&
+   (![info exists ::env(MSYSTEM)] ||
+$::env(MSYSTEM) ne {MSYS})} {
+   set _iscygwin 1
} else {
set _iscygwin 0
}
-- 
2.5.3.windows.1.3.gc322723


--
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/2] git-gui (Windows): use git-gui.exe in `Create Desktop Shortcut`

[Johannes Schindelin]

When calling `Repository>Create Desktop Shortcut`, Git GUI assumes
that it is okay to call `wish.exe` directly on Windows. However, in
Git for Windows 2.x' context, that leaves several crucial environment
variables uninitialized, resulting in a shortcut that does not work.

To fix those environment variable woes, Git for Windows comes with a
convenient `git-gui.exe`, so let's just use it when it is available.

This fixes https://github.com/git-for-windows/git/issues/448

Signed-off-by: Johannes Schindelin 
---
 lib/shortcut.tcl | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl
index 78878ef..715916b 100644
--- a/lib/shortcut.tcl
+++ b/lib/shortcut.tcl
@@ -11,11 +11,17 @@ proc do_windows_shortcut {} {
if {[file extension $fn] ne {.lnk}} {
set fn ${fn}.lnk
}
+   # Use /cmd/git-gui.exe if available
+   set normalized [file normalize $::argv0]
+   regsub "/mingw../libexec/git-core/git-gui$" \
+   $normalized "/cmd/git-gui.exe" cmdLine
+   if {$cmdLine != $normalized && [file exists $cmdLine]} {
+   set cmdLine [list [file nativename $cmdLine]]
+   } else {
+   set cmdLine [list [info nameofexecutable] $normalized]
+   }
if {[catch {
-   win32_create_lnk $fn [list \
-   [info nameofexecutable] \
-   [file normalize $::argv0] \
-   ] \
+   win32_create_lnk $fn $cmdLine \
[file normalize $_gitworktree]
} err]} {
error_popup [strcat [mc "Cannot write shortcut:"] 
"\n\n$err"]
-- 
2.5.3.windows.1.3.gc322723

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

[Johannes Schindelin]

Hi Jonathan,

On Fri, 30 Oct 2015, Jonathan Nieder wrote:

> Junio C Hamano wrote:
> > Johannes Schindelin  writes:
> >> On Tue, 27 Oct 2015, Junio C Hamano wrote:
> 
> >>> It can be called GDB=1, perhaps?
> >>
> >> No, this is way too generic. As I only test that the environment
> >> variable's existence, even something like GDB=/usr/opt/gdb/bin/gdb would
> >> trigger it.
> >>
> >> I could be talked into GDB_GIT=1, though.
> >
> > As I said in another message, I have no preference myself over the
> > name of this variable (or making it a shell function like Duy
> > mentioned, which incidentally may give us more visual pleasantness
> > by losing '=').
> >
> > I'd just be happy as long as the feature becomes available, and I'd
> > leave the choice of consistent and convenient naming to others who
> > have stronger opinions ;-)
> 
> Here's a suggested patch.

I am fine with this patch as a replacement for my original version.

Thanks,
Dscho
--
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:

> Maybe add a #define LIBCURL_CAN_HANDLE_PROXY_AUTH to clarify this, like we do
> with LIBCURL_CAN_HANDLE_AUTH_ANY?

Quite frankly, my first preference is to have a code that is clear
enough so that you do not need such an intermediate macro.  By
isolating implementation details that have to be version dependent
into a helper function whose purpose is well defined, the rest of
the code can be #ifdef free; it would become sufficiently clear to
switch based on curl version where implementation details matter.

By refraining from littering #ifdef all over the code, we do assign
to disable_gssnegotiate even though the value is not even used when
compiled for ancient version of libCURL, but the benefit of code
clarity outweighs such downside.  We may have to use #ifdef/#endif
in some places, but we should in general minimize their uses and
write code for the more up-to-date API.

For what I mean, see the attached patch outline to show how to get
rid of CAN_HANDLE_AUTH_ANY.

> How about env_override? Not perfect, but probably better.

Much better than anything I'd come up with myself (I am bad at
naming, even though I may sometimes be good at spotting a bad name).


 http.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/http.c b/http.c
index 7da76ed..d272b02 100644
--- a/http.c
+++ b/http.c
@@ -15,10 +15,6 @@ int active_requests;
 int http_is_verbose;
 size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
 
-#if LIBCURL_VERSION_NUM >= 0x070a06
-#define LIBCURL_CAN_HANDLE_AUTH_ANY
-#endif
-
 static int min_curl_sessions = 1;
 static int curl_session_count;
 #ifdef USE_CURL_MULTI
@@ -79,9 +75,6 @@ static const char *user_agent;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-static unsigned long http_auth_methods = CURLAUTH_ANY;
-#endif
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -90,6 +83,19 @@ static struct active_request_slot *active_queue_head;
 
 static char *cached_accept_language;
 
+static int disable_gssnegotiate;
+
+static void set_httpauth_opt(CURL *curl)
+{
+#if LIBCURL_VERSION_NUM >= 0x070a06  /* Is CURLAUTH_ANY available? */
+   unsigned long auth_methods = CURLAUTH_ANY;
+
+   if (disable_gssnegotiate)
+   auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+   curl_easy_setopt(curl, CURLOPT_HTTPAUTH, auth_methods);
+#endif
+}
+
 size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
 {
size_t size = eltsize * nmemb;
@@ -375,9 +381,7 @@ static CURL *get_curl_handle(void)
 #if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
 #endif
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-   curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
-#endif
+   set_httpauth_opt(result);
 
if (http_proactive_auth)
init_curl_http_auth(result);
@@ -681,9 +685,7 @@ struct active_request_slot *get_active_slot(void)
curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-   curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
-#endif
+   set_httpauth_opt(slot->curl);
if (http_auth.password)
init_curl_http_auth(slot->curl);
 
@@ -943,9 +945,7 @@ static int handle_curl_result(struct slot_results *results)
credential_reject(_auth);
return HTTP_NOAUTH;
} else {
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-   http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
-#endif
+   disable_gssnegotiate = 1;
return HTTP_REAUTH;
}
} else {
--
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

[Jeff King]

On Fri, Oct 30, 2015 at 07:25:24PM +0100, Johannes Schindelin wrote:

> > I agree doing so would be crazy. But would:
> > 
> >   ./t1234-frotz.sh --gdb=17
> > 
> > be sane to run gdb only inside test 17?
> 
> It would probably be sane, but I never encountered the need for something
> like that. It was always much easier to run the test using `sh -x t... -i
> -v` to find out what command was behaving funnily (mind you, that can be a
> pretty hard thing todo, we have some quite convoluted test scripts in our
> code base) and then edit the test.
> 
> I would expect that `--gdb=` thing to drive me crazy: first, I would
> choose the wrong number. Next, I would probably forget that test_commit
> and other commands *also* calls Git.

Yeah, good points. You somehow have to say "debug _this_ git
invocation", and there is probably not a more precise way to do that
than sticking something in the code on the right line.

I do think I like Junio's "debug git foo" rather than setting the
environment variable, as its syntactically a little simpler to type (and
of course it would probably be implemented with an environment variable,
so one could whichever style they prefer).

-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: [PATCHv2 4/8] submodule-config: parse_config

[Stefan Beller]

On Thu, Oct 29, 2015 at 6:53 PM, Eric Sunshine  wrote:
> On Wed, Oct 28, 2015 at 7:21 PM, Stefan Beller  wrote:
>> submodule-config: parse_config
>
> Um, what?

submodule-config: Introduce parse_generic_submodule_config

>
>> This rewrites parse_config to distinguish between configs specific to
>> one submodule and configs which apply generically to all submodules.
>> We do not have generic submodule configs yet, but the next patch will
>> introduce "submodule.jobs".
>>
>> Signed-off-by: Stefan Beller 
>>
>> # Conflicts:
>> #   submodule-config.c
>
> Interesting.

fixed

>
> Minor: Are these 'key', 'value', 'var' arguments analogous to the
> like-named arguments of parse_generic_submodule_config()? If so, why
> is the order of arguments different?

Reordered. I thought how they made most sense individually, but consistency
across functions is better.

>
>> +{
>> +   int ret = 0;
>> +   struct submodule *submodule = lookup_or_create_by_name(me->cache,
>> +  
>> me->gitmodules_sha1,
>> +  name);
>>
>> if (!strcmp(key, "path")) {
>> if (!value)
>> @@ -318,6 +314,30 @@ static int parse_config(const char *var, const char 
>> *value, void *data)
>> return ret;
>>  }
>>
>> +static int parse_config(const char *var, const char *value, void *data)
>> +{
>> +   struct parse_config_parameter *me = data;
>> +
>> +   int subsection_len;
>> +   const char *subsection, *key;
>> +   char *name;
>> +
>> +   if (parse_config_key(var, "submodule", ,
>> +_len, ) < 0)
>> +   return 0;
>> +
>> +   if (!subsection_len)
>> +   return parse_generic_submodule_config(var, key, value);
>> +   else {
>> +   int ret;
>> +   /* subsection is not null terminated */
>> +   name = xmemdupz(subsection, subsection_len);
>> +   ret = parse_specific_submodule_config(me, name, key, value, 
>> var);
>> +   free(name);
>> +   return ret;
>> +   }
>> +}
>
> Minor: You could drop the 'else' and outdent its body, thus losing one
> indentation level.

By passing on the subsection, subsection_len, we only have one statement there

 if (!subsection_len)
 return parse_generic_submodule_config(key, var, value, me);
 else
 return parse_specific_submodule_config(subsection,
   subsection_len, key,
  var, value, me);

will do without dedenting I guess.
--
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/2] http: use credential API to handle proxy authentication

[Junio C Hamano]

Knut Franke  writes:

> On 2015-10-28 14:58, Eric Sunshine wrote:
>> > +   }
>> > +   if (!curl_http_proxy) {
>> > +   copy_from_env(_http_proxy, "ALL_PROXY");
>> > +   copy_from_env(_http_proxy, "all_proxy");
>> > +   }
>> 
>> If this sort of upper- and lowercase environment variable name
>> checking is indeed desirable, I wonder if it would make sense to fold
>> that functionality into the helper function.
>
> It's just for consistency with libcurl here, not generally desirable; so I 
> don't
> think it makes sense to add it to the helper.

I agree.  Unlike many environment variables, historically these
proxy environment variables were all lowercase only for quite a
while (found as early as in CERN libwww 2.15 March '94).

It appears that CURL did not know this and implemented only
uppercase variants, which was later corrected to take both
(http://sourceforge.net/p/curl/bugs/40/ shows a fix in Nov 2000).

So both unfortunately are used in user's environment and we need to
pay attention to both.  As lowercase version is the more kosher one,
looking at uppercase first and then overriding it with the lowercase
one is the right order, I think.

It's a mess, but it is limited to these proxy-ish variables and does
not deserve a helper to promote the pattern.
--
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] checkout: add --progress option

[Jeff King]

On Fri, Oct 30, 2015 at 11:48:36AM -0700, Junio C Hamano wrote:

> > Does this logic also need to be applied to the other instance where
> > isatty() is consulted in merge_working_tree()?
> 
> This makes me wonder if option_progress and o->quiet change once
> parse_options() finished doing its job.  If we know that these two
> will never change once parse_options() is done, then we can
> introduce a variable "show_progress", assign the value of that
> variable to opts.verbose_update in these places and then arrange
> that variable is set to the combination of quiet, option_progress
> and isatty(2) just once after parse_options().
> 
> That is, something like this on top of Edmundo's patch.

Yeah, I like splitting it out like this.

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e28c36b..53d7c49 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -27,7 +27,7 @@ static const char * const checkout_usage[] = {
>   NULL,
>  };
>  
> -static int option_progress = -1;
> +static int show_progress = 1;

You don't need this initialization, right? Looks like we assign
unconditionally to it in cmd_checkout.

> @@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const 
> char *prefix)
>   struct branch_info new;
>   char *conflict_style = NULL;
>   int dwim_new_local_branch = 1;
> + int option_progress = -1;
>   struct option options[] = {
>   OPT__QUIET(, N_("suppress progress reporting")),
>   OPT_STRING('b', NULL, _branch, N_("branch"),
> @@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const 
> char *prefix)
>   argc = parse_options(argc, argv, prefix, options, checkout_usage,
>PARSE_OPT_KEEP_DASHDASH);
>  
> + show_progress = (!opts.quiet &&
> +  (0 < option_progress || (option_progress < 0 && 
> isatty(2;
> +

I sometimes find it confusing when there are two variables with very
similar meanings (option_progress and show_progress here). I wonder if
we could use one variable, like:

  static int show_progress = -1;
  ...
  OPT_BOOL(0, "progress", _progress, ...);
  ...
  parse_options(...);
  if (show_progress < 0) {
if (opts.quiet)
show_progress = 0;
else
show_progress = isatty(2);
  }

That somehow is much clearer to me, especially around the behavior of
"-q --progress". Mine does the opposite of what you wrote above, but I
think it makes more sense.

I can live with it either way, though. :)

-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

[off topic] xkcd

[Duy Nguyen]

We are on xkcd! We're famous now! /me goes hide

https://xkcd.com/1597/
-- 
Duy
--
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/2] http: use credential API to handle proxy authentication

[Eric Sunshine]

On Fri, Oct 30, 2015 at 3:31 PM, Junio C Hamano  wrote:
> Knut Franke  writes:
>> On 2015-10-28 14:58, Eric Sunshine wrote:
>>> > +   }
>>> > +   if (!curl_http_proxy) {
>>> > +   copy_from_env(_http_proxy, "ALL_PROXY");
>>> > +   copy_from_env(_http_proxy, "all_proxy");
>>> > +   }
>>>
>>> If this sort of upper- and lowercase environment variable name
>>> checking is indeed desirable, I wonder if it would make sense to fold
>>> that functionality into the helper function.
>>
>> It's just for consistency with libcurl here, not generally desirable; so I 
>> don't
>> think it makes sense to add it to the helper.
>
> I agree.  Unlike many environment variables, historically these
> proxy environment variables were all lowercase only for quite a
> while (found as early as in CERN libwww 2.15 March '94).
>
> It appears that CURL did not know this and implemented only
> uppercase variants, which was later corrected to take both
> (http://sourceforge.net/p/curl/bugs/40/ shows a fix in Nov 2000).
>
> So both unfortunately are used in user's environment and we need to
> pay attention to both.  As lowercase version is the more kosher one,
> looking at uppercase first and then overriding it with the lowercase
> one is the right order, I think.
>
> It's a mess, but it is limited to these proxy-ish variables and does
> not deserve a helper to promote the pattern.

It would be great to have this explanation in the commit message or
in-code comment. 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] checkout: add --progress option

[Junio C Hamano]

Jeff King  writes:

> I sometimes find it confusing when there are two variables with very
> similar meanings (option_progress and show_progress here). I wonder if
> we could use one variable, like:
>
>   static int show_progress = -1;
>   ...
>   OPT_BOOL(0, "progress", _progress, ...);
>   ...
>   parse_options(...);
>   if (show_progress < 0) {
>   if (opts.quiet)
>   show_progress = 0;
>   else
>   show_progress = isatty(2);
>   }
>
> That somehow is much clearer to me, especially around the behavior of
> "-q --progress". Mine does the opposite of what you wrote above, but I
> think it makes more sense.
>
> I can live with it either way, though. :)

Actually, using a single variable is my preference.  In this case I
wanted to illustrate that the value parsed by parse_options() does
not have to be the one that is used in the final location deep in
the callchain for educational purposes, and I found it clearer to
use two separate variables in the illustration.
--
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/2] checkout: added two options to control progress output

[Junio C Hamano]

Jeff King  writes:

>> --progress-lf: print progress information using LFs instead of CRs
>
> I notice this is part of your patch 1, but it really seems orthogonal to
> checkout's --progress option. It should probably be a separate patch,
> and it probably needs some justification in the commit message (i.e.,
> explain not just _what_ you are doing in the patch, but _why_ it is
> useful).

Yes, and as I doubted its validity, I would really have preferred to
see it done as a later step.  The "we want to say --[no-]progress"
one on the other hand looked a very sensible change, and it was sad
to see it taken hostage by that -lf thing.

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] ident.c: add support for IPv6

[Torsten Bögershausen]

On 2015-10-30 15.48, Elia Pinto wrote:
> Add IPv6 support by implementing name resolution with the
Minor question: How is this related to IPV6?
Could the header line be written something like

"ident.c: Use getaddrinfo() instead of gethostbyname() if available"

On which systems has the patch been tested ?
Linux ?
Mac OS X ?
Windows ?
BSD ?

The motivation on which platforms the usage of getaddrinfo() is preferred
over gethostbyname() could be helpful to motivate this patch:
System XYZ behaves bad when gethostbyname() is used.
Fix it by using getaddrinfo() instead.

A more defensive patch could call getaddrinfo() (If available, iow
when NO_IPV6 is false), and if that fails for whatever reason,
fall back to gethostbyname(), which should be available on all systems.


> protocol agnostic getaddrinfo(3) API. The old gethostbyname(3)
> code is still available when git is compiled with NO_IPV6.
> 
> Signed-off-by: Elia Pinto 
> ---
>  ident.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/ident.c b/ident.c
> index 5ff1aad..86b62be 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -69,6 +69,34 @@ static int add_mailname_host(struct strbuf *buf)
>   fclose(mailname);
>   return 0;
>  }
> +#ifndef NO_IPV6
> +
> +static void add_domainname(struct strbuf *out)
> +{
> + char buf[1024];
> + struct addrinfo hints, *ai;
> + int gai;
The scope of these variables can be narrowed, by moving them into the "{" block,
where they are needed. (Before the memset())
> +
> + if (gethostname(buf, sizeof(buf))) {
> + warning("cannot get host name: %s", strerror(errno));
> + strbuf_addstr(out, "(none)");
> + return;
> + }
> + if (strchr(buf, '.'))
> + strbuf_addstr(out, buf);
> + else{
Many ' ' between else and '{', one should be enough
> + memset (, '\0', sizeof (hints));
> + hints.ai_flags = AI_CANONNAME;
> + if (!(gai = getaddrinfo(buf, NULL, , )) && ai && 
> strchr(ai->ai_canonname, '.')) {
> + strbuf_addstr(out, ai->ai_canonname);
> + freeaddrinfo(ai);
> + }
> + else
Colud be written in one line as "} else"
> + strbuf_addf(out, "%s.(none)", buf);
> + }
> +}
> +#else /* NO_IPV6 */

--
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] git-p4: Handle p4 submit failure

[Etienne Girard]

Hello,

> Note: this time, you do not need to resend the patch; just
> please let me know if you want me to do the equivalent of
> the above while applying to make your murex address and name
> appear as the author in "git log" and "git shortlog" output.

I'd like my murex address to appear on the log please, if it is not
too much trouble. Thank you for all these tips on the submitting
process.

>> The p4 submit command may fail, for example if the changelist contains
>> a job that does not exist in the Jobs section. If this is the case,
>> p4_write_pipe will exit the script. This change makes it so that the
>> workspace is reverted before letting the script die.
>
> Some of the information contained in this paragraph deserves to be
> in the log message proper.  How about
>
> From: GIRARD Etienne 
> Subject: git-p4: clean up after p4 submit failure
>
> When "p4 submit" command fails in P4Submit.applyCommit, the
> workspace is left with the changes.  We already have a code
> to revert the changes to the workspace when the user decides
> to cancel submission by aborting the editor that edits the
> change description, and we should treat the "p4 submit"
> failure the same way.
>
> Clean the workspace if p4_write_pipe raised SystemExit,
> so that the user don't have to do it themselves.
>
> Signed-off-by: GIRARD Etienne 
>
> or something like that?

It seems like a good description. Please let me know if I should
submit another patch with the proper log message

>
> While trying to come up with the above description, I started
> wondering if the error message wants to differentiate the two cases.
>
> When self.edit_template() returns false, we know that the user
> actively said "no I do not want to submit", and "Submission
> cancelled" is perfectly fine, but when "p4 submit" fails because it
> did not like the change description or if it found some other
> issues, it is not necessarily that the user said "I want to cancel";
> it is more like "Submission failed".

Yes, however if `p4 submit` fails the corresponding "Command failed"
error message is displayed, and the p4 error message itself is
displayed if any.
Tthe script will also terminate successfully if self.edit_template
returns false but it will exit with error code 1 if p4 submit fails.

So the user will get "Command failed: [...]" followed by "Submission
cancelled, undoing p4 changes", to let him know that the script failed
because of p4 and that nothing was submitted.
--
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: [PATCHv2 2/8] submodule config: keep update strategy around

[Stefan Beller]

On Thu, Oct 29, 2015 at 6:14 PM, Eric Sunshine  wrote:
>> +   else if (!me->overwrite && submodule->update != NULL)
>
> Although "foo != NULL" is unusual in this code-base, it is used
> elsewhere in this file, including just outside the context seen above.
> Okay.

ok, I'll clean that up as we go.

>> +   free((void *)submodule->update);
>
> Minor: Every other 'free((void *) foo)' in this file has a space after
> "(void *)", one of which can be seen in the context just above.

done
--
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] checkout: add --progress option

[Junio C Hamano]

Edmundo Carmona Antoranz  writes:

> Under normal circumstances, and like other git commands,
> git checkout will write progress info to stderr if
> attached to a terminal. This option allows progress
> to be forced even if not using a terminal. Also,
> progress can be skipped if using option --no-progress.
>
> Signed-off-by: Edmundo Carmona Antoranz 
> ---
>  Documentation/git-checkout.txt |  6 ++
>  builtin/checkout.c | 17 -
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index e269fb1..93ba35a 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -107,6 +107,12 @@ OPTIONS
>  --quiet::
>   Quiet, suppress feedback messages.
>  
> +--progress::
> + Progress status is reported on the standard error stream
> + by default when it is attached to a terminal, unless -q
> + is specified. This flag forces progress status even if the
> + standard error stream is not directed to a terminal.
> +

Unlike some other codepaths like pack-objects (hence "fetch"), "git
checkout" uses start_progress_delay() to avoid showing the progress
when the operation finishes quickly.  I do not think the --progress
option should "force" the output in such a case, but the text reads
as if that is what happens.

Probably it is not a big deal, though.

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index bc703c0..e28c36b 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -27,6 +27,8 @@ static const char * const checkout_usage[] = {
>   NULL,
>  };
>  
> +static int option_progress = -1;
> +
>  struct checkout_opts {
>   int patch_mode;
>   int quiet;
> @@ -417,7 +419,19 @@ static int reset_tree(struct tree *tree, const struct 
> checkout_opts *o,
>   opts.reset = 1;
>   opts.merge = 1;
>   opts.fn = oneway_merge;
> - opts.verbose_update = !o->quiet && isatty(2);
> + /**
> +  * Rules to display progress:
> +  * -q is selected
> +  *  no verbiage

All the other say "progress will be..."; this only confuses readers
if "verbiage" is referring to the same "progress" or it is something
else (and if so at least two important things are left unsaid: (1)
if "progress" is also part of "verbiage", and (2) what kind of
output you are squelching).

And I cannot quite tell which among these possibilities (and there
may be others) you meant.

> +  * -q is _not_ selected and --no-progress _is_ selected,
> +  *  progress will be skipped
> +  * -q is _not_ selected and --progress _is_ selected,
> +  *  progress will be printed to stderr
> +  * -q is _not_ selected and --progress is 'undefined'
> +  *  progress will be printed to stderr _if_ working on a terminal
> +  */
> + opts.verbose_update = !o->quiet && (option_progress > 0 ||
> +(option_progress < 0 && isatty(2)));
>   opts.src_index = _index;
>   opts.dst_index = _index;
>   parse_tree(tree);
> @@ -1156,6 +1170,7 @@ int cmd_checkout(int argc, const char **argv, const 
> char *prefix)
>   N_("second guess 'git checkout 
> '")),
>   OPT_BOOL(0, "ignore-other-worktrees", 
> _other_worktrees,
>N_("do not check if another worktree is holding the 
> given ref")),
> + OPT_BOOL(0, "progress", _progress, N_("force progress 
> reporting")),
>   OPT_END(),
>   };

I see some existing commands say "show progress" and some others say
"force progress reporting".  At some point we may want to pick one
and use it consistently (but that is not on a topic for 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: Anonymous commit (empty Author and Committer)

[Jeff King]

On Fri, Oct 30, 2015 at 05:26:00PM +0300, Andrei Borzenkov wrote:

> See 
> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> 
> I was not even aware that this is possible. Is there anything on server side
> that can prevent it?

I would have thought that receive.fsckObjects would reject it, but seems
that git-fsck does not complain about it at all, as it is otherwise
syntactically valid (a space separating the zero-length name from the
email, and <> surrounding the empty email).

We do complain during "git commit" about an empty name. We don't seem to
do so for blank emails, though. The only discussion I could find
mentions that should probably disallow both[1].

I wonder how this commit was created in the first place (through
git-commit, and we have an empty-name case that is not covered, or using
a lower-level tool that bypasses the checks).

-Peff

[1] http://article.gmane.org/gmane.comp.version-control.git/261237
--
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: [PATCHv2 1/8] run_processes_parallel: Add output to tracing messages

[Stefan Beller]

On Thu, Oct 29, 2015 at 6:10 PM, Eric Sunshine  wrote:
> On Wed, Oct 28, 2015 at 7:21 PM, Stefan Beller  wrote:
>> run_processes_parallel: Add output to tracing messages
>
> This doesn't really say much. I guess you mean that the intention is
> to delimit a section in which output from various tasks may be
> intermixed.

My original intention is to have it there for testing in later patches,
so I am not so much interested in the delimiting but the raw number
%d here.

> run_processes_parallel: delimit intermixed task output

Sounds good to me, better than my subject.

> s/children/tasks/ maybe?
>
> Minor: Perhaps drop "in parallel" since the parallelism is already
> implied by the "run_processes_parallel" prefix.

done

>> +   trace_printf("run_processes_parallel: parallel processing done");
>
> Minor: Likewise, perhaps just "done" rather than "parallel processing
> done" since the "run_processes_parallel" prefix already implies
> parallelism.

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

[Johannes Schindelin]

Hi Victor,

On Thu, 29 Oct 2015, Victor Leschuk wrote:

> >   +if test -n "$TEST_GDB_GIT"
> > +then
> > +   exec gdb -args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> Maybe we could make $TEST_GDB_GIT not just a boolean flag? It would be useful
> to contain "gdb" executable name. It would allow to set path to GDB when it
> is not in $PATH, set different debuggers (for example, I usually use cgdb),
> or even set it to /path/to/gdb_wrapper.sh which could contain different gdb
> options and tunings.

Sure, as long as TEST_GDB_GIT=1 still works. Why don't you make an add-on
patch and submit it?

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] checkout: add --progress option

[Junio C Hamano]

Eric Sunshine  writes:

> On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz
>  wrote:
>> Under normal circumstances, and like other git commands,
>> git checkout will write progress info to stderr if
>> attached to a terminal. This option allows progress
>> to be forced even if not using a terminal. Also,
>> progress can be skipped if using option --no-progress.
>>
>> Signed-off-by: Edmundo Carmona Antoranz 
>> ---
>> -   opts.verbose_update = !o->quiet && isatty(2);
>> +   /**
>> +* Rules to display progress:
>> +* -q is selected
>> +*  no verbiage
>> +* -q is _not_ selected and --no-progress _is_ selected,
>> +*  progress will be skipped
>> +* -q is _not_ selected and --progress _is_ selected,
>> +*  progress will be printed to stderr
>> +* -q is _not_ selected and --progress is 'undefined'
>> +*  progress will be printed to stderr _if_ working on a terminal
>> +*/
>> +   opts.verbose_update = !o->quiet && (option_progress > 0 ||
>> +  (option_progress < 0 && 
>> isatty(2)));
>
> Does this logic also need to be applied to the other instance where
> isatty() is consulted in merge_working_tree()?

This makes me wonder if option_progress and o->quiet change once
parse_options() finished doing its job.  If we know that these two
will never change once parse_options() is done, then we can
introduce a variable "show_progress", assign the value of that
variable to opts.verbose_update in these places and then arrange
that variable is set to the combination of quiet, option_progress
and isatty(2) just once after parse_options().

That is, something like this on top of Edmundo's patch.

 builtin/checkout.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e28c36b..53d7c49 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -27,7 +27,7 @@ static const char * const checkout_usage[] = {
NULL,
 };
 
-static int option_progress = -1;
+static int show_progress = 1;
 
 struct checkout_opts {
int patch_mode;
@@ -430,8 +430,7 @@ static int reset_tree(struct tree *tree, const struct 
checkout_opts *o,
 * -q is _not_ selected and --progress is 'undefined'
 *  progress will be printed to stderr _if_ working on a terminal
 */
-   opts.verbose_update = !o->quiet && (option_progress > 0 ||
-  (option_progress < 0 && isatty(2)));
+   opts.verbose_update = show_progress;
opts.src_index = _index;
opts.dst_index = _index;
parse_tree(tree);
@@ -515,7 +514,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
-   topts.verbose_update = !opts->quiet && isatty(2);
+   topts.verbose_update = show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
struct branch_info new;
char *conflict_style = NULL;
int dwim_new_local_branch = 1;
+   int option_progress = -1;
struct option options[] = {
OPT__QUIET(, N_("suppress progress reporting")),
OPT_STRING('b', NULL, _branch, N_("branch"),
@@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
 PARSE_OPT_KEEP_DASHDASH);
 
+   show_progress = (!opts.quiet &&
+(0 < option_progress || (option_progress < 0 && 
isatty(2;
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
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:
> On Tue, 27 Oct 2015, Johannes Schindelin wrote:
>> On Mon, 26 Oct 2015, Jonathan Nieder wrote:

>>> 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.
[...]
> Actually, after reading the patch again, I think it is better to be less
> intrusive and add the error message *just* for the gdb case, as it is
> right now:

Why?  Unlike the C library function of the same name, the shell
builtin 'exec' prints an error message and exits on error.

Sorry for the lack of clarity,
Jonathan
--
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]

Junio C Hamano wrote:
> Johannes Schindelin  writes:
>> On Tue, 27 Oct 2015, Junio C Hamano wrote:

>>> It can be called GDB=1, perhaps?
>>
>> No, this is way too generic. As I only test that the environment
>> variable's existence, even something like GDB=/usr/opt/gdb/bin/gdb would
>> trigger it.
>>
>> I could be talked into GDB_GIT=1, though.
>
> As I said in another message, I have no preference myself over the
> name of this variable (or making it a shell function like Duy
> mentioned, which incidentally may give us more visual pleasantness
> by losing '=').
>
> I'd just be happy as long as the feature becomes available, and I'd
> leave the choice of consistent and convenient naming to others who
> have stronger opinions ;-)

Here's a suggested patch.

-- >8 --
From: Johannes Schindelin 
Subject: Facilitate debugging Git executables in tests with gdb

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

Signed-off-by: Johannes Schindelin 
Signed-off-by: Jonathan Nieder 
---
 t/README| 5 +
 t/test-lib-functions.sh | 8 
 wrap-for-bin.sh | 8 +++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/t/README b/t/README
index 35438bc..1dc908e 100644
--- a/t/README
+++ b/t/README
@@ -563,6 +563,11 @@ library for your script to use.
argument.  This is primarily meant for use during the
development of a new test script.
 
+ - debug 
+
+   Run a git command inside a debugger. This is primarily meant for
+   use when debugging a failing test script.
+
  - test_done
 
Your test script must have test_done at the end.  Its purpose
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6dffb8b..73e37a1 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -145,6 +145,14 @@ test_pause () {
fi
 }
 
+# Wrap git in gdb. Adding this to a command can make it easier to
+# understand what is going on in a failing test.
+#
+# Example: "debug git checkout master".
+debug () {
+GIT_TEST_GDB=1 "$@"
+}
+
 # Call test_commit with the arguments " [ [ []]]"
 #
 # This will commit a file with the given contents and the given commit
diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
index 701d233..db0ec6a 100644
--- a/wrap-for-bin.sh
+++ b/wrap-for-bin.sh
@@ -19,4 +19,10 @@ GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
 PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
 export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
 
-exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+if test -n "$GIT_TEST_GDB"
+then
+   unset GIT_TEST_GDB
+   exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+else
+   exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+fi
-- 
2.6.0.rc2.230.g3dd15c0

--
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] git-p4: Handle p4 submit failure

[Junio C Hamano]

Etienne Girard  writes:

> Yes, however if `p4 submit` fails the corresponding "Command failed"
> error message is displayed, and the p4 error message itself is
> displayed if any.
> Tthe script will also terminate successfully if self.edit_template
> returns false but it will exit with error code 1 if p4 submit fails.
>
> So the user will get "Command failed: [...]" followed by "Submission
> cancelled, undoing p4 changes", to let him know that the script failed
> because of p4 and that nothing was submitted.

OK, then it sounds like all I have to do is to update the log
message with the "How about this" version and correct the authorship
to use your murex address, and then wait for reviews from real "git
p4" reviewers.

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 1/2] http: allow selection of proxy authentication method

[Knut Franke]

Junio C Hamano wrote:
> > +   if (http_proxy_authmethod) {
> > +   int i;
> > +   for (i = 0; i < ARRAY_SIZE(http_proxy_authmethods); i++) {
> > +   if (!strcmp(http_proxy_authmethod, 
> > http_proxy_authmethods[i].name)) {
> > +   curl_easy_setopt(result, CURLOPT_PROXYAUTH,
> > +   
> > http_proxy_authmethods[i].curlauth_param);
> > +   break;
> > +   }
> > +   }
> > +   if (i == ARRAY_SIZE(http_proxy_authmethods)) {
> > +   warning("unsupported proxy authentication method %s: 
> > using default",
> > + http_proxy_authmethod);
> > +   }
> > +   }
> > +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> > +   else
> > +   curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> > +#endif
> > +}
> 
> This patch should take what 1c2dbf20 (http: support curl < 7.10.7,
> 2015-02-03) wanted to do into account.  Having the configuration
> variable or the environment variable defined by itself, while
> running a Git built with old cURL, shouldn't trigger any warning,
> but the entire function should perhaps be ifdefed out or something?

Maybe add a #define LIBCURL_CAN_HANDLE_PROXY_AUTH to clarify this, like we do
with LIBCURL_CAN_HANDLE_AUTH_ANY? If so, should this be a separate patch? With
the current (scattered) version dependencies, it took me a while to realize that
if the function is ifdefed out for LIBCURL_VERSION_NUM < 0x070a07, we don't need
to worry about default behavior in case LIBCURL_CAN_HANDLE_AUTH_ANY is not
defined. (on the other hand, looking at other curl-version-ifdefs, the define
for AUTH_ANY is the exception)

> >> +static void copy_from_env(const char **var, const char *envname)
> >> +{
> >> +  const char *val = getenv(envname);
> >> +  if (val)
> >> +  *var = xstrdup(val);
> >> +}
[...]
> I primarily was
> wishing that its name more clearly conveyed that it sets the
> variable from the environment _only if_ the environment variable
> exists, and otherwise it does not clobber.

How about env_override? Not perfect, but probably better. I don't think
squeezing in more information (maybe_env_override, override_from_env_var) would
help.

> The implementation of the helper seems to assume that the variable
> must not be pointing at a free-able piece of memory when it is
> called

In fact, if http.proxyAuthMethod (btw, I agree with Eric about capitalization)
is set, I do call it with *var pointing to free-able memory. Will fix this.

FWIW, set_from_env() has the same pre-condition, which doesn't seem to be
satisfied in all cases (namely when overwriting variables previously set by
git_config_string()); not to mention missing free()s in http_cleanup.


Otherwise, I'll make the suggested fixes. Thanks.


Cheers,
Knut
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

--
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] ident.c: add support for IPv6

[Junio C Hamano]

Torsten Bögershausen  writes:

> On 2015-10-30 15.48, Elia Pinto wrote:
>> Add IPv6 support by implementing name resolution with the
> Minor question: How is this related to IPV6?
> Could the header line be written something like
>
> "ident.c: Use getaddrinfo() instead of gethostbyname() if available"
>
> On which systems has the patch been tested ?
> Linux ?
> Mac OS X ?
> Windows ?
> BSD ?
>
> The motivation on which platforms the usage of getaddrinfo() is preferred
> over gethostbyname() could be helpful to motivate this patch:
> System XYZ behaves bad when gethostbyname() is used.
> Fix it by using getaddrinfo() instead.

gethostbyname() fills a hostent that gives us the official name,
list of aliases, _one_ addrtype (either AF_INET or AF_INET6), and
list of addresses, so if we were asking for the physical addresses,
we may not be able to obtain all addresses for a host with both IPv4
and IPv6 addresses, which may be an issue.

But this function is about learning the official name of the host,
given the result of gethostname().  In that context, does that
"limited to single address family" issue of gethostbyname() still
matter?  I am guessing it doesn't, and I somehow doubt that the
value of this patch is about working around any platform bug.

I think the real reason to favour getaddrinfo() over gethostbyname()
is that the family of functions the latter belongs to is obsolete.
In other codepaths where we need to learn the inet address, we
already use getaddrinfo() if available (i.e. NO_IPV6 is not set),
and gethostbyname() is used only when compiling with NO_IPV6.

Converting this codepath to match that pattern incidentally allows
you to work around a platform bugs like "gethostbyname() is broken
on sysmte XYZ" (which you work around by not saying NO_IPV6), but I
view it as a side effect.

>> +static void add_domainname(struct strbuf *out)
>> +{
>> +char buf[1024];
>> +struct addrinfo hints, *ai;
>> +int gai;
> The scope of these variables can be narrowed, by moving them into the "{" 
> block,
> where they are needed. (Before the memset())
>> +
>> +if (gethostname(buf, sizeof(buf))) {
>> +warning("cannot get host name: %s", strerror(errno));
>> +strbuf_addstr(out, "(none)");
>> +return;
>> +}
>> +if (strchr(buf, '.'))
>> +strbuf_addstr(out, buf);
>> +else{
> Many ' ' between else and '{', one should be enough
>> +memset (, '\0', sizeof (hints));
>> +hints.ai_flags = AI_CANONNAME;
>> +if (!(gai = getaddrinfo(buf, NULL, , )) && ai && 
>> strchr(ai->ai_canonname, '.')) {
>> +strbuf_addstr(out, ai->ai_canonname);
>> +freeaddrinfo(ai);
>> +}
>> +else
> Colud be written in one line as "} else"
>> +strbuf_addf(out, "%s.(none)", buf);
>> +}
>> +}
>> +#else /* NO_IPV6 */
--
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] checkout: add --progress option

[Eric Sunshine]

On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz
 wrote:
> Under normal circumstances, and like other git commands,
> git checkout will write progress info to stderr if
> attached to a terminal. This option allows progress
> to be forced even if not using a terminal. Also,
> progress can be skipped if using option --no-progress.
>
> Signed-off-by: Edmundo Carmona Antoranz 
> ---
> -   opts.verbose_update = !o->quiet && isatty(2);
> +   /**
> +* Rules to display progress:
> +* -q is selected
> +*  no verbiage
> +* -q is _not_ selected and --no-progress _is_ selected,
> +*  progress will be skipped
> +* -q is _not_ selected and --progress _is_ selected,
> +*  progress will be printed to stderr
> +* -q is _not_ selected and --progress is 'undefined'
> +*  progress will be printed to stderr _if_ working on a terminal
> +*/
> +   opts.verbose_update = !o->quiet && (option_progress > 0 ||
> +  (option_progress < 0 && 
> isatty(2)));

Does this logic also need to be applied to the other instance where
isatty() is consulted in merge_working_tree()?
--
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: [PATCHv2 2/8] submodule config: keep update strategy around

[Eric Sunshine]

On Fri, Oct 30, 2015 at 1:38 PM, Stefan Beller  wrote:
> On Thu, Oct 29, 2015 at 6:14 PM, Eric Sunshine  wrote:
>>> +   else if (!me->overwrite && submodule->update != NULL)
>>
>> Although "foo != NULL" is unusual in this code-base, it is used
>> elsewhere in this file, including just outside the context seen above.
>> Okay.
>
> ok, I'll clean that up as we go.

Oh, I wasn't suggesting that you clean this up (though you may if you
want). I was merely commenting (for the sake of others reviewing this
patch) that, while not the norm for the project, this instance is
consistent with surrounding code.

>>> +   free((void *)submodule->update);
>>
>> Minor: Every other 'free((void *) foo)' in this file has a space after
>> "(void *)", one of which can be seen in the context just above.
>
> done
--
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

[Johannes Schindelin]

Hi Junio,

On Mon, 26 Oct 2015, Junio C Hamano wrote:

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

Sure. But it would make the patch harder to read.

Do you insist?

Ciao,
Dscho
--
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] ident.c: add support for IPv6

[Eric Sunshine]

On Fri, Oct 30, 2015 at 1:26 PM, Torsten Bögershausen  wrote:
> On 2015-10-30 15.48, Elia Pinto wrote:
>> Add IPv6 support by implementing name resolution with the
>> ---
>> +#ifndef NO_IPV6
>> +
>> +static void add_domainname(struct strbuf *out)
>> +{
>> + char buf[1024];
>> + struct addrinfo hints, *ai;
>> + int gai;
> The scope of these variables can be narrowed, by moving them into the "{" 
> block,
> where they are needed. (Before the memset())
>> +
>> + if (gethostname(buf, sizeof(buf))) {
>> + warning("cannot get host name: %s", strerror(errno));
>> + strbuf_addstr(out, "(none)");
>> + return;
>> + }
>> + if (strchr(buf, '.'))
>> + strbuf_addstr(out, buf);
>> + else{
> Many ' ' between else and '{', one should be enough
>> + memset (, '\0', sizeof (hints));
>> + hints.ai_flags = AI_CANONNAME;
>> + if (!(gai = getaddrinfo(buf, NULL, , )) && ai && 
>> strchr(ai->ai_canonname, '.')) {

Why is 'gai' needed and assigned? It's value is never consulted thereafter.

>> + strbuf_addstr(out, ai->ai_canonname);
>> + freeaddrinfo(ai);

Also, aren't you leaking 'ai' when 'ai_canonname' doesn't contain a '.'?

>> + }
>> + else
> Colud be written in one line as "} else"
>> + strbuf_addf(out, "%s.(none)", buf);
>> + }
>> +}
>> +#else /* NO_IPV6 */
--
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:

> Hi Junio,
>
> On Mon, 26 Oct 2015, Junio C Hamano wrote:
>
>> 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)?
>
> Sure. But it would make the patch harder to read.
>
> Do you insist?

Not at all.  I think the series is already in 'next', and if I
haven't merged it yet, I should.


--
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: [PATCHv2 2/8] submodule config: keep update strategy around

[Stefan Beller]

On Fri, Oct 30, 2015 at 11:16 AM, Eric Sunshine  wrote:
> On Fri, Oct 30, 2015 at 1:38 PM, Stefan Beller  wrote:
>> On Thu, Oct 29, 2015 at 6:14 PM, Eric Sunshine  
>> wrote:
 +   else if (!me->overwrite && submodule->update != NULL)
>>>
>>> Although "foo != NULL" is unusual in this code-base, it is used
>>> elsewhere in this file, including just outside the context seen above.
>>> Okay.
>>
>> ok, I'll clean that up as we go.
>
> Oh, I wasn't suggesting that you clean this up (though you may if you
> want). I was merely commenting (for the sake of others reviewing this
> patch) that, while not the norm for the project, this instance is
> consistent with surrounding code.

I only did a separate patch on top cleaning up 4 occurrences in that file.
We use != NULL quite often throughout the code base, specially in
conditions with side effects like:

while ((char *c = string++) != NULL) {
...

where I think that makes even sense. But there are a minor number of
cases where we have no side effects

$ grep -rI "!= NULL" |grep -v "((" |grep -v "))" |wc -l
135



>
 +   free((void *)submodule->update);
>>>
>>> Minor: Every other 'free((void *) foo)' in this file has a space after
>>> "(void *)", one of which can be seen in the context just above.
>>
>> done
--
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

[Johannes Schindelin]

Hi Peff,

On Tue, 27 Oct 2015, Jeff King wrote:

> On Tue, Oct 27, 2015 at 09:34:48AM -0700, Junio C Hamano wrote:
> 
> > Yeah, that was my first reaction when I saw this patch.  Instead of
> > having to munge that line to "gdb -whatever-args git", you can do a
> > single-shot debugging in a convenient way.  And quite honestly,
> > because nobody sane will run:
> > 
> >  $ cd t && TEST_GDB_GIT=1 sh ./t1234-frotz.sh
> > 
> > and can drive all the "git" running under gdb at the same time, I
> > think what you showed would be the _only_ practical use case.
> 
> I agree doing so would be crazy. But would:
> 
>   ./t1234-frotz.sh --gdb=17
> 
> be sane to run gdb only inside test 17?

It would probably be sane, but I never encountered the need for something
like that. It was always much easier to run the test using `sh -x t... -i
-v` to find out what command was behaving funnily (mind you, that can be a
pretty hard thing todo, we have some quite convoluted test scripts in our
code base) and then edit the test.

I would expect that `--gdb=` thing to drive me crazy: first, I would
choose the wrong number. Next, I would probably forget that test_commit
and other commands *also* calls Git.

Ciao,
Dscho
--
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

[Johannes Schindelin]

Hi Jonathan,

On Tue, 27 Oct 2015, Johannes Schindelin wrote:

> On Mon, 26 Oct 2015, Jonathan Nieder wrote:
> 
> > 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
> 
> I suppose you're right! The `exec` can fail easily, e.g. when `gdb` was
> not found.

Actually, after reading the patch again, I think it is better to be less
intrusive and add the error message *just* for the gdb case, as it is
right now:

-- snipsnap --
 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@@" "$@"

--
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/2] http: use credential API to handle proxy authentication

[Knut Franke]

On 2015-10-28 14:58, Eric Sunshine wrote:
> > +   }
> > +   if (!curl_http_proxy) {
> > +   copy_from_env(_http_proxy, "ALL_PROXY");
> > +   copy_from_env(_http_proxy, "all_proxy");
> > +   }
> 
> If this sort of upper- and lowercase environment variable name
> checking is indeed desirable, I wonder if it would make sense to fold
> that functionality into the helper function.

It's just for consistency with libcurl here, not generally desirable; so I don't
think it makes sense to add it to the helper.

Otherwise, will fix. Thanks.


Cheers,
Knut
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

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

[Johannes Schindelin]

Hi Junio,

On Tue, 27 Oct 2015, Junio C Hamano wrote:

> We are very lucky that the original was posted to SO by our friend
> Pat, and you did the right thing to ask Pat to relicense.

I suspect Pat is on a six month trip around the globe or something,
judging from the feedback I got here:

https://github.com/patthoyts/git-gui/pull/1

Ciao,
Dscho
--
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

[Johannes Schindelin]

Hi Junio,

On Tue, 27 Oct 2015, Junio C Hamano wrote:

> It can be called GDB=1, perhaps?

No, this is way too generic. As I only test that the environment
variable's existence, even something like GDB=/usr/opt/gdb/bin/gdb would
trigger it.

I could be talked into GDB_GIT=1, though.

Ciao,
Dscho
--
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] ident.c: add support for IPv6

[Jeff King]

On Fri, Oct 30, 2015 at 03:48:07PM +0100, Elia Pinto wrote:

> Add IPv6 support by implementing name resolution with the
> protocol agnostic getaddrinfo(3) API. The old gethostbyname(3)
> code is still available when git is compiled with NO_IPV6.

Makes sense. I'm not excited by the duplication in the early part of the
function, though:

> +#ifndef NO_IPV6
> +
> +static void add_domainname(struct strbuf *out)
> +{
> + char buf[1024];
> + struct addrinfo hints, *ai;
> + int gai;
> +
> + if (gethostname(buf, sizeof(buf))) {
> + warning("cannot get host name: %s", strerror(errno));
> + strbuf_addstr(out, "(none)");
> + return;
> + }
> + if (strchr(buf, '.'))
> + strbuf_addstr(out, buf);
> + else{
> + memset (, '\0', sizeof (hints));
> + hints.ai_flags = AI_CANONNAME;
> + if (!(gai = getaddrinfo(buf, NULL, , )) && ai && 
> strchr(ai->ai_canonname, '.')) {
> + strbuf_addstr(out, ai->ai_canonname);
> + freeaddrinfo(ai);
> + }
> + else
> + strbuf_addf(out, "%s.(none)", buf);
> + }
> +}

Especially the "(none)" stuff is ugly enough as it is, without being
duplicated in two spots. Can we factor out the else clause that calls
gethostbyname(), and just override that part with the #ifdef?

For that matter, we have a few other spots that use getaddrinfo and
#ifdef. I wonder if it would be possible to simply use getaddrinfo
everywhere, and make a compatibility wrapper that uses gethostbyname for
older systems. The cut-and-paste duplication in connect.c, for example,
is pretty egregious.

-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 3/6] Facilitate debugging Git executables in tests with gdb

[Junio C Hamano]

Johannes Schindelin  writes:

> Hi Junio,
>
> On Tue, 27 Oct 2015, Junio C Hamano wrote:
>
>> It can be called GDB=1, perhaps?
>
> No, this is way too generic. As I only test that the environment
> variable's existence, even something like GDB=/usr/opt/gdb/bin/gdb would
> trigger it.
>
> I could be talked into GDB_GIT=1, though.

As I said in another message, I have no preference myself over the
name of this variable (or making it a shell function like Duy
mentioned, which incidentally may give us more visual pleasantness
by losing '=').

I'd just be happy as long as the feature becomes available, and I'd
leave the choice of consistent and convenient naming to others who
have stronger opinions ;-)



--
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] git-p4: Handle p4 submit failure

[Luke Diamand]

On 30/10/15 17:57, Junio C Hamano wrote:

Etienne Girard  writes:


Yes, however if `p4 submit` fails the corresponding "Command failed"
error message is displayed, and the p4 error message itself is
displayed if any.
Tthe script will also terminate successfully if self.edit_template
returns false but it will exit with error code 1 if p4 submit fails.

So the user will get "Command failed: [...]" followed by "Submission
cancelled, undoing p4 changes", to let him know that the script failed
because of p4 and that nothing was submitted.


OK, then it sounds like all I have to do is to update the log
message with the "How about this" version and correct the authorship
to use your murex address, and then wait for reviews from real "git
p4" reviewers.



Looks good to me. Nice use of try...finally.

One very small thing - test t9807-git-p4-submit.sh is now failing with 
this change.


That's because it tries to delete one of the files it created, but you 
are now deleting it already! Could you just update that please?


Thanks!
Luke

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

[Jeff King]

On Fri, Oct 30, 2015 at 12:02:56PM -0700, Jonathan Nieder wrote:

> > I'd just be happy as long as the feature becomes available, and I'd
> > leave the choice of consistent and convenient naming to others who
> > have stronger opinions ;-)
> 
> Here's a suggested patch.
> 
> -- >8 --
> From: Johannes Schindelin 
> Subject: Facilitate debugging Git executables in tests with gdb
> 
> When prefixing a Git call in the test suite with 'debug ', it will now
> be run with GDB, allowing the developer to debug test failures more
> conveniently.

At the risk of repeating what I just said elsewhere in the thread, I
think this patch is the best of the proposed solutions.

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

Somebody suggested elsewhere that the name "gdb" be configurable. We
could stick that in the same variable, like:

  test "$GIT_TEST_GDB" = 1 && GIT_TEST_GDB=gdb
  exec ${GIT_TEST_GDB} --args ...

but that does not play well with the "debug" function, which does not
know which value to set it to. I guess we would need GIT_TEST_GDB_PATH
or something.

I am happy to let that get added later by interested parties (I am happy
with "gdb" myself). I just wanted to mention it to make sure we are not
painting ourselves into any corners.

-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: Anonymous commit (empty Author and Committer)

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 30.10.2015 15:26, Andrei Borzenkov wrote:
>> See
>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
>>
>>
>> I was not even aware that this is possible. Is there anything on server
>> side that can prevent it?
>>
>> Would be good if commit were amended and force pushed to fix it.
>>
> It is a bug in SGit. I'll investigate how it happened
>> ___
>> Grub-devel mailing list
>> grub-de...@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>> .
>>
> 
> 




signature.asc
Description: OpenPGP digital signature