{"thread":{"id":"7636","subject":"[PATCH 1/2] Teach git list-objects logic not to follow gitlinks","startedAt":"2007-04-13T04:03:39Z","lastAt":"2007-04-13T04:54:34Z","messageCount":5,"participants":["Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"39258","messageId":"Pine.LNX.4.64.0704122102320.4061@woody.linux-foundation.org","threadId":"7636","inReplyTo":null,"subject":"[PATCH 1/2] Teach git list-objects logic not to follow gitlinks","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-13T04:03:39Z","receivedAt":"2007-04-13T04:03:39Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis allows us to pack superprojects and thus clone them (but not yet\ncheck them out on the receiving side - that's the next patch)\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n\nMost of this diff is a comment saying why there is nothing to do ;)\n\n list-objects.c |   34 ++++++++++++++++++++++++++++++++++\n 1 files changed, 34 insertions(+), 0 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 2ba2c95..310f8d3 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -25,6 +25,37 @@ static void process_blob(struct rev_info *revs,\n \tadd_object(obj, p, path, name);\n }\n \n+/*\n+ * Processing a gitlink entry currently does nothing, since\n+ * we do not recurse into the subproject.\n+ *\n+ * We *could* eventually add a flag that actually does that,\n+ * which would involve:\n+ *  - is the subproject actually checked out?\n+ *  - if so, see if the subproject has already been added\n+ *    to the alternates list, and add it if not.\n+ *  - process the commit (or tag) the gitlink points to\n+ *    recursively.\n+ *\n+ * However, it's unclear whether there is really ever any\n+ * reason to see superprojects and subprojects as such a\n+ * \"unified\" object pool (potentially resulting in a totally\n+ * humongous pack - avoiding which was the whole point of\n+ * having gitlinks in the first place!).\n+ *\n+ * So for now, there is just a note that we *could* follow\n+ * the link, and how to do it. Whether it necessarily makes\n+ * any sense what-so-ever to ever do that is another issue.\n+ */\n+static void process_gitlink(struct rev_info *revs,\n+\t\t\t    const unsigned char *sha1,\n+\t\t\t    struct object_array *p,\n+\t\t\t    struct name_path *path,\n+\t\t\t    const char *name)\n+{\n+\t/* Nothing to do */\n+}\n+\n static void process_tree(struct rev_info *revs,\n \t\t\t struct tree *tree,\n \t\t\t struct object_array *p,\n@@ -56,6 +87,9 @@ static void process_tree(struct rev_info *revs,\n \t\t\tprocess_tree(revs,\n \t\t\t\t     lookup_tree(entry.sha1),\n \t\t\t\t     p, &me, entry.path);\n+\t\telse if (S_ISDIRLNK(entry.mode))\n+\t\t\tprocess_gitlink(revs, entry.sha1,\n+\t\t\t\t\tp, &me, entry.path);\n \t\telse\n \t\t\tprocess_blob(revs,\n \t\t\t\t     lookup_blob(entry.sha1),\n"},{"id":"39259","messageId":"Pine.LNX.4.64.0704122104030.4061@woody.linux-foundation.org","threadId":"7636","inReplyTo":"Pine.LNX.4.64.0704122102320.4061@woody.linux-foundation.org","subject":"[PATCH 2/2] Teach \"git-read-tree -u\" to check out submodules as a directory","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-13T04:08:52Z","receivedAt":"2007-04-13T04:08:52Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis actually allows us to check out a supermodule after cloning, although \nthe submodules will obviously not be checked out, and will just be an \nempty subdirectory.\n\n[ Side note: this also shows that we currently don't correctly handle\n  such subprojects that aren't checked out correctly yet.  They should\n  always show up as not being modified, but failing to resolve the\n  gitlink HEAD does not properly trigger the \"not modified\" logic in all \n  places it needs to..\n\n  So more work to be done, but that's a separate issue, unrelated to\n  the action of checking out the superproject. ]\n\nThe bulk of this patch is simply because we need to check the type of the \nindex entry *before* we try to read the object it points to, and that \nmeant that the code needed some re-organization. So I moved some of the \ncode in common to both symlinks and files to be a trivial helper function.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n entry.c |   42 +++++++++++++++++++++++++++++-------------\n 1 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex d72f811..9545e89 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -62,26 +62,33 @@ static int create_file(const char *path, unsigned int mode)\n \treturn open(path, O_WRONLY | O_CREAT | O_EXCL, mode);\n }\n \n+static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned long *size)\n+{\n+\tenum object_type type;\n+\tvoid *new = read_sha1_file(ce->sha1, &type, size);\n+\n+\tif (new) {\n+\t\tif (type == OBJ_BLOB)\n+\t\t\treturn new;\n+\t\tfree(new);\n+\t}\n+\treturn NULL;\n+}\n+\n static int write_entry(struct cache_entry *ce, char *path, struct checkout *state, int to_tempfile)\n {\n \tint fd;\n-\tvoid *new;\n-\tunsigned long size;\n \tlong wrote;\n-\tenum object_type type;\n \n-\tnew = read_sha1_file(ce->sha1, &type, &size);\n-\tif (!new || type != OBJ_BLOB) {\n-\t\tif (new)\n-\t\t\tfree(new);\n-\t\treturn error(\"git-checkout-index: unable to read sha1 file of %s (%s)\",\n-\t\t\tpath, sha1_to_hex(ce->sha1));\n-\t}\n \tswitch (ntohl(ce->ce_mode) & S_IFMT) {\n-\t\tchar *buf;\n-\t\tunsigned long nsize;\n+\t\tchar *buf, *new;\n+\t\tunsigned long size, nsize;\n \n \tcase S_IFREG:\n+\t\tnew = read_blob_entry(ce, path, &size);\n+\t\tif (!new)\n+\t\t\treturn error(\"git-checkout-index: unable to read sha1 file of %s (%s)\",\n+\t\t\t\tpath, sha1_to_hex(ce->sha1));\n \t\tif (to_tempfile) {\n \t\t\tstrcpy(path, \".merge_file_XXXXXX\");\n \t\t\tfd = mkstemp(path);\n@@ -111,6 +118,10 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n \t\t\treturn error(\"git-checkout-index: unable to write file %s\", path);\n \t\tbreak;\n \tcase S_IFLNK:\n+\t\tnew = read_blob_entry(ce, path, &size);\n+\t\tif (!new)\n+\t\t\treturn error(\"git-checkout-index: unable to read sha1 file of %s (%s)\",\n+\t\t\t\tpath, sha1_to_hex(ce->sha1));\n \t\tif (to_tempfile || !has_symlinks) {\n \t\t\tif (to_tempfile) {\n \t\t\t\tstrcpy(path, \".merge_link_XXXXXX\");\n@@ -136,8 +147,13 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n \t\t\t\t\t\t \"symlink %s (%s)\", path, strerror(errno));\n \t\t}\n \t\tbreak;\n+\tcase S_IFDIRLNK:\n+\t\tif (to_tempfile)\n+\t\t\treturn error(\"git-checkout-index: cannot create temporary subproject %s\", path);\n+\t\tif (mkdir(path, 0777) < 0)\n+\t\t\treturn error(\"git-checkout-index: cannot create subproject directory %s\", path);\n+\t\tbreak;\n \tdefault:\n-\t\tfree(new);\n \t\treturn error(\"git-checkout-index: unknown file mode for %s\", path);\n \t}\n \n"},{"id":"39260","messageId":"Pine.LNX.4.64.0704122109070.4061@woody.linux-foundation.org","threadId":"7636","inReplyTo":"Pine.LNX.4.64.0704122104030.4061@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Teach \"git-read-tree -u\" to check out submodules as a directory","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-13T04:15:16Z","receivedAt":"2007-04-13T04:15:16Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 12 Apr 2007, Linus Torvalds wrote:\n> \n> [ Side note: this also shows that we currently don't correctly handle\n>   such subprojects that aren't checked out correctly yet.  They should\n>   always show up as not being modified, but failing to resolve the\n>   gitlink HEAD does not properly trigger the \"not modified\" logic in all \n>   places it needs to..\n> \n>   So more work to be done, but that's a separate issue, unrelated to\n>   the action of checking out the superproject. ]\n\nApart from this issue, and the fact that we don't actually diff \nsubprojects at all right now (even to the point of saying \"subproject XyZ \nhas changed from commit X to commit Y\" - the raw format knows to say that, \nbut the patch format does not), I think the \"really core plumbing\" is \nactually mostly done with this series.\n\nSo there's certainly some loose ends to tie up and some missing \nfunctionality even at the core level, but I suspect this is getting to the \npoint where a big chunk of the remaining part is actually to teach the \nupper layers (like \"fetch\", \"checkout\", \"merge\" etc) to actually iterate \nover subprojects using the theoretical higher-level \".subprojects\" file, \nand do the plumbing-level operations on those.\n\nOf course, actually trying to start to *use* these things is bound to find \ntons and tons of issues, but I still feel like a lot of the core is at \nleast \"fleshed out\", if not necessarily actually usable yet.\n\nI may take a few days off on this, and hope others will step in and fix up \nsome of the remaining problems ..\n\n\t\t\tLinus\n"},{"id":"39263","messageId":"7v7isghp1r.fsf@assigned-by-dhcp.cox.net","threadId":"7636","inReplyTo":"Pine.LNX.4.64.0704122104030.4061@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Teach \"git-read-tree -u\" to check out submodules as a directory","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-13T04:32:48Z","receivedAt":"2007-04-13T04:32:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> @@ -136,8 +147,13 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n>  \t\t\t\t\t\t \"symlink %s (%s)\", path, strerror(errno));\n>  \t\t}\n>  \t\tbreak;\n> +\tcase S_IFDIRLNK:\n> +\t\tif (to_tempfile)\n> +\t\t\treturn error(\"git-checkout-index: cannot create temporary subproject %s\", path);\n> +\t\tif (mkdir(path, 0777) < 0)\n> +\t\t\treturn error(\"git-checkout-index: cannot create subproject directory %s\", path);\n> +\t\tbreak;\n>  \tdefault:\n>  \t\treturn error(\"git-checkout-index: unknown file mode for %s\", path);\n>  \t}\n>  \n\nHmm.  Perhaps something like this on top?\n\ndiff --git a/entry.c b/entry.c\nindex 9545e89..0874d61 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -1,6 +1,23 @@\n #include \"cache.h\"\n #include \"blob.h\"\n \n+static int make_directory(const char *path, int unlink_as_needed)\n+{\n+\tif (mkdir(path, 0777) < 0) {\n+\t\tif (errno == EEXIST) {\n+\t\t\tstruct stat st;\n+\t\t\tif (unlink_as_needed &&\n+\t\t\t    !unlink(path) &&\n+\t\t\t    !mkdir(path, 0777))\n+\t\t\t\treturn 0; /* ok */\n+\t\t\tif (!stat(path, &st) && S_ISDIR(st.st_mode))\n+\t\t\t\treturn 0; /* ok */\n+\t\t}\n+\t\treturn -1;\n+\t}\n+\treturn 0;\n+}\n+\n static void create_directories(const char *path, struct checkout *state)\n {\n \tint len = strlen(path);\n@@ -8,19 +25,14 @@ static void create_directories(const char *path, struct checkout *state)\n \tconst char *slash = path;\n \n \twhile ((slash = strchr(slash+1, '/')) != NULL) {\n+\t\tint unlink_as_needed;\n+\n \t\tlen = slash - path;\n \t\tmemcpy(buf, path, len);\n \t\tbuf[len] = 0;\n-\t\tif (mkdir(buf, 0777)) {\n-\t\t\tif (errno == EEXIST) {\n-\t\t\t\tstruct stat st;\n-\t\t\t\tif (len > state->base_dir_len && state->force && !unlink(buf) && !mkdir(buf, 0777))\n-\t\t\t\t\tcontinue;\n-\t\t\t\tif (!stat(buf, &st) && S_ISDIR(st.st_mode))\n-\t\t\t\t\tcontinue; /* ok */\n-\t\t\t}\n+\t\tunlink_as_needed = (state->base_dir_len < len && state->force);\n+\t\tif (make_directory(buf, unlink_as_needed) < 0)\n \t\t\tdie(\"cannot create directory at %s\", buf);\n-\t\t}\n \t}\n \tfree(buf);\n }\n@@ -150,7 +162,7 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n \tcase S_IFDIRLNK:\n \t\tif (to_tempfile)\n \t\t\treturn error(\"git-checkout-index: cannot create temporary subproject %s\", path);\n-\t\tif (mkdir(path, 0777) < 0)\n+\t\tif (make_directory(path, 0))\n \t\t\treturn error(\"git-checkout-index: cannot create subproject directory %s\", path);\n \t\tbreak;\n \tdefault:\n"},{"id":"39264","messageId":"Pine.LNX.4.64.0704122143420.28042@woody.linux-foundation.org","threadId":"7636","inReplyTo":"7v7isghp1r.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Teach \"git-read-tree -u\" to check out submodules as a directory","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-13T04:54:34Z","receivedAt":"2007-04-13T04:54:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 12 Apr 2007, Junio C Hamano wrote:\n> \n> Hmm.  Perhaps something like this on top?\n> \n> diff --git a/entry.c b/entry.c\n> index 9545e89..0874d61 100644\n> --- a/entry.c\n> +++ b/entry.c\n> @@ -1,6 +1,23 @@\n>  #include \"cache.h\"\n>  #include \"blob.h\"\n>  \n> +static int make_directory(const char *path, int unlink_as_needed)\n\nI actually tried to think this through, and I don't *think* we should need \nto do this. It should literally be sufficient to just do the \"mkdir()\" \ncall.\n\nWhy? Because we've already done the \"create_directories()\" calls earlier, \nand if you actually look at \"create_file()\", that one also just does a \nsimple \"open(path, O_WRONLY | O_CREAT | O_EXCL, mode)\" - so doing just the \nmkdir() for creating a subdirectory should be the logically equivalent \noperation.\n\nSimilarly, the S_IFLNK case just does a \"symlink()\" system call. No \"try \nto unlink if there was an old entry there before\" code.\n\nBasically, once we are inside \"write_entry()\", we expect to get a success \nor a failure, not a \"needs cleanup\". If it needed resolving of other state \nbefore, we shouldn't even have gotten to that stage in the first place.\n\nThat said, looking closer, you're right - the cleanup that is done earlier \nis actually wrong for the gitlink case. So I think the *real* problem is \nthe cleanup in \"checkout_entry()\". In particular, note the\n\n\tif (!lstat(path, &st)) {\n\t\t...\n\t\tif (S_ISDIR(st.st_mode)) {\n\t\t\t...\n\t\t}\n\t}\n\ncode. If there was a directory there, we should actually leave it alone \nfor the gitlink case, because it is up to the *subproject* checkout (not \nthe superproject checkout) to handle any issues within that \nsubdirectory!\n\nSo I think the *right* patch (on top of the one I send out) is actually \njust this:\n\n\t\tLinus\n\n---\ndiff --git a/entry.c b/entry.c\nindex 9545e89..50ffae4 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -195,6 +195,9 @@ int checkout_entry(struct cache_entry *ce, struct checkout *state, char *topath)\n \t\t */\n \t\tunlink(path);\n \t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\t/* If it is a gitlink, leave it alone! */\n+\t\t\tif (S_ISDIRLNK(ntohl(ce->ce_mode)))\n+\t\t\t\treturn 0;\n \t\t\tif (!state->force)\n \t\t\t\treturn error(\"%s is a directory\", path);\n \t\t\tremove_subtree(path);\n"}]}