Re: [RFC/PATCH] Detailed diagnostic when parsing an object name fails.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:

>> Perhaps the second step would be to teach the machinery to understand a
>> syntax like "<tree-ish>:./<path>" and have it prefix the path to the
>> current subdirectory from the root of the work tree, and with such an
>> enhancement, the suggestion given by this patch would probably change to
>> "Did you mean 'HEAD:./test-lib.sh'?", but that would be a separate
>> topic.
>
> Exactly. I think this HEAD:./relative-path syntax has been discussed
> here already, but I don't remember the outcome of the discussion. If
> it's ever implemented, my patch, modified as you suggest will help
> users to discover the feature ;-).

I gave it a try, and it seems it's not as easy to implement as it
seems.

The main task is to teach get_sha1_with_mode_1(..., prefix, ...) to
notice a ./ in "HEAD:./filename", and to replace it with prefix, which
is easy. The problem is to get "prefix" reliably. Since
get_sha1_with_mode_1 is called by get_sha1, which doesn't know about
prefix, and which is called from at least 92 places in the code, this
would require changing all these calls to add the "prefix" argument
(and to find out where to get this prefix from).

So, I guess I'll give up :-(. (unless someone either show a great
motivation for the feature, or a magic formula to make the patch
short)

FYI, here's the toy patch I started (works for "git show HEAD:./file",
but not for "git rev-parse HEAD:./file" for example):

diff --git a/cache.h b/cache.h
index c122bfa..a44f06f 100644
--- a/cache.h
+++ b/cache.h
@@ -708,10 +708,10 @@ static inline unsigned int hexval(unsigned char c)
 #define DEFAULT_ABBREV 7
 
 extern int get_sha1(const char *str, unsigned char *sha1);
-extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
-static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, const char *prefix, int gently);
+static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode, const char *prefix)
 {
-	return get_sha1_with_mode_1(str, sha1, mode, 1, NULL);
+	return get_sha1_with_mode_1(str, sha1, mode, prefix, 1);
 }
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
diff --git a/revision.c b/revision.c
index 25fa14d..19ddb21 100644
--- a/revision.c
+++ b/revision.c
@@ -944,7 +944,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		local_flags = UNINTERESTING;
 		arg++;
 	}
-	if (get_sha1_with_mode(arg, sha1, &mode))
+	if (get_sha1_with_mode(arg, sha1, &mode, revs->prefix))
 		return -1;
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
@@ -1419,7 +1419,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		unsigned char sha1[20];
 		struct object *object;
 		unsigned mode;
-		if (get_sha1_with_mode(revs->def, sha1, &mode))
+		if (get_sha1_with_mode(revs->def, sha1, &mode, revs->prefix))
 			die("bad default revision '%s'", revs->def);
 		object = get_reference(revs, revs->def, sha1, 0);
 		add_pending_object_with_mode(revs, object, revs->def, mode);
diff --git a/setup.c b/setup.c
index 5792eb7..60c1e08 100644
--- a/setup.c
+++ b/setup.c
@@ -79,7 +79,7 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg)
 	unsigned char sha1[20];
 	unsigned mode;
 	/* try a detailed diagnostic ... */
-	get_sha1_with_mode_1(arg, sha1, &mode, 0, prefix);
+	get_sha1_with_mode_1(arg, sha1, &mode, prefix, 0);
 	/* ... or fall back the most general message. */
 	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
 	    "Use '--' to separate paths from revisions", arg);
diff --git a/sha1_name.c b/sha1_name.c
index ca8f9db..330d3fe 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -801,7 +801,7 @@ release_return:
 int get_sha1(const char *name, unsigned char *sha1)
 {
 	unsigned unused;
-	return get_sha1_with_mode(name, sha1, &unused);
+	return get_sha1_with_mode(name, sha1, &unused, NULL);
 }
 
 /* Must be called only when object_name:filename doesn't exist. */
@@ -893,7 +893,7 @@ static void diagnose_invalid_index_path(int stage,
 }
 
 
-int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)
+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, const char *prefix, int gently)
 {
 	int ret, bracket_depth;
 	int namelen = strlen(name);
@@ -961,11 +961,26 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 		}
 		if (!get_sha1_1(name, cp-name, tree_sha1)) {
 			const char *filename = cp+1;
-			ret = get_tree_entry(tree_sha1, filename, sha1, mode);
-			if (!gently) {
-				diagnose_invalid_sha1_path(prefix, filename,
-							   tree_sha1, object_name);
-				free(object_name);
+			if (filename[0] == '.' && filename[1] == '/') {
+				if (!prefix)
+					prefix = "";
+				char *absfilename = xmalloc((strlen(filename) - 2)
+							    + strlen(prefix) + 1);
+				strcpy(absfilename, prefix);
+				strcat(absfilename, filename+2);
+				ret = get_tree_entry(tree_sha1, absfilename, sha1, mode);
+				if (!gently) {
+					diagnose_invalid_sha1_path(prefix, absfilename,
+								   tree_sha1, object_name);
+					free(object_name);
+				}
+			} else {
+				ret = get_tree_entry(tree_sha1, filename, sha1, mode);
+				if (!gently) {
+					diagnose_invalid_sha1_path(prefix, filename,
+								   tree_sha1, object_name);
+					free(object_name);
+				}
 			}
 			return ret;
 		} else {


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]