{"thread":{"id":"33393","subject":"Re: [RFC/PATCH 0/7] Rework git core for native submodules","startedAt":"2013-04-05T16:02:31Z","lastAt":"2013-04-05T16:37:58Z","messageCount":2,"participants":["Ramkumar Ramachandra","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"213286","messageId":"CA+55aFxyyqkS4XyE1as2TdeARce9g9BTK=jdY+XcomNtMdrArg@mail.gmail.com","threadId":"33393","inReplyTo":"CALkWK0n0uKTsPv2d20ESVd5OV5yfK5V1HecGWePYU=UFw5Jkfg@mail.gmail.com","subject":"Re: [RFC/PATCH 0/7] Rework git core for native submodules","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-04-05T16:02:31Z","receivedAt":"2013-04-05T16:02:31Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Thu, Apr 4, 2013 at 1:04 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Linus Torvalds wrote:\n>> Or you could also just edit and carry a dirty .gitmodules around for\n>> your personal use-case.\n>\n> I'm sorry, but a dirty worktree is unnecessarily painful to work with.\n\nBzzt. Wrong.\n\nA dirty worktree is not only easy to work with (I do it all the time,\nhaving random test-patches in my tree that I never even intend to\ncommit), it's a *requirement*.\n\nOne thing that git does really really well is merging. And one of the\nreasons why git does merging well (apart from the obvious meta-issue:\nit's what I care about) is that it not only has the stable information\nin the object database, it also has the staging information in the\nindex, *and* it has dirty data in the working tree.\n\nYou absolutely need all three. Having an \"edit\" command to edit stable\ndata (or staging data) is broken. Trust me, I've been there, done\nthat, got the T-shirt and know it is wrong. The whole \"stable objects\"\n+ \"index\" + \"dirty worktree\" is FUNDAMENTALLY the right way to work,\nand it *has* to work that way for merges to work well.\n\nThe only things that we don't have \"dirty data\" for in the worktree is\ncreating commits and tags, but those aren't relevant for the merging\nprocess anyway, in the sense that you never change them for merging,\nyou create them *after* merging (and this is fundamental, and not just\na git implementation issue).\n\nSo you absolutely need a dirty worktree. You need it for testing, and\nyou need it for merging. Having a model where you don't have a\nin-progress entity that works as a temporary is absolutely and\nentirely wrong.\n\n               Linus\n"},{"id":"213273","messageId":"1365179878-2259-1-git-send-email-artagnon@gmail.com","threadId":"33393","inReplyTo":"CA+55aFxyyqkS4XyE1as2TdeARce9g9BTK=jdY+XcomNtMdrArg@mail.gmail.com","subject":"Re: [RFC/PATCH 0/7] Rework git core for native submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-05T16:37:58Z","receivedAt":"2013-04-05T16:37:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Linus Torvalds wrote:\n> So you absolutely need a dirty worktree. You need it for testing, and\n> you need it for merging. Having a model where you don't have a\n> in-progress entity that works as a temporary is absolutely and\n> entirely wrong.\n\nI agree entirely.  My comment was just a \"by the way\", and specific to\nhow people work with .gitmodules: I didn't imply any strong notions of\nRight or Wrong with respect to dirty worktrees in general.  So, yes:\nlinks stage and unstage, just like blobs do.\n\nOh, and I'm currently writing infrastructure to work with links like\nblobs.  Here's a WIP: git cat-link <link> is exactly the same as cat\n<file>, to the end user.\n\n-- 8< --\nFrom d8a1de6f9075771dde6f1fde9ffa193dce386a17 Mon Sep 17 00:00:00 2001\nFrom: Ramkumar Ramachandra <artagnon@gmail.com>\nDate: Fri, 5 Apr 2013 19:42:56 +0530\nSubject: [PATCH] builtin/cat-link: implement new builtin\n\nThis is a simple program that calls unpack_trees() with a custom\ncallback that just prints the contents of whatever objects were\nmatched using revs.prune_data.  Blobs can be cat'ed directly from the\nfilesystem, so this program is primarily useful for links; git\ncat-link <link> shows it up like a blob.\n\nWe will use this program to build edit-link.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Makefile           |  3 +-\n builtin.h          |  1 +\n builtin/cat-link.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n diff-lib.c         | 10 +++----\n diff.h             |  6 ++++\n git.c              |  1 +\n 6 files changed, 98 insertions(+), 6 deletions(-)\n create mode 100644 builtin/cat-link.c\n\ndiff --git a/Makefile b/Makefile\nindex cd4b6f9..28194d7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -349,7 +349,7 @@ GIT-VERSION-FILE: FORCE\n \n # CFLAGS and LDFLAGS are for the users to override from the command line.\n \n-CFLAGS = -g -O2 -Wall\n+CFLAGS = -g -O0 -Wall\n LDFLAGS =\n ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n@@ -893,6 +893,7 @@ BUILTIN_OBJS += builtin/blame.o\n BUILTIN_OBJS += builtin/branch.o\n BUILTIN_OBJS += builtin/bundle.o\n BUILTIN_OBJS += builtin/cat-file.o\n+BUILTIN_OBJS += builtin/cat-link.o\n BUILTIN_OBJS += builtin/check-attr.o\n BUILTIN_OBJS += builtin/check-ignore.o\n BUILTIN_OBJS += builtin/check-ref-format.o\ndiff --git a/builtin.h b/builtin.h\nindex faef559..be0160d 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -49,6 +49,7 @@ extern int cmd_blame(int argc, const char **argv, const char *prefix);\n extern int cmd_branch(int argc, const char **argv, const char *prefix);\n extern int cmd_bundle(int argc, const char **argv, const char *prefix);\n extern int cmd_cat_file(int argc, const char **argv, const char *prefix);\n+extern int cmd_cat_link(int argc, const char **argv, const char *prefix);\n extern int cmd_checkout(int argc, const char **argv, const char *prefix);\n extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/cat-link.c b/builtin/cat-link.c\nnew file mode 100644\nindex 0000000..14dd92b\n--- /dev/null\n+++ b/builtin/cat-link.c\n@@ -0,0 +1,83 @@\n+/*\n+ * Copyright (c) 2013 Ramkumar Ramachandra\n+ */\n+#include \"cache.h\"\n+#include \"tree.h\"\n+#include \"cache-tree.h\"\n+#include \"unpack-trees.h\"\n+#include \"commit.h\"\n+#include \"diff.h\"\n+#include \"revision.h\"\n+\n+static int cat_file(struct cache_entry **src, struct unpack_trees_options *o) {\n+\tint cached, match_missing = 1;\n+\tunsigned dirty_submodule = 0;\n+\tunsigned int mode;\n+\tconst unsigned char *sha1;\n+\tstruct cache_entry *idx = src[0];\n+\tstruct cache_entry *tree = src[1];\n+\tstruct rev_info *revs = o->unpack_data;\n+\tenum object_type type;\n+\tunsigned long size;\n+\tchar *buf;\n+\n+\tcached = o->index_only;\n+\tif (ce_path_match(idx ? idx : tree, &revs->prune_data)) {\n+\t\tif (get_stat_data(idx, &sha1, &mode, cached, match_missing,\n+\t\t\t\t\t&dirty_submodule, NULL) < 0)\n+\t\t\tdie(\"Something went wrong!\");\n+\t\tbuf = read_sha1_file(sha1, &type, &size);\n+\t\tprintf(\"%s\", buf);\n+\t}\n+\treturn 0;\n+}\n+\n+int cmd_cat_link(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct unpack_trees_options opts;\n+\tint cached = 1;\n+\tstruct rev_info revs;\n+\tstruct tree *tree;\n+\tstruct tree_desc tree_desc;\n+\tstruct object_array_entry *ent;\n+\n+\tif (argc < 2)\n+\t\tdie(\"Usage: git cat-link <link>\");\n+\n+\tinit_revisions(&revs, prefix);\n+\tsetup_revisions(argc, argv, &revs, NULL); /* For revs.prune_data */\n+\tadd_head_to_pending(&revs);\n+\n+\t/* Hack to diff against index; we create a dummy tree for the\n+\t   index information */\n+\tif (!revs.pending.nr) {\n+\t\tstruct tree *tree;\n+\t\ttree = lookup_tree(EMPTY_TREE_SHA1_BIN);\n+\t\tadd_pending_object(&revs, &tree->object, \"HEAD\");\n+\t}\n+\n+\tif (read_cache() < 0)\n+\t\tdie(\"read_cache() failed\");\n+\tent = revs.pending.objects;\n+\ttree = parse_tree_indirect(ent->item->sha1);\n+\tif (!tree)\n+\t\treturn error(\"bad tree object %s\",\n+\t\t\t     ent->name ? ent->name : sha1_to_hex(ent->item->sha1));\n+\n+\tmemset(&opts, 0, sizeof(opts));\n+\topts.head_idx = 1;\n+\topts.index_only = cached;\n+\topts.diff_index_cached = cached;\n+\topts.merge = 1;\n+\topts.fn = cat_file;\n+\topts.unpack_data = &revs;\n+\topts.src_index = &the_index;\n+\topts.dst_index = NULL;\n+\topts.pathspec = &revs.diffopt.pathspec;\n+\topts.pathspec->recursive = 1;\n+\topts.pathspec->max_depth = -1;\n+\n+\tinit_tree_desc(&tree_desc, tree->buffer, tree->size);\n+\tunpack_trees(1, &tree_desc, &opts);\n+\treturn 0;\n+}\ndiff --git a/diff-lib.c b/diff-lib.c\nindex f35de0f..b0ba136 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -246,11 +246,11 @@ static void diff_index_show_file(struct rev_info *revs,\n \t\t       sha1, sha1_valid, ce->name, dirty_submodule);\n }\n \n-static int get_stat_data(struct cache_entry *ce,\n-\t\t\t const unsigned char **sha1p,\n-\t\t\t unsigned int *modep,\n-\t\t\t int cached, int match_missing,\n-\t\t\t unsigned *dirty_submodule, struct diff_options *diffopt)\n+int get_stat_data(struct cache_entry *ce,\n+\t\tconst unsigned char **sha1p,\n+\t\tunsigned int *modep,\n+\t\tint cached, int match_missing,\n+\t\tunsigned *dirty_submodule, struct diff_options *diffopt)\n {\n \tconst unsigned char *sha1 = ce->sha1;\n \tunsigned int mode = ce->ce_mode;\ndiff --git a/diff.h b/diff.h\nindex 78b4091..02ed497 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -326,6 +326,12 @@ extern int diff_result_code(struct diff_options *, int);\n \n extern void diff_no_index(struct rev_info *, int, const char **, int, const char *);\n \n+extern int get_stat_data(struct cache_entry *ce,\n+\t\t\tconst unsigned char **sha1p,\n+\t\t\tunsigned int *modep,\n+\t\t\tint cached, int match_missing,\n+\t\t\tunsigned *dirty_submodule, struct diff_options *diffopt);\n+\n extern int index_differs_from(const char *def, int diff_flags);\n \n extern size_t fill_textconv(struct userdiff_driver *driver,\ndiff --git a/git.c b/git.c\nindex 850d3f5..3f3f074 100644\n--- a/git.c\n+++ b/git.c\n@@ -313,6 +313,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"branch\", cmd_branch, RUN_SETUP },\n \t\t{ \"bundle\", cmd_bundle, RUN_SETUP_GENTLY },\n \t\t{ \"cat-file\", cmd_cat_file, RUN_SETUP },\n+\t\t{ \"cat-link\", cmd_cat_link, RUN_SETUP },\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP },\n \t\t{ \"check-ignore\", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"check-ref-format\", cmd_check_ref_format },\n-- \n1.8.2.380.g0d4e79b\n"}]}