Fsck configurations and cloning.

Fsck configurations and cloning.

[Ryne Everett]

I'm assuming fsck configurations are supposed to apply to clones but
I'm having no luck:

    $ git --version
    git version 2.7.2
    $ git config --get transfer.fsckobjects
    true
    $ git config --get fsck.badTimezone
    ignore
    $ cat $(git config --get fsck.skiplist)
    5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8
    $ git clone https://github.com/kennethreitz/requests.git
    Cloning into 'requests'...
    remote: Counting objects: 16904, done.
    error: object 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8:
badTimezone: invalid author/committer line - bad time zone
    fatal: Error in object
    fatal: index-pack failed

Am I doing something obviously wrong here?

Re: Fsck configurations and cloning.

[Jacob Keller]

On Sat, Mar 5, 2016 at 5:44 PM, Ryne Everett <ryneeverett@gmail.com> wrote:
> I'm assuming fsck configurations are supposed to apply to clones but
> I'm having no luck:
>
>     $ git --version
>     git version 2.7.2
>     $ git config --get transfer.fsckobjects
>     true
>     $ git config --get fsck.badTimezone
>     ignore
>     $ cat $(git config --get fsck.skiplist)
>     5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8
>     $ git clone https://github.com/kennethreitz/requests.git
>     Cloning into 'requests'...
>     remote: Counting objects: 16904, done.
>     error: object 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8:
> badTimezone: invalid author/committer line - bad time zone
>     fatal: Error in object
>     fatal: index-pack failed
>
> Am I doing something obviously wrong here?

You have to make sure the fields are set within a global git
configuration. Did you set this within a previous clone using "git
config"? If so you need to make sure you use "--global" option to
write it to your user configuration file so that it is persistent
across multiple repositories.

Not sure if that is what your problem was or not, it is unclear from
the information provided.

Thanks,
Jake

Re: Fsck configurations and cloning.

[Ryne Everett]

> You have to make sure the fields are set within a global git
> configuration. Did you set this within a previous clone using "git
> config"? If so you need to make sure you use "--global" option to
> write it to your user configuration file so that it is persistent
> across multiple repositories.

Yes, the settings are in my global config.

On Sun, Mar 6, 2016 at 12:18 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Sat, Mar 5, 2016 at 5:44 PM, Ryne Everett <ryneeverett@gmail.com> wrote:
>> I'm assuming fsck configurations are supposed to apply to clones but
>> I'm having no luck:
>>
>>     $ git --version
>>     git version 2.7.2
>>     $ git config --get transfer.fsckobjects
>>     true
>>     $ git config --get fsck.badTimezone
>>     ignore
>>     $ cat $(git config --get fsck.skiplist)
>>     5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8
>>     $ git clone https://github.com/kennethreitz/requests.git
>>     Cloning into 'requests'...
>>     remote: Counting objects: 16904, done.
>>     error: object 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8:
>> badTimezone: invalid author/committer line - bad time zone
>>     fatal: Error in object
>>     fatal: index-pack failed
>>
>> Am I doing something obviously wrong here?
>
> You have to make sure the fields are set within a global git
> configuration. Did you set this within a previous clone using "git
> config"? If so you need to make sure you use "--global" option to
> write it to your user configuration file so that it is persistent
> across multiple repositories.
>
> Not sure if that is what your problem was or not, it is unclear from
> the information provided.
>
> Thanks,
> Jake

Re: Fsck configurations and cloning.

[Jeff King]

On Sat, Mar 05, 2016 at 08:44:38PM -0500, Ryne Everett wrote:

> I'm assuming fsck configurations are supposed to apply to clones but
> I'm having no luck:
> 
>     $ git --version
>     git version 2.7.2
>     $ git config --get transfer.fsckobjects
>     true
>     $ git config --get fsck.badTimezone
>     ignore
>     $ cat $(git config --get fsck.skiplist)
>     5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8
>     $ git clone https://github.com/kennethreitz/requests.git
>     Cloning into 'requests'...
>     remote: Counting objects: 16904, done.
>     error: object 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8:
> badTimezone: invalid author/committer line - bad time zone
>     fatal: Error in object
>     fatal: index-pack failed
> 
> Am I doing something obviously wrong here?

I think there are two problems here.

The first is that fsck.skiplist takes a filename that contains a list of
sha1s, not the sha1s themselves. So it would be more like:

  echo 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8 >skiplist
  git -c fsck.skiplist="$(pwd)/skiplist" clone ...

The second is is that only "fsck" and "receive-pack" seem to have
learned about skiplist and other per-error config. There is no matching
fetch.fsck.* config to cover fetches.

With the patch below (which I don't think is suitable for inclusion;
it's a copy-paste from receive-pack, and should be re-factored to share
code; doing so might be a nice low-hanging-fruit project for somebody),
I can do:

   echo 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8 >skiplist
   git -c transfer.fsckobjects=true \
       -c fetch.fsck.skiplist="$(pwd)/skiplist" \
       clone https://github.com/kennethreitz/requests.git

---
diff --git a/fetch-pack.c b/fetch-pack.c
index f96f6df..19c83fa 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -16,6 +16,7 @@
 #include "prio-queue.h"
 #include "sha1-array.h"
 #include "sigchain.h"
+#include "fsck.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -24,6 +25,7 @@ static int prefer_ofs_delta = 1;
 static int no_done;
 static int fetch_fsck_objects = -1;
 static int transfer_fsck_objects = -1;
+static struct strbuf fsck_msg_types = STRBUF_INIT;
 static int agent_supported;
 static struct lock_file shallow_lock;
 static const char *alternate_shallow_file;
@@ -762,7 +764,7 @@ static int get_pack(struct fetch_pack_args *args,
 	    : transfer_fsck_objects >= 0
 	    ? transfer_fsck_objects
 	    : 0)
-		argv_array_push(&cmd.args, "--strict");
+		argv_array_pushf(&cmd.args, "--strict%s", fsck_msg_types.buf);
 
 	cmd.in = demux.out;
 	cmd.git_cmd = 1;
@@ -895,6 +897,32 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 	return ref;
 }
 
+static int fetch_config_cb(const char *var, const char *value, void *data)
+{
+
+	if (strcmp(var, "fetch.fsck.skiplist") == 0) {
+		const char *path;
+
+		if (git_config_pathname(&path, var, value))
+			return 1;
+		strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
+			    fsck_msg_types.len ? ',' : '=', path);
+		free((char *)path);
+		return 0;
+	}
+
+	if (skip_prefix(var, "fetch.fsck.", &var)) {
+		if (is_valid_msg_type(var, value))
+			strbuf_addf(&fsck_msg_types, "%c%s=%s",
+				    fsck_msg_types.len ? ',' : '=', var, value);
+		else
+			warning("Skipping unknown msg id '%s'", var);
+		return 0;
+	}
+
+	return 0;
+}
+
 static void fetch_pack_config(void)
 {
 	git_config_get_int("fetch.unpacklimit", &fetch_unpack_limit);
@@ -903,7 +931,7 @@ static void fetch_pack_config(void)
 	git_config_get_bool("fetch.fsckobjects", &fetch_fsck_objects);
 	git_config_get_bool("transfer.fsckobjects", &transfer_fsck_objects);
 
-	git_config(git_default_config, NULL);
+	git_config(fetch_config_cb, NULL);
 }
 
 static void fetch_pack_setup(void)

Re: Fsck configurations and cloning.

[Ryne Everett]

> I think there are two problems here.
>
> The first is that fsck.skiplist takes a filename that contains a list of
> sha1s, not the sha1s themselves. So it would be more like:
>
>   echo 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8 >skiplist
>   git -c fsck.skiplist="$(pwd)/skiplist" clone ...

I think I got this one right. Note the use of `cat` in my original post:

  $ cat $(git config --get fsck.skiplist)
    5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8

> The second is is that only "fsck" and "receive-pack" seem to have
> learned about skiplist and other per-error config. There is no matching
> fetch.fsck.* config to cover fetches.

Ah, well that sounds like the answer -- my original assumption was wrong.

> With the patch below (which I don't think is suitable for inclusion;
> it's a copy-paste from receive-pack, and should be re-factored to share
> code; doing so might be a nice low-hanging-fruit project for somebody),
> I can do:
>
>    echo 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8 >skiplist
>    git -c transfer.fsckobjects=true \
>        -c fetch.fsck.skiplist="$(pwd)/skiplist" \
>        clone https://github.com/kennethreitz/requests.git
>
> ---
> diff --git a/fetch-pack.c b/fetch-pack.c
> index f96f6df..19c83fa 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -16,6 +16,7 @@
>  #include "prio-queue.h"
>  #include "sha1-array.h"
>  #include "sigchain.h"
> +#include "fsck.h"
>
>  static int transfer_unpack_limit = -1;
>  static int fetch_unpack_limit = -1;
> @@ -24,6 +25,7 @@ static int prefer_ofs_delta = 1;
>  static int no_done;
>  static int fetch_fsck_objects = -1;
>  static int transfer_fsck_objects = -1;
> +static struct strbuf fsck_msg_types = STRBUF_INIT;
>  static int agent_supported;
>  static struct lock_file shallow_lock;
>  static const char *alternate_shallow_file;
> @@ -762,7 +764,7 @@ static int get_pack(struct fetch_pack_args *args,
>             : transfer_fsck_objects >= 0
>             ? transfer_fsck_objects
>             : 0)
> -               argv_array_push(&cmd.args, "--strict");
> +               argv_array_pushf(&cmd.args, "--strict%s", fsck_msg_types.buf);
>
>         cmd.in = demux.out;
>         cmd.git_cmd = 1;
> @@ -895,6 +897,32 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
>         return ref;
>  }
>
> +static int fetch_config_cb(const char *var, const char *value, void *data)
> +{
> +
> +       if (strcmp(var, "fetch.fsck.skiplist") == 0) {
> +               const char *path;
> +
> +               if (git_config_pathname(&path, var, value))
> +                       return 1;
> +               strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
> +                           fsck_msg_types.len ? ',' : '=', path);
> +               free((char *)path);
> +               return 0;
> +       }
> +
> +       if (skip_prefix(var, "fetch.fsck.", &var)) {
> +               if (is_valid_msg_type(var, value))
> +                       strbuf_addf(&fsck_msg_types, "%c%s=%s",
> +                                   fsck_msg_types.len ? ',' : '=', var, value);
> +               else
> +                       warning("Skipping unknown msg id '%s'", var);
> +               return 0;
> +       }
> +
> +       return 0;
> +}
> +
>  static void fetch_pack_config(void)
>  {
>         git_config_get_int("fetch.unpacklimit", &fetch_unpack_limit);
> @@ -903,7 +931,7 @@ static void fetch_pack_config(void)
>         git_config_get_bool("fetch.fsckobjects", &fetch_fsck_objects);
>         git_config_get_bool("transfer.fsckobjects", &transfer_fsck_objects);
>
> -       git_config(git_default_config, NULL);
> +       git_config(fetch_config_cb, NULL);
>  }
>
>  static void fetch_pack_setup(void)

Re: Fsck configurations and cloning.

[Jeff King]

On Sun, Mar 06, 2016 at 10:38:15AM -0500, Ryne Everett wrote:

> > I think there are two problems here.
> >
> > The first is that fsck.skiplist takes a filename that contains a list of
> > sha1s, not the sha1s themselves. So it would be more like:
> >
> >   echo 5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8 >skiplist
> >   git -c fsck.skiplist="$(pwd)/skiplist" clone ...
> 
> I think I got this one right. Note the use of `cat` in my original post:
> 
>   $ cat $(git config --get fsck.skiplist)
>     5e6ecdad9f69b1ff789a17733b8edc6fd7091bd8

Oh, indeed, I missed that completely.

> > The second is is that only "fsck" and "receive-pack" seem to have
> > learned about skiplist and other per-error config. There is no matching
> > fetch.fsck.* config to cover fetches.
> 
> Ah, well that sounds like the answer -- my original assumption was wrong.

Yeah. I was kind of surprised to find this was the case, given that we
_do_ have fetch.fsckObjects.

-Peff