Re: [PATCH 0/2] infinite loop in "git ls-tree" for broken symlink

2016-10-14 Thread Petr Stodulka
Thank you for info, I totally missed that. Yes, this fixes the issue
perfectly.

Petr

On 14.10.2016 15:42, Jeff King wrote:
> On Fri, Oct 14, 2016 at 03:16:50PM +0200, Petr Stodulka wrote:
> 
>> I have realized that this wasn't fixed after all when refs.c
>> was "rewritten". Issue is caused by broken symlink under refs/heads,
>> which causes infinite loop for "git ls-tree" command. It was replied
>> earlier [0] and Michael previously fixed that in better way probably,
>> then my proposed patch 2/2, but it contains more changes and I have not
>> enough time to study changes in refs* code, so I propose now just my
>> little patch, which was previously modified by Michael.
>>
>> If you prefer different solution, I am ok with this. It is really
>> just quick proposal. Patch 1/2 contains test for this issue. If you
>> will prefer different solution with different exit code, test should
>> be corrected. Basic idea is, that timeout should'n expire with
>> exit code 124.
>>
>> [0] http://marc.info/?l=git=142712669103790=2
>> [1] https://github.com/mhagger/git, branch "ref-broken-symlinks"
> 
> I think I fixed this semi-independently last week; see the thread at:
> 
>   
> http://public-inbox.org/git/20161006164723.ocg2nbgtulpjc...@sigill.intra.peff.net/
> 
> I say semi-independently because I didn't actually know about your
> previous report, but saw it in the wild myself. But I did talk with
> Michael off-list, and he suggested the belt-and-suspenders retry counter
> in my second patch.
> 
> The fix is at e8c42cb in Junio's tree, and it's currently merged to
> 'next'.
> 
> -Peff
> 



signature.asc
Description: OpenPGP digital signature


[PATCH 2/2] resolve_ref_unsafe(): limit the number of "stat_ref" retries

2016-10-14 Thread Petr Stodulka
From: Michael Haggerty <mhag...@alum.mit.edu>

If there is a broken symlink where a loose reference file is expected,
then the attempt to open() it fails with ENOENT. This error is
misinterpreted to mean that the loose reference file itself has
disappeared due to a race, causing the lookup to be retried. But in
this scenario, the retries all suffer from the same problem, causing
an infinite loop.

So put a limit (of 5) on the number of times that the stat_ref step
can be retried.

Based-on-patch-by: Petr Stodulka <pstod...@redhat.com>
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu>
---
 refs/files-backend.c | 6 --
 refs/refs-internal.h | 6 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index d16feb1..245a0b5 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1353,6 +1353,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
int fd;
int ret = -1;
int save_errno;
+   int retries = 0;
 
*type = 0;
strbuf_reset(_path);
@@ -1390,7 +1391,8 @@ static int files_read_raw_ref(struct ref_store *ref_store,
if (S_ISLNK(st.st_mode)) {
strbuf_reset(_contents);
if (strbuf_readlink(_contents, path, 0) < 0) {
-   if (errno == ENOENT || errno == EINVAL)
+   if ((errno == ENOENT || errno == EINVAL) &&
+   retries++ < MAXRETRIES) 
/* inconsistent with lstat; retry */
goto stat_ref;
else
@@ -1426,7 +1428,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 */
fd = open(path, O_RDONLY);
if (fd < 0) {
-   if (errno == ENOENT)
+   if (errno == ENOENT && retries++ < MAXRETRIES)
/* inconsistent with lstat; retry */
goto stat_ref;
else
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 708b260..37e6b99 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -255,6 +255,12 @@ int rename_ref_available(const char *old_refname, const 
char *new_refname);
 /* We allow "recursive" symbolic refs. Only within reason, though */
 #define SYMREF_MAXDEPTH 5
 
+/* 
+ * We allow only MAXRETRIES tries to jump on stat_ref, because of possible
+ * infinite loop
+ */
+#define MAXRETRIES 5
+
 /* Include broken references in a do_for_each_ref*() iteration: */
 #define DO_FOR_EACH_INCLUDE_BROKEN 0x01
 
-- 
2.5.5



[PATCH 1/2] Add test for ls-tree with broken symlink under refs/heads

2016-10-14 Thread Petr Stodulka
git ls-tree goes into an infinite loop while serving pretty vanilla requests,
if the refs/heads/ directory contains a symlink that's broken. Added test
which check if git ends with expected exit code or timeout expires.
---
 t/t3103-ls-tree-misc.sh | 9 +
 1 file changed, 9 insertions(+)

diff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh
index 09dcf04..faf79c4 100755
--- a/t/t3103-ls-tree-misc.sh
+++ b/t/t3103-ls-tree-misc.sh
@@ -21,4 +21,13 @@ test_expect_success 'ls-tree fails with non-zero exit code 
on broken tree' '
test_must_fail git ls-tree -r HEAD
 '
 
+test_expect_success 'ls-tree fails due to broken symlink instead of infinite 
loop' '
+   mkdir foo_infinit &&
+   cd foo_infinit &&
+   git init testrepo &&
+   cd testrepo &&
+   mkdir -p .git/refs/remotes &&
+   ln -s ../remotes/foo .git/refs/heads/bar &&
+   test_expect_code 128 timeout 2 git ls-tree bar
+'
 test_done
-- 
2.5.5



[PATCH 0/2] infinite loop in "git ls-tree" for broken symlink

2016-10-14 Thread Petr Stodulka
Hi,
I have realized that this wasn't fixed after all when refs.c
was "rewritten". Issue is caused by broken symlink under refs/heads,
which causes infinite loop for "git ls-tree" command. It was replied
earlier [0] and Michael previously fixed that in better way probably,
then my proposed patch 2/2, but it contains more changes and I have not
enough time to study changes in refs* code, so I propose now just my
little patch, which was previously modified by Michael.

If you prefer different solution, I am ok with this. It is really
just quick proposal. Patch 1/2 contains test for this issue. If you
will prefer different solution with different exit code, test should
be corrected. Basic idea is, that timeout should'n expire with
exit code 124.

[0] http://marc.info/?l=git=142712669103790=2
[1] https://github.com/mhagger/git, branch "ref-broken-symlinks"

Michael Haggerty (1):
  resolve_ref_unsafe(): limit the number of "stat_ref" retries

Petr Stodulka (1):
  Add test for ls-tree with broken symlink under refs/heads

 refs/files-backend.c| 6 --
 refs/refs-internal.h| 6 ++
 t/t3103-ls-tree-misc.sh | 9 +
 3 files changed, 19 insertions(+), 2 deletions(-)

-- 
2.5.5



Re: [PATCH 2/2] resolve_ref_unsafe(): limit the number of "stat_ref" retries

2016-10-14 Thread Petr Stodulka
FYI,
I modified the patch slightly.

On 14.10.2016 15:16, Petr Stodulka wrote:
> From: Michael Haggerty <mhag...@alum.mit.edu>
> 
> If there is a broken symlink where a loose reference file is expected,
> then the attempt to open() it fails with ENOENT. This error is
> misinterpreted to mean that the loose reference file itself has
> disappeared due to a race, causing the lookup to be retried. But in
> this scenario, the retries all suffer from the same problem, causing
> an infinite loop.
> 
> So put a limit (of 5) on the number of times that the stat_ref step
> can be retried.
> 
> Based-on-patch-by: Petr Stodulka <pstod...@redhat.com>
> Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu>
> ---
>  refs/files-backend.c | 6 --
>  refs/refs-internal.h | 6 ++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index d16feb1..245a0b5 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1353,6 +1353,7 @@ static int files_read_raw_ref(struct ref_store 
> *ref_store,
>   int fd;
>   int ret = -1;
>   int save_errno;
> + int retries = 0;
>  
>   *type = 0;
>   strbuf_reset(_path);
> @@ -1390,7 +1391,8 @@ static int files_read_raw_ref(struct ref_store 
> *ref_store,
>   if (S_ISLNK(st.st_mode)) {
>   strbuf_reset(_contents);
>   if (strbuf_readlink(_contents, path, 0) < 0) {
> - if (errno == ENOENT || errno == EINVAL)
> + if ((errno == ENOENT || errno == EINVAL) &&
> + retries++ < MAXRETRIES) 
>   /* inconsistent with lstat; retry */
>   goto stat_ref;
>   else
> @@ -1426,7 +1428,7 @@ static int files_read_raw_ref(struct ref_store 
> *ref_store,
>*/
>   fd = open(path, O_RDONLY);
>   if (fd < 0) {
> - if (errno == ENOENT)
> + if (errno == ENOENT && retries++ < MAXRETRIES)
>   /* inconsistent with lstat; retry */
>   goto stat_ref;
>   else
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 708b260..37e6b99 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -255,6 +255,12 @@ int rename_ref_available(const char *old_refname, const 
> char *new_refname);
>  /* We allow "recursive" symbolic refs. Only within reason, though */
>  #define SYMREF_MAXDEPTH 5
>  
> +/* 
> + * We allow only MAXRETRIES tries to jump on stat_ref, because of possible
> + * infinite loop
> + */
> +#define MAXRETRIES 5
> +
>  /* Include broken references in a do_for_each_ref*() iteration: */
>  #define DO_FOR_EACH_INCLUDE_BROKEN 0x01
>  
> 



signature.asc
Description: OpenPGP digital signature


[PATCH] Replace deprecated CURLAUTH_GSSNEGOTIATE with CURLAUTH_NEGOTIATE.

2016-09-28 Thread Petr Stodulka
Macro CURLAUTH_GSSNEGOTIATE is deprecated since cURL v7.38.0 and
should be used CURLAUTH_NEGOTIATE instead. For compatibility
with older versions of cURL is CURLAUTH_NEGOTIATE set as alias
to CURLAUTH_GSSNEGOTIATE

Signed-off-by: Petr Stodulka <pstod...@redhat.com>
---
 http.c| 4 ++--
 http.h| 7 +++
 remote-curl.c | 2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/http.c b/http.c
index 82ed542..7c2d9ef 100644
--- a/http.c
+++ b/http.c
@@ -79,7 +79,7 @@ static struct {
 } proxy_authmethods[] = {
{ "basic", CURLAUTH_BASIC },
{ "digest", CURLAUTH_DIGEST },
-   { "negotiate", CURLAUTH_GSSNEGOTIATE },
+   { "negotiate", CURLAUTH_NEGOTIATE },
{ "ntlm", CURLAUTH_NTLM },
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
{ "anyauth", CURLAUTH_ANY },
@@ -1277,7 +1277,7 @@ static int handle_curl_result(struct slot_results 
*results)
return HTTP_NOAUTH;
} else {
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-   http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+   http_auth_methods &= ~CURLAUTH_NEGOTIATE;
 #endif
return HTTP_REAUTH;
}
diff --git a/http.h b/http.h
index 5ab9d9c..bcc7d7d 100644
--- a/http.h
+++ b/http.h
@@ -42,6 +42,13 @@
 #endif
 
 /*
+ * Keep it compatible on system with cURL < 7.38.0
+ * */
+#ifndef CURLAUTH_NEGOTIATE
+#define CURLAUTH_NEGOTIATE CURLAUTH_GSSNEGOTIATE
+#endif
+
+/*
  * CURLOPT_USE_SSL was known as CURLOPT_FTP_SSL up to 7.16.4,
  * and the constants were known as CURLFTPSSL_*
 */
diff --git a/remote-curl.c b/remote-curl.c
index 6b83b77..d4fce63 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -542,7 +542,7 @@ static int post_rpc(struct rpc_state *rpc)
if (err != HTTP_OK)
return -1;
 
-   if (results.auth_avail & CURLAUTH_GSSNEGOTIATE)
+   if (results.auth_avail & CURLAUTH_NEGOTIATE)
needs_100_continue = 1;
}
 
-- 
2.5.5



Re: [PATCH] http: Control GSSAPI credential delegation.

2016-09-28 Thread Petr Stodulka


On 28.9.2016 19:16, Jeff King wrote:
> On Wed, Sep 28, 2016 at 06:05:52PM +0200, Petr Stodulka wrote:
> 
>> Delegation of credentials is disabled by default in libcurl since
>> version 7.21.7 due to security vulnerability CVE-2011-2192. Which
>> makes troubles with GSS/kerberos authentication where delegation
>> of credentials is required. This can be changed with option
>> CURLOPT_GSSAPI_DELEGATION in libcurl with set expected parameter
>> since libcurl version 7.22.0.
> 
> I don't have any real knowledge of GSSAPI, so I'll refrain from
> commenting on that aspect. But I did notice one mechanical issue:
> 

Me neither. I have just basic knowledge and I am not able to configure
virtual machine, which really need set delegation in libcurl (I need
just negotiation, which is in git possible, I guess since v2.8.0).

However, I discuss it with libcurl maintainer and he confirm that this
option can be required in some cases and this is what I need to do.
this already. I tested just setting of parameter in libcurl according
to description and nothing else seems broken. So anyone else who will
be able to test complete behaviour, where delegation is needed, is welcomed.

[snip]
> We only declare the curl_deleg variable if we have a new-enough curl.
> But...
> 
>> @@ -323,6 +335,10 @@ static int http_options(const char *var, const char 
>> *value, void *cb)
>>  return 0;
>>  }
>>  
>> +if (!strcmp("http.delegation", var)) {
>> +return git_config_string(_deleg, var, value);
>> +}
>> +
> 
> ...here we try to use it regardless. I think you want another #ifdef,
> and probably to warn the user in the #else block (similar to what the
> http.pinnedpubkey code does).
> 
> -Peff
> 

You are right. Thanks. I sent new version of patch with fix.

Petr



signature.asc
Description: OpenPGP digital signature


[PATCH v2] http: Control GSSAPI credential delegation.

2016-09-28 Thread Petr Stodulka
Delegation of credentials is disabled by default in libcurl since
version 7.21.7 due to security vulnerability CVE-2011-2192. Which
makes troubles with GSS/kerberos authentication when delegation
of credentials is required. This can be changed with option
CURLOPT_GSSAPI_DELEGATION in libcurl with set expected parameter
since libcurl version 7.22.0.

This patch provides new configuration variable http.delegation
which corresponds to curl parameter "--delegation" (see man 1 curl).

The following values are supported:

* none (default).
* policy
* always

Signed-off-by: Petr Stodulka <pstod...@redhat.com>
---
 Documentation/config.txt | 14 ++
 http.c   | 37 +
 2 files changed, 51 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e78293b..a179474 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1736,6 +1736,20 @@ http.emptyAuth::
a username in the URL, as libcurl normally requires a username for
authentication.
 
+http.delegation::
+   Control GSSAPI credential delegation. The delegation is disabled
+   by default in libcurl since version 7.21.7. Set parameter to tell
+   the server what it is allowed to delegate when it comes to user
+   credentials. Used with GSS/kerberos. Possible values are:
++
+--
+* `none` - Don't allow any delegation.
+* `policy` - Delegates if and only if the OK-AS-DELEGATE flag is set in the
+  Kerberos service ticket, which is a matter of realm policy.
+* `always` - Unconditionally allow the server to delegate.
+--
+
+
 http.extraHeader::
Pass an additional HTTP header when communicating with a server.  If
more than one such entry exists, all of them are added as extra
diff --git a/http.c b/http.c
index 82ed542..0c65639 100644
--- a/http.c
+++ b/http.c
@@ -90,6 +90,18 @@ static struct {
 * here, too
 */
 };
+#if LIBCURL_VERSION_NUM >= 0x071600
+static const char *curl_deleg;
+static struct {
+   const char *name;
+   long curl_deleg_param;
+} curl_deleg_levels[] = {
+   { "none", CURLGSSAPI_DELEGATION_NONE },
+   { "policy", CURLGSSAPI_DELEGATION_POLICY_FLAG },
+   { "always", CURLGSSAPI_DELEGATION_FLAG },
+};
+#endif
+
 static struct credential proxy_auth = CREDENTIAL_INIT;
 static const char *curl_proxyuserpwd;
 static const char *curl_cookie_file;
@@ -323,6 +335,15 @@ static int http_options(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (!strcmp("http.delegation", var)) {
+#if LIBCURL_VERSION_NUM >= 0x071600
+   return git_config_string(_deleg, var, value);
+#else
+   warning(_("Delegation control is not supported with cURL < 
7.22.0"));
+   return 0;
+#endif
+   }
+
if (!strcmp("http.pinnedpubkey", var)) {
 #if LIBCURL_VERSION_NUM >= 0x072c00
return git_config_pathname(_pinnedkey, var, value);
@@ -629,6 +650,22 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
+#if LIBCURL_VERSION_NUM >= 0x071600
+   if (curl_deleg) {
+   int i;
+   for (i = 0; i < ARRAY_SIZE(curl_deleg_levels); i++) {
+   if (!strcmp(curl_deleg, curl_deleg_levels[i].name)) {
+   curl_easy_setopt(result, 
CURLOPT_GSSAPI_DELEGATION,
+   
curl_deleg_levels[i].curl_deleg_param);
+   break;
+   }
+   }
+   if (i == ARRAY_SIZE(curl_deleg_levels))
+   warning("Unknown delegation method '%s': using default",
+   curl_deleg);
+   }
+#endif
+
if (http_proactive_auth)
init_curl_http_auth(result);
 
-- 
2.5.5



Re: [PATCH] http: Control GSSAPI credential delegation.

2016-09-28 Thread Petr Stodulka


On 28.9.2016 18:05, Petr Stodulka wrote:
> Delegation of credentials is disabled by default in libcurl since
> version 7.21.7 due to security vulnerability CVE-2011-2192. Which
> makes troubles with GSS/kerberos authentication where delegation
> of credentials is required. This can be changed with option
> CURLOPT_GSSAPI_DELEGATION in libcurl with set expected parameter
> since libcurl version 7.22.0.

Correction:
  Which makes troubles with GSS/kerberos authentication when delegation
  of credentials is required.



signature.asc
Description: OpenPGP digital signature


[PATCH] http: Control GSSAPI credential delegation.

2016-09-28 Thread Petr Stodulka
Delegation of credentials is disabled by default in libcurl since
version 7.21.7 due to security vulnerability CVE-2011-2192. Which
makes troubles with GSS/kerberos authentication where delegation
of credentials is required. This can be changed with option
CURLOPT_GSSAPI_DELEGATION in libcurl with set expected parameter
since libcurl version 7.22.0.

This patch provides new configuration variable http.delegation
which corresponds to curl parameter "--delegation" (see man 1 curl).

The following values are supported:

* none (default).
* policy
* always

Signed-off-by: Petr Stodulka <pstod...@redhat.com>
---
 Documentation/config.txt | 14 ++
 http.c   | 32 
 2 files changed, 46 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e78293b..a179474 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1736,6 +1736,20 @@ http.emptyAuth::
a username in the URL, as libcurl normally requires a username for
authentication.
 
+http.delegation::
+   Control GSSAPI credential delegation. The delegation is disabled
+   by default in libcurl since version 7.21.7. Set parameter to tell
+   the server what it is allowed to delegate when it comes to user
+   credentials. Used with GSS/kerberos. Possible values are:
++
+--
+* `none` - Don't allow any delegation.
+* `policy` - Delegates if and only if the OK-AS-DELEGATE flag is set in the
+  Kerberos service ticket, which is a matter of realm policy.
+* `always` - Unconditionally allow the server to delegate.
+--
+
+
 http.extraHeader::
Pass an additional HTTP header when communicating with a server.  If
more than one such entry exists, all of them are added as extra
diff --git a/http.c b/http.c
index 82ed542..5f8fab3 100644
--- a/http.c
+++ b/http.c
@@ -90,6 +90,18 @@ static struct {
 * here, too
 */
 };
+#if LIBCURL_VERSION_NUM >= 0x071600
+static const char *curl_deleg;
+static struct {
+   const char *name;
+   long curl_deleg_param;
+} curl_deleg_levels[] = {
+   { "none", CURLGSSAPI_DELEGATION_NONE },
+   { "policy", CURLGSSAPI_DELEGATION_POLICY_FLAG },
+   { "always", CURLGSSAPI_DELEGATION_FLAG },
+};
+#endif
+
 static struct credential proxy_auth = CREDENTIAL_INIT;
 static const char *curl_proxyuserpwd;
 static const char *curl_cookie_file;
@@ -323,6 +335,10 @@ static int http_options(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (!strcmp("http.delegation", var)) {
+   return git_config_string(_deleg, var, value);
+   }
+
if (!strcmp("http.pinnedpubkey", var)) {
 #if LIBCURL_VERSION_NUM >= 0x072c00
return git_config_pathname(_pinnedkey, var, value);
@@ -629,6 +645,22 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
+#if LIBCURL_VERSION_NUM >= 0x071600
+   if (curl_deleg) {
+   int i;
+   for (i = 0; i < ARRAY_SIZE(curl_deleg_levels); i++) {
+   if (!strcmp(curl_deleg, curl_deleg_levels[i].name)) {
+   curl_easy_setopt(result, 
CURLOPT_GSSAPI_DELEGATION,
+   
curl_deleg_levels[i].curl_deleg_param);
+   break;
+   }
+   }
+   if (i == ARRAY_SIZE(curl_deleg_levels))
+   warning("Unknown delegation method '%s': using default",
+   curl_deleg);
+   }
+#endif
+
if (http_proactive_auth)
init_curl_http_auth(result);
 
-- 
2.5.5



Re: Forgotten declaration of function path_name() in revision.h?

2016-03-19 Thread Petr Stodulka
On 16.3.2016 19:48, Jeff King wrote:
> On Wed, Mar 16, 2016 at 07:05:49PM +0100, Petr Stodulka wrote:
> 
>> according to commit 9831e92 (merge) there is maybe by mistake kept 
>> declaration
>> of function path_name() in revision.h, whose definition was removed
>> and isn't used in git anymore.
> 
> Yes, this should have been part of de1e67d (list-objects: pass full
> pathname to callbacks, 2016-02-11), but I missed it.
> 
> The patch itself looks fine to me (though it probably makes more sense
> to point to de1e67d than the merge).
> 
> Note also that:
> 
>> ==
>> From ae72c8f9085b3b7bd84f94f90ff5b0416db59d67 Mon Sep 17 00:00:00 2001
>> From: Petr Stodulka <pstod...@redhat.com>
>> Date: Wed, 16 Mar 2016 18:51:53 +0100
>> Subject: [PATCH] remove obsoleted function path_name() from header file
>>  revision.h
> 
> It makes things easier on the maintainer if you format your patch such
> that "git am" can apply it directly. Use scissors like:
> 
> -- >8 --
> 
> to separate the patch from anything that should not go into the commit
> message (rather than "=..." as you have here).
> 
> Drop the "From " line which says nothing (it is an mbox separator, but
> we are already inside a message).
> 
> The rest of the headers do not hurt, but are generally redundant with
> what is in your email header (though in this case, the Subject is
> different, so you would want to retain that).
> 
> -Peff
> 

Thanks for advice. I will remember this. When I have time, I will look
at git-send-mail eventually.

Petr



signature.asc
Description: OpenPGP digital signature


Forgotten declaration of function path_name() in revision.h?

2016-03-19 Thread Petr Stodulka
Hi,
according to commit 9831e92 (merge) there is maybe by mistake kept declaration
of function path_name() in revision.h, whose definition was removed
and isn't used in git anymore.

Regards,
Petr


==
From ae72c8f9085b3b7bd84f94f90ff5b0416db59d67 Mon Sep 17 00:00:00 2001
From: Petr Stodulka <pstod...@redhat.com>
Date: Wed, 16 Mar 2016 18:51:53 +0100
Subject: [PATCH] remove obsoleted function path_name() from header file
 revision.h

The function was removed by commit 9831e92 due to possible security
issue.
---
 revision.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/revision.h b/revision.h
index dca0d38..f20168e 100644
--- a/revision.h
+++ b/revision.h
@@ -257,8 +257,6 @@ extern void put_revision_mark(const struct rev_info *revs,
 extern void mark_parents_uninteresting(struct commit *commit);
 extern void mark_tree_uninteresting(struct tree *tree);
 
-char *path_name(struct strbuf *path, const char *name);
-
 extern void show_object_with_name(FILE *, struct object *, const char *);
 
 extern void add_pending_object(struct rev_info *revs,
-- 
2.4.3



signature.asc
Description: OpenPGP digital signature


Re: COPYING tabs vs whitespaces

2016-02-09 Thread Petr Stodulka


On 9.2.2016 19:40, Stefan Beller wrote:
> As a reference for this discussion,
> see c376d96 in git.git or e00bfcbf04 in linux.git
> which deals with whitespaces in the developers certificate of origin.
> 
> Just send a patch, I'd say.
> 
> Thanks,
> Stefan
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Just patch makes from whole thread more and more absurd discussion, no offense.
Don't matter if it is used or not. For me it's significant previous response 
from Junio.

I am not familiar with git send-email, so:
===[PATCH 1/2]
From b2509773edf7e886513f450cb8e215a87278cdfc Mon Sep 17 00:00:00 2001
From: Petr Stodulka <pstod...@redhat.com>
Date: Tue, 9 Feb 2016 19:52:28 +0100
Subject: [PATCH 1/2] COPYING: replace tabs by spaces in license file

GPLv2 license file presented in gnu.org has replaced all tabs by spaces.
Unify whitespaces to make equivalent files (except preamble).
---
 COPYING | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/COPYING b/COPYING
index 536e555..158a9b1 100644
--- a/COPYING
+++ b/COPYING
@@ -19,15 +19,16 @@
 
 
 
-   GNU GENERAL PUBLIC LICENSE
-  Version 2, June 1991
+
+GNU GENERAL PUBLIC LICENSE
+   Version 2, June 1991
 
  Copyright (C) 1989, 1991 Free Software Foundation, Inc.,
  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  Everyone is permitted to copy and distribute verbatim copies
  of this license document, but changing it is not allowed.
 
-   Preamble
+Preamble
 
   The licenses for most software are designed to take away your
 freedom to share and change it.  By contrast, the GNU General Public
@@ -77,7 +78,7 @@ patent must be licensed for everyone's free use or not 
licensed at all.
   The precise terms and conditions for copying, distribution and
 modification follow.
 
-   GNU GENERAL PUBLIC LICENSE
+GNU GENERAL PUBLIC LICENSE
TERMS AND CONDITIONS FOR COPYING, DISTRIBUTION AND MODIFICATION
 
   0. This License applies to any program or other work which contains
@@ -276,7 +277,7 @@ make exceptions for this.  Our decision will be guided by 
the two goals
 of preserving the free status of all derivatives of our free software and
 of promoting the sharing and reuse of software generally.
 
-   NO WARRANTY
+NO WARRANTY
 
   11. BECAUSE THE PROGRAM IS LICENSED FREE OF CHARGE, THERE IS NO WARRANTY
 FOR THE PROGRAM, TO THE EXTENT PERMITTED BY APPLICABLE LAW.  EXCEPT WHEN
@@ -298,9 +299,9 @@ YOU OR THIRD PARTIES OR A FAILURE OF THE PROGRAM TO OPERATE 
WITH ANY OTHER
 PROGRAMS), EVEN IF SUCH HOLDER OR OTHER PARTY HAS BEEN ADVISED OF THE
 POSSIBILITY OF SUCH DAMAGES.
 
-END OF TERMS AND CONDITIONS
+ END OF TERMS AND CONDITIONS
 
-   How to Apply These Terms to Your New Programs
+How to Apply These Terms to Your New Programs
 
   If you develop a new program, and you want it to be of the greatest
 possible use to the public, the best way to achieve this is to make it
-- 
2.4.3


===[PATCH 2/2]
From 5a7bfe0b05f35238c7bc8d2680b168ef729d0e91 Mon Sep 17 00:00:00 2001
From: Petr Stodulka <pstod...@redhat.com>
Date: Tue, 9 Feb 2016 20:08:44 +0100
Subject: [PATCH 2/2] Unify tabs and spaces in preamble of COPYING file

---
 COPYING | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/COPYING b/COPYING
index 158a9b1..bc404d9 100644
--- a/COPYING
+++ b/COPYING
@@ -9,13 +9,13 @@
  decision, you might note so in your copyright message, ie something
  like
 
-   This file is licensed under the GPL v2, or a later version
-   at the discretion of Linus.
+This file is licensed under the GPL v2, or a later version
+at the discretion of Linus.
 
   might avoid issues. But we can also just decide to synchronize and
   contact all copyright holders on record if/when the occasion arises.
 
-   Linus Torvalds
+Linus Torvalds
 
 
 
-- 
2.4.3




signature.asc
Description: OpenPGP digital signature


Re: COPYING tabs vs whitespaces

2016-02-08 Thread Petr Stodulka


On 4.2.2016 20:15, Junio C Hamano wrote:
> Petr Stodulka <pstod...@redhat.com> writes:
> 
>> I found that license file COPYING is different as compared with
>> http://www.gnu.org/licenses/gpl-2.0.txt If I pass over with
>> Linus's preamble, change is only about whitespaces - tabs
>> vs. space.  Probably it's minor non-essential change, but some
>> projects do this change, so rather I ask about that.
> 
> Interesting.  I cannot quite connect "some projects do this change"
> and "so rather I ask".  Are you asking why this project changed it?

Nope. I apologize for my czenglish. It means: From my colleagues I hear,
that some projects had same differences (tabs vs. spaces) in their copy
of the license file and they make it later equivalent with the one in
gnu.org. So I ask rather here / point out this difference, if you know
about that or you want to have same one.

Yes, I know, it's just about whitespaces.

Petr.

> 
> After running "diff" between the two, I think the changes are only
> on the indented section title lines, and "git blame" tells us that
> the section title lines in the copy we have has always been that way
> since Linus added to it at 075b845a (Add a COPYING notice, making it
> explicit that the license is GPLv2., 2005-04-11).
> 
> So, perhaps the copy Linus got originally had leading runs of spaces
> that are multiples-of-8-long unexpanded to leading tabs back then,
> or perhaps he did that unexpanding himself.
> 
> 



signature.asc
Description: OpenPGP digital signature


Re: COPYING tabs vs whitespaces

2016-02-08 Thread Petr Stodulka


On 8.2.2016 18:28, Junio C Hamano wrote:
> Petr Stodulka <pstod...@redhat.com> writes:
> 
>> On 4.2.2016 20:15, Junio C Hamano wrote:
>>> Petr Stodulka <pstod...@redhat.com> writes:
>>>
>>>> I found that license file COPYING is different as compared with
>>>> http://www.gnu.org/licenses/gpl-2.0.txt If I pass over with
>>>> Linus's preamble, change is only about whitespaces - tabs
>>>> vs. space.  Probably it's minor non-essential change, but some
>>>> projects do this change, so rather I ask about that.
>>>
>>> Interesting.  I cannot quite connect "some projects do this change"
>>> and "so rather I ask".  Are you asking why this project changed it?
>>
>> Nope. I apologize for my czenglish. It means: From my colleagues I hear,
>> that some projects had same differences (tabs vs. spaces) in their copy
>> of the license file and they make it later equivalent with the one in
>> gnu.org.
> 
> I'd guess that these projects (among which Linux kernel still has
> these indentation the same as the copy we have) and we independently
> obtained the COPYING file from GNU in some past, and back then the
> copy at GNU was indented that way--which later was changed.
> 
> The Wayback Machine supports this theory.
> 
> https://web.archive.org/web/20070713225446/http://www.gnu.org/licenses/gpl-2.0.txt
> 
> i.e. the FSF copy back in 2007-07 indented these section headers
> with tabs, so those projects that obtained this copy would have
> their sections indented with tabs.
> 
> At 703601d6 (Update COPYING with GPLv2 with new FSF address,
> 2010-01-15), we did a fresh update directly from the URL you cited
> above to primarily replace the addresses of the FSF office.
> 
> https://web.archive.org/web/20100105100239/http://www.gnu.org/licenses/gpl-2.0.txt
> 
> matches what we use (minus Linus's preamble, of course).
> 
> The file before that change was what Linus copied from Linux kernel
> project.  The kernel project did their equivalent change at their
> b3358a11 ([PATCH] update FSF address in COPYING, 2005-09-10), and
> the log message says http://www.gnu.org/licenses/gpl.txt was used.
> 
> The Wayback Machine agrees.
> 
> https://web.archive.org/web/20050901115237/http://www.gnu.org/licenses/gpl.txt
> 
> i.e. the FSF copy back in 2005-09 matches what the kernel uses
> (again, minus Linus's preamble).

I have expected that license was copied correctly in the past from gnu.org,
when same differences are in various projects. I just point out on another 
change.

Thank You for tip about web.archive.org - I really don't know about that web
and it can be useful.

> 
>> So I ask rather here / point out this difference, if you know
>> about that or you want to have same one.
> 
> So the answers are:
> 
>  - No, I didn't personally know about the differences, and I suspect
>nobody particularly cared.
> 
>  - Not really, unless the difference has more substance.  For an
>example of an update with substance, the update we did in 2010
>had not just the FSF address change but also updated the fully
>spelled name of LGPL from Library to Lesser.

Thank You for reponse.

> 
> You may want to bug the kernel folks to update their copy; they
> still spell it as Library General Public License.
> 

Everyone can do that. I believe that someone report it already or at least
constult it. I write about this here because I should do that. When You
know about this difference in license in kernel, I believe that they know
it too and they decide it is ok.

Regards,
Petr



signature.asc
Description: OpenPGP digital signature


COPYING tabs vs whitespaces

2016-02-04 Thread Petr Stodulka
Hi,

I found that license file COPYING is different as compared with 
http://www.gnu.org/licenses/gpl-2.0.txt
If I pass over with Linus's preamble, change is only about whitespaces - tabs 
vs. space.
Probably it's minor non-essential change, but some projects do this change, so 
rather I ask about that.



Regards,
Petr



signature.asc
Description: OpenPGP digital signature


Re: [PATCH] request-pull: short sha handling, manual update

2015-06-18 Thread Petr Stodulka

Hi folks,
can you someone look at it? Or were these troubles mentioned somewhere 
earlier and I miss that?


On 2.6.2015 16:14, Petr Stodulka wrote:
request-pull prints incorrectly warn messages about not found commits 
and man pages don't say
anything about todays changed behaviour. People are confused and try 
look for errors at wrong places.

At least these should be fixed/modified.

Warn massage says that commit can't be found ar remote, however there 
it is in and is missing on local repository
in 'many' cases. So I don't know if better solution is check, where 
commit is truly missing or transform warning message.

Something like:

   warn: No match for commit commit found at url or local repository.
   warn: Are you sure you have synchronized branch with remote 
repository?


..
man page could be changed like this:

---
diff --git a/Documentation/git-request-pull.txt 
b/Documentation/git-request-pull.txt

index 283577b..6d34fc7 100644
--- a/Documentation/git-request-pull.txt
+++ b/Documentation/git-request-pull.txt
@@ -73,6 +73,17 @@ then you can ask that to be pulled with
git request-pull v1.0 https://git.ko.xz/project master:for-linus


+NOTES
+-
+
+Since git version 2.0.0 is behaviour of git request-pull little 
different.

+It is recommended use of third argument for each request-pull, otherwise
+you can get error message like:
+
+   warn: No match for commit commit found at url
+   warn: Are you sure you pushed 'HEAD' there?
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite

---

Second patch provides right processing of third parameter when short 
version of sha hash is used (e.g. 897a111). Now is
supported only full hash, what is different behaviour against first 
parameter or what can be found in other functions. Extra

solves one of cases of wrong warn message.

---
diff --git a/git-request-pull.sh b/git-request-pull.sh
index d5500fd..2dc735e 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -92,9 +92,11 @@ find_matching_ref='
chomp;
my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
my ($pattern);
+   my ($pattern2);
next unless ($sha1 eq $headrev);

$pattern=/$head\$;
+   $pattern2=^$head;
if ($ref eq $head) {
$found = $ref;
}
@@ -104,6 +106,9 @@ find_matching_ref='
if ($sha1 eq $head) {
$found = $sha1;
}
+   elsif ($sha1 =~ /$pattern2/ and (length $head) gt 7) {
+   $found = $sha1
+   }
}
if ($found) {
print $found\n;
---


--
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] request-pull: short sha handling, manual update

2015-06-02 Thread Petr Stodulka
request-pull prints incorrectly warn messages about not found commits 
and man pages don't say
anything about todays changed behaviour. People are confused and try 
look for errors at wrong places.

At least these should be fixed/modified.

Warn massage says that commit can't be found ar remote, however there it 
is in and is missing on local repository
in 'many' cases. So I don't know if better solution is check, where 
commit is truly missing or transform warning message.

Something like:

   warn: No match for commit commit found at url or local repository.
   warn: Are you sure you have synchronized branch with remote repository?

..
man page could be changed like this:

---
diff --git a/Documentation/git-request-pull.txt 
b/Documentation/git-request-pull.txt

index 283577b..6d34fc7 100644
--- a/Documentation/git-request-pull.txt
+++ b/Documentation/git-request-pull.txt
@@ -73,6 +73,17 @@ then you can ask that to be pulled with
git request-pull v1.0 https://git.ko.xz/project master:for-linus


+NOTES
+-
+
+Since git version 2.0.0 is behaviour of git request-pull little different.
+It is recommended use of third argument for each request-pull, otherwise
+you can get error message like:
+
+   warn: No match for commit commit found at url
+   warn: Are you sure you pushed 'HEAD' there?
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite

---

Second patch provides right processing of third parameter when short 
version of sha hash is used (e.g. 897a111). Now is
supported only full hash, what is different behaviour against first 
parameter or what can be found in other functions. Extra

solves one of cases of wrong warn message.

---
diff --git a/git-request-pull.sh b/git-request-pull.sh
index d5500fd..2dc735e 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -92,9 +92,11 @@ find_matching_ref='
chomp;
my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
my ($pattern);
+   my ($pattern2);
next unless ($sha1 eq $headrev);

$pattern=/$head\$;
+   $pattern2=^$head;
if ($ref eq $head) {
$found = $ref;
}
@@ -104,6 +106,9 @@ find_matching_ref='
if ($sha1 eq $head) {
$found = $sha1;
}
+   elsif ($sha1 =~ /$pattern2/ and (length $head) gt 7) {
+   $found = $sha1
+   }
}
if ($found) {
print $found\n;
---
--
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: [BUG] [PATCH] infinite loop due to broken symlink

2015-03-26 Thread Petr Stodulka


On 25.3.2015  23:53 Michael Haggerty wrote:

On 03/23/2015 05:04 PM, Petr Stodulka wrote:

git goes into an infinite loop due to broken symlink (minimal reproducer
[0]).  Affected code is in function
resolve_ref_unsafe in file refs.c - notice 'stat_ref'. There is comment
about problem with race condition, hovewer in that case it's regular broken
symlink which cause infinite loop.

Thanks for the bug report. I can confirm the problem. In fact, I noticed
the same problem when I was working on a refactoring in the area, but I
still haven't submitted those patches. Luckily, modern Git doesn't use
symlinks in the loose reference hierarchy, so most users will never
encounter this problem.

In fact I think it is only the open() call that can lead to the infinite
loop. Meanwhile, there is another problem caused by falling through the
symlink-handling code, namely that st reflects the lstat() of the
symlink rather than the stat() of the file that it points to.

My approach was to run stat() on the path if the symlink-handling code
falls through. You can see my work in progress in my GitHub repo [1].


Yes, I thought up about similar solution, but I wasn't sure about this way
because of race condition (I don't know well code of git yet). In the case
of approved lstat/stat patch, I am more familiar with this solution.

Possible patch could be something
like this:

---
diff --git a/refs.c b/refs.c
index e23542b..9efe8d2 100644
--- a/refs.c
+++ b/refs.c
@@ -1356,6 +1356,7 @@ static struct ref_dir *get_loose_refs(struct
ref_cache *refs)
  /* We allow recursive symbolic refs. Only within reason, though */
  #define MAXDEPTH 5
  #define MAXREFLEN (1024)
+#define MAXLOOP 1024

  /*
   * Called by resolve_gitlink_ref_recursive() after it failed to read
@@ -1482,6 +1483,7 @@ const char *resolve_ref_unsafe(const char
*refname, int resolve_flags, unsigned
 char buffer[256];
 static char refname_buffer[256];
 int bad_name = 0;
+int loop_counter = 0;

 if (flags)
 *flags = 0;
@@ -1546,7 +1548,8 @@ const char *resolve_ref_unsafe(const char
*refname, int resolve_flags, unsigned
 if (S_ISLNK(st.st_mode)) {
 len = readlink(path, buffer, sizeof(buffer)-1);
 if (len  0) {
-   if (errno == ENOENT || errno == EINVAL)
+   if (loop_counter++  MAXLOOP 
+(errno == ENOENT || errno == EINVAL))
 /* inconsistent with lstat;
retry */
 goto stat_ref;
 else
@@ -1579,7 +1582,7 @@ const char *resolve_ref_unsafe(const char
*refname, int resolve_flags, unsigned
  */
 fd = open(path, O_RDONLY);
 if (fd  0) {
-   if (errno == ENOENT)
+   if (loop_counter++  MAXLOOP  errno == ENOENT)
 /* inconsistent with lstat; retry */
 goto stat_ref;
 else
---

If I understand well that simple check of broken symlink is not possible
due to race conditions.

This change also prevents the infinite loop, in fact in a more failproof
way that doesn't depend on errno values being interpreted correctly. I
would suggest a few stylistic changes, like for example here [2]. On the
other hand, this change doesn't solve the lstat()/stat() problem.

Nevertheless, I wouldn't object to a fix like this being accepted in
addition to the stat() fix when it's ready. It doesn't hurt to wear both
a belt *and* suspenders.

Michael

[1] https://github.com/mhagger/git, branch wip/refactor-resolve-ref.
 See especially commit d2425d8ac8cf73494b318078b92f5b1c510a68fb.
[2] https://github.com/mhagger/git, branch ref-broken-symlinks

When stat/lstat  is ready, probably this will not be valuable anymore, 
but it
could be reliable 'stop' for some unexpected/unknown possibly ways in 
future.


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


[BUG] [PATCH] infinite loop due to broken symlink

2015-03-23 Thread Petr Stodulka

Hi guys,
git goes into an infinite loop due to broken symlink (minimal reproducer 
[0]).  Affected code is in function

resolve_ref_unsafe in file refs.c - notice 'stat_ref'. There is comment
about problem with race condition, hovewer in that case it's regular broken
symlink which cause infinite loop. Possible patch could be something 
like this:


---
diff --git a/refs.c b/refs.c
index e23542b..9efe8d2 100644
--- a/refs.c
+++ b/refs.c
@@ -1356,6 +1356,7 @@ static struct ref_dir *get_loose_refs(struct 
ref_cache *refs)

 /* We allow recursive symbolic refs. Only within reason, though */
 #define MAXDEPTH 5
 #define MAXREFLEN (1024)
+#define MAXLOOP 1024

 /*
  * Called by resolve_gitlink_ref_recursive() after it failed to read
@@ -1482,6 +1483,7 @@ const char *resolve_ref_unsafe(const char 
*refname, int resolve_flags, unsigned

char buffer[256];
static char refname_buffer[256];
int bad_name = 0;
+int loop_counter = 0;

if (flags)
*flags = 0;
@@ -1546,7 +1548,8 @@ const char *resolve_ref_unsafe(const char 
*refname, int resolve_flags, unsigned

if (S_ISLNK(st.st_mode)) {
len = readlink(path, buffer, sizeof(buffer)-1);
if (len  0) {
-   if (errno == ENOENT || errno == EINVAL)
+   if (loop_counter++  MAXLOOP 
+(errno == ENOENT || errno == EINVAL))
/* inconsistent with lstat; 
retry */

goto stat_ref;
else
@@ -1579,7 +1582,7 @@ const char *resolve_ref_unsafe(const char 
*refname, int resolve_flags, unsigned

 */
fd = open(path, O_RDONLY);
if (fd  0) {
-   if (errno == ENOENT)
+   if (loop_counter++  MAXLOOP  errno == ENOENT)
/* inconsistent with lstat; retry */
goto stat_ref;
else
---

If I understand well that simple check of broken symlink is not possible
due to race conditions.

Regards,
Petr

[0] https://bugzilla.redhat.com/show_bug.cgi?id=1204193
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[BUG] resolved deltas

2014-08-21 Thread Petr Stodulka

Hi guys,
I wanted post you patch here for this bug, but I can't find primary 
source of this problem [0], because I don't understand some ideas in the 
code. So what I investigate:


Bug is reprodusible since git version 1.8.3.1 (may earlier 1.8.xx, but I 
don't test it) to actual upstream version.
This problem doesn't exists in version 1.7.xx - or more precisely is 
not reproducible. May this is reproducible
since commit 7218a215 - in this commit was added assert in file 
builtin/index-pack.c (actual line is 918), but I didn't test this.


This assert tests if object's (child) real type == OBJ_REF_DELTA. This 
will failure for object with real_type == OBJ_TREE (set as parent's 
type) and type == OBJ_REF_DELTA. Here some prints of important variables 
before failure assert() (from older version but I think that values are 
still actual in this case):

--
(gdb) p base-ref_first
$9 = 3223

(gdb) p deltas[3223]
$10 = {
  base = {
sha1 = \274\070k\343K\324x\037q\273h\327*n\n\356\061$ \036,
offset = 2267795834784135356
  },
  obj_no = 11152
}

(gdb) p *child
$11 = {
  idx = {
sha1 = J\242i\251\261\273\305\067\236%CE\022\257\252\342[;\tD,
crc32 = 2811659028,
offset = 10392153
  },
  size = 30,
  hdr_size = 22,
  type = OBJ_REF_DELTA,
  real_type = OBJ_TREE,
  delta_depth = 0,
  base_object_no = 5458
}

(gdb) p objects[5458]
$13 = {
  idx = {
sha1 = \274\070k\343K\324x\037q\273h\327*n\n\356\061$ \036,
crc32 = 3724458534,
offset = 6879168
  },
  size = 236,
  hdr_size = 2,
  type = OBJ_TREE,
  real_type = OBJ_TREE,
  delta_depth = 0,
  base_object_no = 0
}
---

base_object_no is static 5458. base-ref_first child's object are 
dynamic. If you want stop process in same position my recommendation for 
gdb (if you use gdb) when you will be in file index-pack.c:

br 1093
cont
set variable nr_threads = 1
br 
cond 2 i == 6300
cont
br 916
cont
---
compilated without any optimisation, line numbers modified for commit 
6c4ab27f2378ce67940b4496365043119d72
Condition i == 6300 --- last iteration before failure has dynamic rank 
in range 6304 to 6309 in the most cases (that's weird for me, when count 
of downloaded objects is statically 12806, may wrong search of children?)

---

Here I am lost. I don't know really what I can do next here, because I 
don't understand some ideas in code. e.g. searching of child - functions 
find_delta(), find_delta_children(). Calculation on line 618:


int next = (first+last) / 2;

I still don't understand. I didn't find description of this searching 
algorithm in tech. documentation but I didn't read all yet. However I 
think that source of problems could be somewhere in these two functions. 
When child is found, its real_type is set to parent's type in function 
resolve_delta() on the line 865 and then lasts wait for failure. I don't 
think that problem is in repository itself [1], but it is possible.


Any next ideas/hints or explanation of these functions? I began study 
source code and mechanisms of the git this week, so don't beat me yet 
please :-)


Regards,
Petr

[0] https://bugzilla.redhat.com/show_bug.cgi?id=1099919
[1] git clone https://code.google.com/p/mapsforge/ mapsforge.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


Re: [BUG] resolved deltas

2014-08-21 Thread Petr Stodulka



snip
Bug is reprodusible since git version 1.8.3.1 (may earlier 1.8.xx, but 
I don't test it) to actual upstream version.
This problem doesn't exists in version 1.7.xx - or more precisely is 
not reproducible. May this is reproducible
since commit 7218a215 - in this commit was added assert in file 
builtin/index-pack.c (actual line is 918), but I didn't test this.

Ok so this is reproducible since this commit because of assert().

Here I am lost. I don't know really what I can do next here, because I 
don't understand some ideas in code. e.g. searching of child - 
functions find_delta(), find_delta_children(). Calculation on line 618:


int next = (first+last) / 2;

I still don't understand. I didn't find description of this searching 
algorithm in tech. documentation but I didn't read all yet. However I 
think that source of problems could be somewhere in these two 
functions. When child is found, its real_type is set to parent's type 
in function resolve_delta() on the line 865 and then lasts wait for 
failure. I don't think that problem is in repository itself [1], but 
it is possible.
I read history of commits and my idea seems to be incorrect. It seems 
more like some error in repository itself. But I'd rather get opinion 
from someone who knows this code and ideas better.


Regards,
Petr


[0] https://bugzilla.redhat.com/show_bug.cgi?id=1099919
[1] git clone https://code.google.com/p/mapsforge/ mapsforge.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