[PATCH 1/5] Add S_IFINVALID mode

[PATCH 1/5] Add S_IFINVALID mode

[Martin Koegler]

S_IFINVALID is used to signal, that no mode information is available.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
If somebody thinks, that S_IFINVALID should have an different impossible value, 
please feel free to change it.

 cache.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 53c2341..d425c26 100644
--- a/cache.h
+++ b/cache.h
@@ -24,6 +24,9 @@
 #define DTYPE(de)	DT_UNKNOWN
 #endif
 
+/* unknown mode (impossible combination S_IFIFO|S_IFCHR) */
+#define S_IFINVALID     0030000
+
 /*
  * Intensive research over the course of many years has shown that
  * port 9418 is totally unused by anything else. Or
-- 
1.5.1.1.206.g4a12-dirty

[PATCH 2/5] add get_sha1_with_mode

[Martin Koegler]

get_sha1_with_mode basically behaves as get_sha1. It has an additional
parameter for storing the mode of the object. This parameter may be NULL.
If the mode can not be determinded, it stores S_IFINVALID.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 cache.h     |    1 +
 sha1_name.c |   11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index d425c26..a9ae3f8 100644
--- a/cache.h
+++ b/cache.h
@@ -320,6 +320,7 @@ static inline unsigned int hexval(unsigned int c)
 #define DEFAULT_ABBREV 7
 
 extern int get_sha1(const char *str, unsigned char *sha1);
+extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 267ea3f..1349c0a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -643,11 +643,18 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
  */
 int get_sha1(const char *name, unsigned char *sha1)
 {
+	return get_sha1_with_mode(name, sha1, NULL);
+}
+
+int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned* mode)
+{
 	int ret, bracket_depth;
 	unsigned unused;
 	int namelen = strlen(name);
 	const char *cp;
 
+	if (mode)
+		*mode = S_IFINVALID;
 	prepare_alt_odb();
 	ret = get_sha1_1(name, namelen, sha1);
 	if (!ret)
@@ -685,6 +692,8 @@ int get_sha1(const char *name, unsigned char *sha1)
 				break;
 			if (ce_stage(ce) == stage) {
 				hashcpy(sha1, ce->sha1);
+				if (mode)
+					*mode = ntohl(ce->ce_mode);
 				return 0;
 			}
 			pos++;
@@ -703,7 +712,7 @@ int get_sha1(const char *name, unsigned char *sha1)
 		unsigned char tree_sha1[20];
 		if (!get_sha1_1(name, cp-name, tree_sha1))
 			return get_tree_entry(tree_sha1, cp+1, sha1,
-					      &unused);
+					      mode ? mode : &unused);
 	}
 	return ret;
 }
-- 
1.5.1.1.206.g4a12-dirty

[PATCH 3/5] add add_object_array_with_mode

[Martin Koegler]

Each object in struct object_array is extended with the mode.
If not specified, S_IFINVALID is used. An object with an mode value
can be added with add_object_array_with_mode.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 object.c |    6 ++++++
 object.h |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/object.c b/object.c
index 78a44a6..fcf06bd 100644
--- a/object.c
+++ b/object.c
@@ -240,6 +240,11 @@ int object_list_contains(struct object_list *list, struct object *obj)
 
 void add_object_array(struct object *obj, const char *name, struct object_array *array)
 {
+	add_object_array_with_mode(obj, name, array, S_IFINVALID);
+}
+
+void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
+{
 	unsigned nr = array->nr;
 	unsigned alloc = array->alloc;
 	struct object_array_entry *objects = array->objects;
@@ -252,5 +257,6 @@ void add_object_array(struct object *obj, const char *name, struct object_array
 	}
 	objects[nr].item = obj;
 	objects[nr].name = name;
+	objects[nr].mode = mode;
 	array->nr = ++nr;
 }
diff --git a/object.h b/object.h
index bdbbc18..d0b5298 100644
--- a/object.h
+++ b/object.h
@@ -17,6 +17,7 @@ struct object_array {
 	struct object_array_entry {
 		struct object *item;
 		const char *name;
+		unsigned mode;
 	} *objects;
 };
 
@@ -77,5 +78,6 @@ int object_list_contains(struct object_list *list, struct object *obj);
 
 /* Object array handling .. */
 void add_object_array(struct object *obj, const char *name, struct object_array *array);
+void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode);
 
 #endif /* OBJECT_H */
-- 
1.5.1.1.206.g4a12-dirty

[PATCH 4/5] store mode in rev_list, if <tree>:<filename> syntax is used

[Martin Koegler]

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 revision.c |   17 ++++++++++++-----
 revision.h |    1 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/revision.c b/revision.c
index ce70f48..49bd292 100644
--- a/revision.c
+++ b/revision.c
@@ -116,9 +116,14 @@ void mark_parents_uninteresting(struct commit *commit)
 
 void add_pending_object(struct rev_info *revs, struct object *obj, const char *name)
 {
+	add_pending_object_with_mode(revs, obj, name, S_IFINVALID);
+}
+
+void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)
+{
 	if (revs->no_walk && (obj->flags & UNINTERESTING))
 		die("object ranges do not make sense when not walking revisions");
-	add_object_array(obj, name, &revs->pending);
+	add_object_array_with_mode(obj, name, &revs->pending, mode);
 	if (revs->reflog_info && obj->type == OBJ_COMMIT)
 		add_reflog_for_walk(revs->reflog_info,
 				(struct commit *)obj, name);
@@ -723,6 +728,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 			int flags,
 			int cant_be_filename)
 {
+	unsigned mode;
 	char *dotdot;
 	struct object *object;
 	unsigned char sha1[20];
@@ -796,12 +802,12 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		local_flags = UNINTERESTING;
 		arg++;
 	}
-	if (get_sha1(arg, sha1))
+	if (get_sha1_with_mode(arg, sha1, &mode))
 		return -1;
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
 	object = get_reference(revs, arg, sha1, flags ^ local_flags);
-	add_pending_object(revs, object, arg);
+	add_pending_object_with_mode(revs, object, arg, mode);
 	return 0;
 }
 
@@ -1177,10 +1183,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	if (def && !revs->pending.nr) {
 		unsigned char sha1[20];
 		struct object *object;
-		if (get_sha1(def, sha1))
+		unsigned mode;
+		if (get_sha1_with_mode(def, sha1, &mode))
 			die("bad default revision '%s'", def);
 		object = get_reference(revs, def, sha1, 0);
-		add_pending_object(revs, object, def);
+		add_pending_object_with_mode(revs, object, def, mode);
 	}
 
 	if (revs->topo_order)
diff --git a/revision.h b/revision.h
index 8a02618..5b41e2d 100644
--- a/revision.h
+++ b/revision.h
@@ -131,5 +131,6 @@ extern void add_object(struct object *obj,
 		       const char *name);
 
 extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);
+extern void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode);
 
 #endif
-- 
1.5.1.1.206.g4a12-dirty

[PATCH 5/5] use mode of the tree in git-diff, if <tree>:<file> syntax is used

[Martin Koegler]

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 builtin-diff.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index 21d13f0..f7fb914 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -13,13 +13,10 @@
 #include "log-tree.h"
 #include "builtin.h"
 
-/* NEEDSWORK: struct object has place for name but we _do_
- * know mode when we extracted the blob out of a tree, which
- * we currently lose.
- */
 struct blobinfo {
 	unsigned char sha1[20];
 	const char *name;
+	unsigned mode;
 };
 
 static const char builtin_diff_usage[] =
@@ -70,8 +67,12 @@ static int builtin_diff_b_f(struct rev_info *revs,
 		die("'%s': %s", path, strerror(errno));
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
 		die("'%s': not a regular file or symlink", path);
+
+	if (blob[0].mode == S_IFINVALID)
+		blob[0].mode = canon_mode(st.st_mode);
+
 	stuff_change(&revs->diffopt,
-		     canon_mode(st.st_mode), canon_mode(st.st_mode),
+		     blob[0].mode, canon_mode(st.st_mode),
 		     blob[0].sha1, null_sha1,
 		     path, path);
 	diffcore_std(&revs->diffopt);
@@ -88,8 +89,14 @@ static int builtin_diff_blobs(struct rev_info *revs,
 	if (argc > 1)
 		usage(builtin_diff_usage);
 
+	if (blob[0].mode == S_IFINVALID)
+		blob[0].mode = mode;
+
+	if (blob[1].mode == S_IFINVALID)
+		blob[1].mode = mode;
+
 	stuff_change(&revs->diffopt,
-		     mode, mode,
+		     blob[0].mode, blob[1].mode,
 		     blob[0].sha1, blob[1].sha1,
 		     blob[0].name, blob[1].name);
 	diffcore_std(&revs->diffopt);
@@ -271,6 +278,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 				die("more than two blobs given: '%s'", name);
 			hashcpy(blob[blobs].sha1, obj->sha1);
 			blob[blobs].name = name;
+			blob[blobs].mode = list->mode;
 			blobs++;
 			continue;
 
-- 
1.5.1.1.206.g4a12-dirty

Re: [PATCH 1/5] Add S_IFINVALID mode

[Junio C Hamano]

Just gave only a cursory look but this looks much nicer ;-)

Re: [PATCH 2/5] add get_sha1_with_mode

[Junio C Hamano]

Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:

> get_sha1_with_mode basically behaves as get_sha1. It has an additional
> parameter for storing the mode of the object. This parameter may be NULL.
> If the mode can not be determinded, it stores S_IFINVALID.
>
> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
> ---
>  cache.h     |    1 +
>  sha1_name.c |   11 ++++++++++-
>  2 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index d425c26..a9ae3f8 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -320,6 +320,7 @@ static inline unsigned int hexval(unsigned int c)
>  #define DEFAULT_ABBREV 7
>  
>  extern int get_sha1(const char *str, unsigned char *sha1);
> +extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
>  extern int get_sha1_hex(const char *hex, unsigned char *sha1);
>  extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
>  extern int read_ref(const char *filename, unsigned char *sha1);
> diff --git a/sha1_name.c b/sha1_name.c
> index 267ea3f..1349c0a 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -643,11 +643,18 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
>   */
>  int get_sha1(const char *name, unsigned char *sha1)
>  {
> +	return get_sha1_with_mode(name, sha1, NULL);
> +}
> +
> +int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned* mode)

Style.  Asterisk "*" is next to variable, not type in our code,
like "unsigned *mode".

> +{
>  	int ret, bracket_depth;
>  	unsigned unused;
>  	int namelen = strlen(name);
>  	const char *cp;
>  
> +	if (mode)
> +		*mode = S_IFINVALID;
>  	prepare_alt_odb();
>  	ret = get_sha1_1(name, namelen, sha1);
>  	if (!ret)
> @@ -685,6 +692,8 @@ int get_sha1(const char *name, unsigned char *sha1)
>  				break;
>  			if (ce_stage(ce) == stage) {
>  				hashcpy(sha1, ce->sha1);
> +				if (mode)
> +					*mode = ntohl(ce->ce_mode);
>  				return 0;
>  			}
>  			pos++;
> @@ -703,7 +712,7 @@ int get_sha1(const char *name, unsigned char *sha1)
>  		unsigned char tree_sha1[20];
>  		if (!get_sha1_1(name, cp-name, tree_sha1))
>  			return get_tree_entry(tree_sha1, cp+1, sha1,
> -					      &unused);
> +					      mode ? mode : &unused);
>  	}
>  	return ret;
>  }

Hmmmm.  I'm not sure if it is worth to have "store only of mode
pointer is not NULL" check in many places.  Wouldn't it make
more sense to require callers of _with_mode() variant to always
send in a valid pointer (after all, it is the caller who chose
to call the _with_mode() variant), and make get_sha1() like
this?

	int get_sha1(const char *name, unsigned char *sha1)
	{
        	unsigned discard;
                return get_sha1_with_mode(name, sha1, &discard);
	}