# Re: [RFC/PATCH 0/7] Rework git core for native submodules

2 messages from 2013-04-05 to 2013-04-05. Participants: Ramkumar Ramachandra, Linus Torvalds.
Thread: https://gitlist.dev/t/33393

## Linus Torvalds, 2013-04-05 16:02

Subject: Re: [RFC/PATCH 0/7] Rework git core for native submodules
Message-ID: <CA+55aFxyyqkS4XyE1as2TdeARce9g9BTK=jdY+XcomNtMdrArg@mail.gmail.com>
URL: https://gitlist.dev/e/CA%2B55aFxyyqkS4XyE1as2TdeARce9g9BTK%3DjdY%2BXcomNtMdrArg%40mail.gmail.com
In-Reply-To: <CALkWK0n0uKTsPv2d20ESVd5OV5yfK5V1HecGWePYU=UFw5Jkfg@mail.gmail.com>

```
On Thu, Apr 4, 2013 at 1:04 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> Linus Torvalds wrote:
>> Or you could also just edit and carry a dirty .gitmodules around for
>> your personal use-case.
>
> I'm sorry, but a dirty worktree is unnecessarily painful to work with.

Bzzt. Wrong.

A dirty worktree is not only easy to work with (I do it all the time,
having random test-patches in my tree that I never even intend to
commit), it's a *requirement*.

One thing that git does really really well is merging. And one of the
reasons why git does merging well (apart from the obvious meta-issue:
it's what I care about) is that it not only has the stable information
in the object database, it also has the staging information in the
index, *and* it has dirty data in the working tree.

You absolutely need all three. Having an "edit" command to edit stable
data (or staging data) is broken. Trust me, I've been there, done
that, got the T-shirt and know it is wrong. The whole "stable objects"
+ "index" + "dirty worktree" is FUNDAMENTALLY the right way to work,
and it *has* to work that way for merges to work well.

The only things that we don't have "dirty data" for in the worktree is
creating commits and tags, but those aren't relevant for the merging
process anyway, in the sense that you never change them for merging,
you create them *after* merging (and this is fundamental, and not just
a git implementation issue).

So you absolutely need a dirty worktree. You need it for testing, and
you need it for merging. Having a model where you don't have a
in-progress entity that works as a temporary is absolutely and
entirely wrong.

               Linus

```

## Ramkumar Ramachandra, 2013-04-05 16:37

Subject: Re: [RFC/PATCH 0/7] Rework git core for native submodules
Message-ID: <1365179878-2259-1-git-send-email-artagnon@gmail.com>
URL: https://gitlist.dev/e/1365179878-2259-1-git-send-email-artagnon%40gmail.com
In-Reply-To: <CA+55aFxyyqkS4XyE1as2TdeARce9g9BTK=jdY+XcomNtMdrArg@mail.gmail.com>

```
Linus Torvalds wrote:
> So you absolutely need a dirty worktree. You need it for testing, and
> you need it for merging. Having a model where you don't have a
> in-progress entity that works as a temporary is absolutely and
> entirely wrong.

I agree entirely.  My comment was just a "by the way", and specific to
how people work with .gitmodules: I didn't imply any strong notions of
Right or Wrong with respect to dirty worktrees in general.  So, yes:
links stage and unstage, just like blobs do.

Oh, and I'm currently writing infrastructure to work with links like
blobs.  Here's a WIP: git cat-link <link> is exactly the same as cat
<file>, to the end user.

-- 8< --
From d8a1de6f9075771dde6f1fde9ffa193dce386a17 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Fri, 5 Apr 2013 19:42:56 +0530
Subject: [PATCH] builtin/cat-link: implement new builtin

This is a simple program that calls unpack_trees() with a custom
callback that just prints the contents of whatever objects were
matched using revs.prune_data.  Blobs can be cat'ed directly from the
filesystem, so this program is primarily useful for links; git
cat-link <link> shows it up like a blob.

We will use this program to build edit-link.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Makefile           |  3 +-
 builtin.h          |  1 +
 builtin/cat-link.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 diff-lib.c         | 10 +++----
 diff.h             |  6 ++++
 git.c              |  1 +
 6 files changed, 98 insertions(+), 6 deletions(-)
 create mode 100644 builtin/cat-link.c

diff --git a/Makefile b/Makefile
index cd4b6f9..28194d7 100644
--- a/Makefile
+++ b/Makefile
@@ -349,7 +349,7 @@ GIT-VERSION-FILE: FORCE
 
 # CFLAGS and LDFLAGS are for the users to override from the command line.
 
-CFLAGS = -g -O2 -Wall
+CFLAGS = -g -O0 -Wall
 LDFLAGS =
 ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
@@ -893,6 +893,7 @@ BUILTIN_OBJS += builtin/blame.o
 BUILTIN_OBJS += builtin/branch.o
 BUILTIN_OBJS += builtin/bundle.o
 BUILTIN_OBJS += builtin/cat-file.o
+BUILTIN_OBJS += builtin/cat-link.o
 BUILTIN_OBJS += builtin/check-attr.o
 BUILTIN_OBJS += builtin/check-ignore.o
 BUILTIN_OBJS += builtin/check-ref-format.o
diff --git a/builtin.h b/builtin.h
index faef559..be0160d 100644
--- a/builtin.h
+++ b/builtin.h
@@ -49,6 +49,7 @@ extern int cmd_blame(int argc, const char **argv, const char *prefix);
 extern int cmd_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_bundle(int argc, const char **argv, const char *prefix);
 extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
+extern int cmd_cat_link(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
 extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
diff --git a/builtin/cat-link.c b/builtin/cat-link.c
new file mode 100644
index 0000000..14dd92b
--- /dev/null
+++ b/builtin/cat-link.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2013 Ramkumar Ramachandra
+ */
+#include "cache.h"
+#include "tree.h"
+#include "cache-tree.h"
+#include "unpack-trees.h"
+#include "commit.h"
+#include "diff.h"
+#include "revision.h"
+
+static int cat_file(struct cache_entry **src, struct unpack_trees_options *o) {
+	int cached, match_missing = 1;
+	unsigned dirty_submodule = 0;
+	unsigned int mode;
+	const unsigned char *sha1;
+	struct cache_entry *idx = src[0];
+	struct cache_entry *tree = src[1];
+	struct rev_info *revs = o->unpack_data;
+	enum object_type type;
+	unsigned long size;
+	char *buf;
+
+	cached = o->index_only;
+	if (ce_path_match(idx ? idx : tree, &revs->prune_data)) {
+		if (get_stat_data(idx, &sha1, &mode, cached, match_missing,
+					&dirty_submodule, NULL) < 0)
+			die("Something went wrong!");
+		buf = read_sha1_file(sha1, &type, &size);
+		printf("%s", buf);
+	}
+	return 0;
+}
+
+int cmd_cat_link(int argc, const char **argv, const char *prefix)
+{
+	struct unpack_trees_options opts;
+	int cached = 1;
+	struct rev_info revs;
+	struct tree *tree;
+	struct tree_desc tree_desc;
+	struct object_array_entry *ent;
+
+	if (argc < 2)
+		die("Usage: git cat-link <link>");
+
+	init_revisions(&revs, prefix);
+	setup_revisions(argc, argv, &revs, NULL); /* For revs.prune_data */
+	add_head_to_pending(&revs);
+
+	/* Hack to diff against index; we create a dummy tree for the
+	   index information */
+	if (!revs.pending.nr) {
+		struct tree *tree;
+		tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+		add_pending_object(&revs, &tree->object, "HEAD");
+	}
+
+	if (read_cache() < 0)
+		die("read_cache() failed");
+	ent = revs.pending.objects;
+	tree = parse_tree_indirect(ent->item->sha1);
+	if (!tree)
+		return error("bad tree object %s",
+			     ent->name ? ent->name : sha1_to_hex(ent->item->sha1));
+
+	memset(&opts, 0, sizeof(opts));
+	opts.head_idx = 1;
+	opts.index_only = cached;
+	opts.diff_index_cached = cached;
+	opts.merge = 1;
+	opts.fn = cat_file;
+	opts.unpack_data = &revs;
+	opts.src_index = &the_index;
+	opts.dst_index = NULL;
+	opts.pathspec = &revs.diffopt.pathspec;
+	opts.pathspec->recursive = 1;
+	opts.pathspec->max_depth = -1;
+
+	init_tree_desc(&tree_desc, tree->buffer, tree->size);
+	unpack_trees(1, &tree_desc, &opts);
+	return 0;
+}
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..b0ba136 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -246,11 +246,11 @@ static void diff_index_show_file(struct rev_info *revs,
 		       sha1, sha1_valid, ce->name, dirty_submodule);
 }
 
-static int get_stat_data(struct cache_entry *ce,
-			 const unsigned char **sha1p,
-			 unsigned int *modep,
-			 int cached, int match_missing,
-			 unsigned *dirty_submodule, struct diff_options *diffopt)
+int get_stat_data(struct cache_entry *ce,
+		const unsigned char **sha1p,
+		unsigned int *modep,
+		int cached, int match_missing,
+		unsigned *dirty_submodule, struct diff_options *diffopt)
 {
 	const unsigned char *sha1 = ce->sha1;
 	unsigned int mode = ce->ce_mode;
diff --git a/diff.h b/diff.h
index 78b4091..02ed497 100644
--- a/diff.h
+++ b/diff.h
@@ -326,6 +326,12 @@ extern int diff_result_code(struct diff_options *, int);
 
 extern void diff_no_index(struct rev_info *, int, const char **, int, const char *);
 
+extern int get_stat_data(struct cache_entry *ce,
+			const unsigned char **sha1p,
+			unsigned int *modep,
+			int cached, int match_missing,
+			unsigned *dirty_submodule, struct diff_options *diffopt);
+
 extern int index_differs_from(const char *def, int diff_flags);
 
 extern size_t fill_textconv(struct userdiff_driver *driver,
diff --git a/git.c b/git.c
index 850d3f5..3f3f074 100644
--- a/git.c
+++ b/git.c
@@ -313,6 +313,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "branch", cmd_branch, RUN_SETUP },
 		{ "bundle", cmd_bundle, RUN_SETUP_GENTLY },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
+		{ "cat-link", cmd_cat_link, RUN_SETUP },
 		{ "check-attr", cmd_check_attr, RUN_SETUP },
 		{ "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
 		{ "check-ref-format", cmd_check_ref_format },
-- 
1.8.2.380.g0d4e79b

```
