{"thread":{"id":"43307","subject":"Re: [PATCH 4/5] allow deepening of a shallow repository","startedAt":"2006-10-30T19:09:53Z","lastAt":"2006-11-18T12:58:21Z","messageCount":7,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"295170","messageId":"Pine.LNX.4.63.0610302009390.26682@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43307","inReplyTo":null,"subject":"[PATCH 4/5] allow deepening of a shallow repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-10-30T19:09:53Z","receivedAt":"2006-10-30T19:09:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nNow, by saying \"git fetch -depth <n> <repo>\" you can deepen\na shallow repository.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n commit.c      |   13 +++++++++++++\n commit.h      |    3 ++-\n fetch-pack.c  |   22 ++++++++++++++++------\n git-fetch.sh  |   12 +++++++++++-\n shallow.c     |    8 ++++++--\n upload-pack.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++-----------\n 6 files changed, 94 insertions(+), 21 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex bffa278..d5103cd 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -255,6 +255,19 @@ int write_shallow_commits(int fd, int us\n \treturn count;\n }\n \n+int unregister_shallow(const unsigned char *sha1)\n+{\n+\tint pos = commit_graft_pos(sha1);\n+\tif (pos < 0)\n+\t\treturn -1;\n+\tif (pos + 1 < commit_graft_nr)\n+\t\tmemcpy(commit_graft + pos, commit_graft + pos + 1,\n+\t\t\t\tsizeof(struct commit_graft *)\n+\t\t\t\t* (commit_graft_nr - pos - 1));\n+\tcommit_graft_nr--;\n+\treturn 0;\n+}\n+\n int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size)\n {\n \tchar *tail = buffer;\ndiff --git a/commit.h b/commit.h\nindex c559510..e9e158f 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -108,9 +108,10 @@ int read_graft_file(const char *graft_fi\n extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup);\n \n extern int register_shallow(const unsigned char *sha1);\n+extern int unregister_shallow(const unsigned char *sha1);\n extern int write_shallow_commits(int fd, int use_pack_protocol);\n extern int is_repository_shallow();\n extern struct commit_list *get_shallow_commits(struct object_array *heads,\n-\t\tint depth);\n+\t\tint depth, int shallow_flag, int not_shallow_flag);\n \n #endif /* COMMIT_H */\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 9619d6e..a82a5ba 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -199,7 +199,17 @@ static int find_common(int fd[2], unsign\n \t\t\t\tif (lookup_object(sha1))\n \t\t\t\t\tcontinue;\n \t\t\t\tregister_shallow(sha1);\n-\t\t\t}\n+\t\t\t}  else if (!strncmp(\"unshallow \", line, 10)) {\n+\t\t\t\tif (get_sha1_hex(line + 10, sha1))\n+\t\t\t\t\tdie(\"invalid unshallow line: %s\", line);\n+\t\t\t\tif (!lookup_object(sha1))\n+\t\t\t\t\tdie(\"object not found: %s\", line);\n+\t\t\t\t/* make sure that it is parsed as shallow */\n+\t\t\t\tparse_object(sha1);\n+\t\t\t\tif (unregister_shallow(sha1))\n+\t\t\t\t\tdie(\"no shallow found: %s\", line);\n+\t\t\t} else\n+\t\t\t\tdie(\"expected shallow/unshallow, got %s\", line);\n \t\t}\n \t}\n \n@@ -388,9 +398,11 @@ static int everything_local(struct ref *\n \t\t}\n \t}\n \n-\tfor_each_ref(mark_complete, NULL);\n-\tif (cutoff)\n-\t\tmark_recent_complete_commits(cutoff);\n+\tif (!depth) {\n+\t\tfor_each_ref(mark_complete, NULL);\n+\t\tif (cutoff)\n+\t\t\tmark_recent_complete_commits(cutoff);\n+\t}\n \n \t/*\n \t * Mark all complete remote refs as common refs.\n@@ -550,8 +562,6 @@ int main(int argc, char **argv)\n \t}\n \tif (!dest)\n \t\tusage(fetch_pack_usage);\n-\tif (is_repository_shallow() && depth > 0)\n-\t\tdie(\"Deepening of a shallow repository not yet supported!\");\n \tpid = git_connect(fd, dest, exec);\n \tif (pid < 0)\n \t\treturn 1;\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex 539dff6..2a4b7a0 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -21,6 +21,7 @@ update_head_ok=\n exec=\n upload_pack=\n keep=--thin\n+depth=\n while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n@@ -56,6 +57,13 @@ do\n \t--reflog-action=*)\n \t\trloga=`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\n \t\t;;\n+\t--depth=*)\n+\t\tdepth=\"--depth=`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\"\n+\t\t;;\n+\t--depth)\n+\t\tshift\n+\t\tdepth=\"--depth=$1\"\n+\t\t;;\n \t-*)\n \t\tusage\n \t\t;;\n@@ -296,6 +304,7 @@ fetch_main () {\n       # There are transports that can fetch only one head at a time...\n       case \"$remote\" in\n       http://* | https://* | ftp://*)\n+\t  test -n \"$depth\" && die \"shallow clone with http not supported\"\n \t  proto=`expr \"$remote\" : '\\([^:]*\\):'`\n \t  if [ -n \"$GIT_SSL_NO_VERIFY\" ]; then\n \t      curl_extra_args=\"-k\"\n@@ -324,6 +333,7 @@ fetch_main () {\n \t  git-http-fetch -v -a \"$head\" \"$remote/\" || exit\n \t  ;;\n       rsync://*)\n+\t  test -n \"$depth\" && die \"shallow clone with rsync not supported\"\n \t  TMP_HEAD=\"$GIT_DIR/TMP_HEAD\"\n \t  rsync -L -q \"$remote/$remote_name\" \"$TMP_HEAD\" || exit 1\n \t  head=$(git-rev-parse --verify TMP_HEAD)\n@@ -370,7 +380,7 @@ fetch_main () {\n     ( : subshell because we muck with IFS\n       IFS=\" \t$LF\"\n       (\n-\t  git-fetch-pack $exec $keep \"$remote\" $rref || echo failed \"$remote\"\n+\t  git-fetch-pack $exec $keep $depth \"$remote\" $rref || echo failed \"$remote\"\n       ) |\n       while read sha1 remote_name\n       do\ndiff --git a/shallow.c b/shallow.c\nindex 3cf2127..58a7b20 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -41,7 +41,8 @@ int is_repository_shallow()\n \treturn is_shallow;\n }\n \n-struct commit_list *get_shallow_commits(struct object_array *heads, int depth)\n+struct commit_list *get_shallow_commits(struct object_array *heads, int depth,\n+\t\tint shallow_flag, int not_shallow_flag)\n {\n \tint i = 0, cur_depth = 0;\n \tstruct commit_list *result = NULL;\n@@ -67,6 +68,7 @@ struct commit_list *get_shallow_commits(\n \t\t\t}\n \t\t}\n \t\tparse_commit(commit);\n+\t\tcommit->object.flags |= not_shallow_flag;\n \t\tcur_depth++;\n \t\tfor (p = commit->parents, commit = NULL; p; p = p->next) {\n \t\t\tif (!p->item->util) {\n@@ -87,8 +89,10 @@ struct commit_list *get_shallow_commits(\n \t\t\t\t\tcommit = p->item;\n \t\t\t\t\tcur_depth = *(int *)commit->util;\n \t\t\t\t}\n-\t\t\t} else\n+\t\t\t} else {\n \t\t\t\tcommit_list_insert(p->item, &result);\n+\t\t\t\tp->item->object.flags |= shallow_flag;\n+\t\t\t}\n \t\t}\n \t}\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex ebe1e5a..162dd34 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -134,6 +134,7 @@ static void create_pack_file(void)\n \t\t} else {\n \t\t\tfor (i = 0; i < want_obj.nr; i++) {\n \t\t\t\tstruct object *o = want_obj.objects[i].item;\n+\t\t\t\to->flags &= ~UNINTERESTING;\n \t\t\t\tadd_pending_object(&revs, o, NULL);\n \t\t\t}\n \t\t\tfor (i = 0; i < have_obj.nr; i++) {\n@@ -490,6 +491,9 @@ static void receive_needs(void)\n \tstatic char line[1000];\n \tint len, depth = 0;\n \n+#define SHALLOW (1u<<8)\n+#define NOT_SHALLOW (1u<<9)\n+#define CLIENT_SHALLOW (1u<<10)\n \tfor (;;) {\n \t\tstruct object *o;\n \t\tunsigned char sha1_buf[20];\n@@ -501,16 +505,19 @@ static void receive_needs(void)\n \t\tif (!strncmp(\"shallow \", line, 8)) {\n \t\t\tunsigned char sha1[20];\n \t\t\tstruct object *object;\n+\t\t\tuse_thin_pack = 0;\n \t\t\tif (get_sha1(line + 8, sha1))\n \t\t\t\tdie(\"invalid shallow line: %s\", line);\n \t\t\tobject = parse_object(sha1);\n \t\t\tif (!object)\n \t\t\t\tdie(\"did not find object for %s\", line);\n+\t\t\tobject->flags |= CLIENT_SHALLOW;\n \t\t\tadd_object_array(object, NULL, &shallows);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strncmp(\"deepen \", line, 7)) {\n \t\t\tchar *end;\n+\t\t\tuse_thin_pack = 0;\n \t\t\tdepth = strtol(line + 7, &end, 0);\n \t\t\tif (end == line + 7 || depth <= 0)\n \t\t\t\tdie(\"Invalid deepen: %s\", line);\n@@ -547,23 +554,51 @@ static void receive_needs(void)\n \t\t\tadd_object_array(o, NULL, &want_obj);\n \t\t}\n \t}\n+\tif (depth == 0 && shallows.nr == 0)\n+\t\treturn;\n \tif (depth > 0) {\n \t\tstruct commit_list *result, *backup;\n-\t\tif (shallows.nr > 0)\n-\t\t\tdie(\"Deepening a shallow repository not yet supported\");\n-\t\tbackup = result = get_shallow_commits(&want_obj, depth);\n+\t\tint i;\n+\t\tbackup = result = get_shallow_commits(&want_obj, depth,\n+\t\t\tSHALLOW, NOT_SHALLOW);\n \t\twhile (result) {\n-\t\t\tpacket_write(1, \"shallow %s\",\n-\t\t\t\t\tsha1_to_hex(result->item->object.sha1));\n+\t\t\tstruct object *object = &result->item->object;\n+\t\t\tif (!(object->flags & CLIENT_SHALLOW)) {\n+\t\t\t\tpacket_write(1, \"shallow %s\",\n+\t\t\t\t\t\tsha1_to_hex(object->sha1));\n+\t\t\t\tregister_shallow(object->sha1);\n+\t\t\t}\n \t\t\tresult = result->next;\n \t\t}\n \t\tfree_commit_list(backup);\n-\t}\n-\tif (shallows.nr > 0) {\n-\t\tint i;\n-\t\tfor (i = 0; i < shallows.nr; i++)\n-\t\t\tregister_shallow(shallows.objects[i].item->sha1);\n-\t}\n+\t\tfor (i = 0; i < shallows.nr; i++) {\n+\t\t\tstruct object *object = shallows.objects[i].item;\n+\t\t\tif (object->flags & NOT_SHALLOW) {\n+\t\t\t\tstruct commit_list *parents;\n+\t\t\t\tpacket_write(1, \"unshallow %s\",\n+\t\t\t\t\tsha1_to_hex(object->sha1));\n+\t\t\t\tobject->flags &= ~CLIENT_SHALLOW;\n+\t\t\t\t/* make sure the real parents are parsed */\n+\t\t\t\tunregister_shallow(object->sha1);\n+\t\t\t\tparse_commit((struct commit *)object);\n+\t\t\t\tparents = ((struct commit *)object)->parents;\n+\t\t\t\twhile (parents) {\n+\t\t\t\t\tadd_object_array(&parents->item->object,\n+\t\t\t\t\t\t\tNULL, &want_obj);\n+\t\t\t\t\tparents = parents->next;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\t/* make sure commit traversal conforms to client */\n+\t\t\tregister_shallow(object->sha1);\n+\t\t}\n+\t\tpacket_flush(1);\n+\t} else\n+\t\tif (shallows.nr > 0) {\n+\t\t\tint i;\n+\t\t\tfor (i = 0; i < shallows.nr; i++)\n+\t\t\t\tregister_shallow(shallows.objects[i].item->sha1);\n+\t\t}\n+\tfree(shallows.objects);\n }\n \n static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n-- \n1.4.3.3.gca42\n"},{"id":"295015","messageId":"7vy7qeze0q.fsf@assigned-by-dhcp.cox.net","threadId":"43307","inReplyTo":"Pine.LNX.4.63.0610302009390.26682@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 4/5] allow deepening of a shallow repository","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-14T07:24:53Z","receivedAt":"2006-11-14T07:24:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Now, by saying \"git fetch -depth <n> <repo>\" you can deepen\n> a shallow repository.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  commit.c      |   13 +++++++++++++\n>  commit.h      |    3 ++-\n>  fetch-pack.c  |   22 ++++++++++++++++------\n>  git-fetch.sh  |   12 +++++++++++-\n>  shallow.c     |    8 ++++++--\n>  upload-pack.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++-----------\n> diff --git a/upload-pack.c b/upload-pack.c\n> index ebe1e5a..162dd34 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -134,6 +134,7 @@ static void create_pack_file(void)\n>  \t\t} else {\n>  \t\t\tfor (i = 0; i < want_obj.nr; i++) {\n>  \t\t\t\tstruct object *o = want_obj.objects[i].item;\n> +\t\t\t\to->flags &= ~UNINTERESTING;\n>  \t\t\t\tadd_pending_object(&revs, o, NULL);\n>  \t\t\t}\n>  \t\t\tfor (i = 0; i < have_obj.nr; i++) {\n\nI am puzzled why this is needed in this series.  In other words,\ndo we have a bug in the current upload-pack that does not have\nshallow already by not clearing UNINTERESTING bit?  In yet other\nwords, I haven't figured out which part of the shallow series\nmakes it necessary to clear UNINTERESTING bit from wanted\nobject.\n\n> @@ -547,23 +554,51 @@ static void receive_needs(void)\n>...\n> +\t\tfor (i = 0; i < shallows.nr; i++) {\n> +\t\t\tstruct object *object = shallows.objects[i].item;\n> +\t\t\tif (object->flags & NOT_SHALLOW) {\n> +\t\t\t\tstruct commit_list *parents;\n> +\t\t\t\tpacket_write(1, \"unshallow %s\",\n> +\t\t\t\t\tsha1_to_hex(object->sha1));\n> +\t\t\t\tobject->flags &= ~CLIENT_SHALLOW;\n> +\t\t\t\t/* make sure the real parents are parsed */\n> +\t\t\t\tunregister_shallow(object->sha1);\n> +\t\t\t\tparse_commit((struct commit *)object);\n> +\t\t\t\tparents = ((struct commit *)object)->parents;\n> +\t\t\t\twhile (parents) {\n> +\t\t\t\t\tadd_object_array(&parents->item->object,\n> +\t\t\t\t\t\t\tNULL, &want_obj);\n> +\t\t\t\t\tparents = parents->next;\n> +\t\t\t\t}\n> +\t\t\t}\n\nI doubt unregister_shallow() is enough to ensure that the next\nparse_commit() re-parses to recover its parents.  parse_commit()\nsays \"if (item->object.parsed) return 0\" upfront.  Don't you\nneed to do:\n\n\tobject->parsed = 0;\n\nbefore parse_commit()?\n"},{"id":"297110","messageId":"Pine.LNX.4.63.0611141150010.13772@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43307","inReplyTo":"7vy7qeze0q.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/5] allow deepening of a shallow repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-11-14T11:03:39Z","receivedAt":"2006-11-14T11:03:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 13 Nov 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > Now, by saying \"git fetch -depth <n> <repo>\" you can deepen\n> > a shallow repository.\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  commit.c      |   13 +++++++++++++\n> >  commit.h      |    3 ++-\n> >  fetch-pack.c  |   22 ++++++++++++++++------\n> >  git-fetch.sh  |   12 +++++++++++-\n> >  shallow.c     |    8 ++++++--\n> >  upload-pack.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++-----------\n> > diff --git a/upload-pack.c b/upload-pack.c\n> > index ebe1e5a..162dd34 100644\n> > --- a/upload-pack.c\n> > +++ b/upload-pack.c\n> > @@ -134,6 +134,7 @@ static void create_pack_file(void)\n> >  \t\t} else {\n> >  \t\t\tfor (i = 0; i < want_obj.nr; i++) {\n> >  \t\t\t\tstruct object *o = want_obj.objects[i].item;\n> > +\t\t\t\to->flags &= ~UNINTERESTING;\n> >  \t\t\t\tadd_pending_object(&revs, o, NULL);\n> >  \t\t\t}\n> >  \t\t\tfor (i = 0; i < have_obj.nr; i++) {\n> \n> I am puzzled why this is needed in this series.  In other words,\n> do we have a bug in the current upload-pack that does not have\n> shallow already by not clearing UNINTERESTING bit?  In yet other\n> words, I haven't figured out which part of the shallow series\n> makes it necessary to clear UNINTERESTING bit from wanted\n> object.\n\nIIRC (has been a long time, hasn't it?) the test failed without that line, \nand I was too lazy to investigate why. But ICRI ;-)\n\n> > @@ -547,23 +554,51 @@ static void receive_needs(void)\n> >...\n> > +\t\tfor (i = 0; i < shallows.nr; i++) {\n> > +\t\t\tstruct object *object = shallows.objects[i].item;\n> > +\t\t\tif (object->flags & NOT_SHALLOW) {\n> > +\t\t\t\tstruct commit_list *parents;\n> > +\t\t\t\tpacket_write(1, \"unshallow %s\",\n> > +\t\t\t\t\tsha1_to_hex(object->sha1));\n> > +\t\t\t\tobject->flags &= ~CLIENT_SHALLOW;\n> > +\t\t\t\t/* make sure the real parents are parsed */\n> > +\t\t\t\tunregister_shallow(object->sha1);\n> > +\t\t\t\tparse_commit((struct commit *)object);\n> > +\t\t\t\tparents = ((struct commit *)object)->parents;\n> > +\t\t\t\twhile (parents) {\n> > +\t\t\t\t\tadd_object_array(&parents->item->object,\n> > +\t\t\t\t\t\t\tNULL, &want_obj);\n> > +\t\t\t\t\tparents = parents->next;\n> > +\t\t\t\t}\n> > +\t\t\t}\n> \n> I doubt unregister_shallow() is enough to ensure that the next\n> parse_commit() re-parses to recover its parents.  parse_commit()\n> says \"if (item->object.parsed) return 0\" upfront.  Don't you\n> need to do:\n> \n> \tobject->parsed = 0;\n> \n> before parse_commit()?\n\nYes. Somehow, an important part of unregister_shallow() went missing (yet \nanother proof that my poor-man's-StGit does not always work). I think that \nthe \"object->parsed = 0;\" should go into unregister_shallow() like this:\n\ndiff --git a/commit.c b/commit.c\nindex d5103cd..4451376 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -258,6 +258,7 @@ int write_shallow_commits(int fd, int us\n int unregister_shallow(const unsigned char *sha1)\n {\n \tint pos = commit_graft_pos(sha1);\n+\tstruct commit *commit = lookup_commit(sha1);\n \tif (pos < 0)\n \t\treturn -1;\n \tif (pos + 1 < commit_graft_nr)\n@@ -265,6 +266,8 @@ int unregister_shallow(const unsigned ch\n \t\t\t\tsizeof(struct commit_graft *)\n \t\t\t\t* (commit_graft_nr - pos - 1));\n \tcommit_graft_nr--;\n+\tif (commit)\n+\t\tcommit->object.parsed = 0;\n \treturn 0;\n }\n \nCiao,\nDscho\n"},{"id":"298537","messageId":"7vy7qaqw12.fsf@assigned-by-dhcp.cox.net","threadId":"43307","inReplyTo":"Pine.LNX.4.63.0611141150010.13772@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 4/5] allow deepening of a shallow repository","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-17T09:09:45Z","receivedAt":"2006-11-17T09:09:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Yes. Somehow, an important part of unregister_shallow() went missing (yet \n> another proof that my poor-man's-StGit does not always work). I think that \n> the \"object->parsed = 0;\" should go into unregister_shallow() like this:\n\nThat does not fly so well.  Your fetch-pack.c does this in\nfind_common() and dropping the parsed flag inside unregister\ncauses it to be parsed again later with its true parents, which\ndefeats what the commented part of the code wanted to do.\n\n\tif (!lookup_object(sha1))\n\t\tdie(\"object not found: %s\", line);\n\t/* make sure that it is parsed as shallow */\n\tparse_object(sha1);\n\tif (unregister_shallow(sha1))\n\t\tdie(\"no shallow found: %s\", line);\n\tcontinue;\n\nAlthough I am reasonably sure we can eventually make it work, it\nis very subtle and fragile -- somebody touching this code can\neasily break it.\n.\n"},{"id":"296141","messageId":"Pine.LNX.4.63.0611171050440.13772@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43307","inReplyTo":"7vy7qaqw12.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/5] allow deepening of a shallow repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-11-17T09:52:54Z","receivedAt":"2006-11-17T09:52:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 17 Nov 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > Yes. Somehow, an important part of unregister_shallow() went missing (yet \n> > another proof that my poor-man's-StGit does not always work). I think that \n> > the \"object->parsed = 0;\" should go into unregister_shallow() like this:\n> \n> That does not fly so well.  Your fetch-pack.c does this in\n> find_common() and dropping the parsed flag inside unregister\n> causes it to be parsed again later with its true parents, which\n> defeats what the commented part of the code wanted to do.\n> \n> \tif (!lookup_object(sha1))\n> \t\tdie(\"object not found: %s\", line);\n> \t/* make sure that it is parsed as shallow */\n> \tparse_object(sha1);\n> \tif (unregister_shallow(sha1))\n> \t\tdie(\"no shallow found: %s\", line);\n> \tcontinue;\n\nAh! _That_ was the reason I removed it from unregister_shallow().\n\n> Although I am reasonably sure we can eventually make it work, it\n> is very subtle and fragile -- somebody touching this code can\n> easily break it.\n\nI fully agree. Even the OA did not understand the code fully ;-)\n\nHow about adding \"int force_reparse\" to the signature of \nunregister_shallow()? (Just like we added \"int cleanup\" to \nget_merge_bases() to avoid pilot errors.)\n\nCiao,\nDscho\n"},{"id":"298142","messageId":"7vu00ymhk1.fsf@assigned-by-dhcp.cox.net","threadId":"43307","inReplyTo":"Pine.LNX.4.63.0611171050440.13772@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 4/5] allow deepening of a shallow repository","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-17T11:35:58Z","receivedAt":"2006-11-17T11:35:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Although I am reasonably sure we can eventually make it work, it\n>> is very subtle and fragile -- somebody touching this code can\n>> easily break it.\n>\n> I fully agree. Even the OA did not understand the code fully ;-)\n>\n> How about adding \"int force_reparse\" to the signature of \n> unregister_shallow()? (Just like we added \"int cleanup\" to \n> get_merge_bases() to avoid pilot errors.)\n\nI do not think that's where its fragility lies.  It is that any\nrandom place can later call parse_object() on the commit, after\nyou elaborately pre-parsed it with shallow so that it appears to\nhave fewer parents, with the expectation that nobody ever would\nclear the parsed bit and cause it to be reparsed again.  With\nthat assumption, find_common() manipulated the shallow entry\nafter setting that scheme up.  A mechanism to prevent the commit\nfrom getting re-parsed would have made it more robust.\n"},{"id":"294291","messageId":"Pine.LNX.4.63.0611181356300.13772@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43307","inReplyTo":"7vu00ymhk1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/5] allow deepening of a shallow repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-11-18T12:58:21Z","receivedAt":"2006-11-18T12:58:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 17 Nov 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> Although I am reasonably sure we can eventually make it work, it\n> >> is very subtle and fragile -- somebody touching this code can\n> >> easily break it.\n> >\n> > I fully agree. Even the OA did not understand the code fully ;-)\n> >\n> > How about adding \"int force_reparse\" to the signature of \n> > unregister_shallow()? (Just like we added \"int cleanup\" to \n> > get_merge_bases() to avoid pilot errors.)\n> \n> I do not think that's where its fragility lies.  It is that any\n> random place can later call parse_object() on the commit, after\n> you elaborately pre-parsed it with shallow so that it appears to\n> have fewer parents, with the expectation that nobody ever would\n> clear the parsed bit and cause it to be reparsed again.  With\n> that assumption, find_common() manipulated the shallow entry\n> after setting that scheme up.  A mechanism to prevent the commit\n> >from getting re-parsed would have made it more robust.\n\nHow about putting yet more meaning into nr_parents: if it is negative, it \nis a shallow commit, but write_shallow_commits() only writes the SHA1 if \nnr_parents is -1. Hmm?\n\nCiao,\nDscho\n"}]}