Re: [PATCH] TODO: add item for searching based on git-patch-id(1)

[Konstantin Ryabitsev <konstantin@linuxfoundation.org>]

On Tue, Oct 01, 2019 at 03:37:47AM +0000, Eric Wong wrote:
> I forgot about this feature when I was implementing
> blob-ID-based searches :x
> ---
>  TODO | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/TODO b/TODO
> index 2c525615..93054bb3 100644
> --- a/TODO
> +++ b/TODO
> @@ -112,3 +112,6 @@ all need to be considered for everything we introduce)
>  
>  * make "git cat-file --batch" detect unlinked packfiles so we don't
>    have to restart processes (very long-term)
> +
> +* support searching based on `git-patch-id --stable` to improve
> +  bidirectional mapping of commits <=> emails

It would be handy, but a word of caution -- because it strips
whitespace, git-patch-id is not great for languages with syntactic
indentation, like Python. For example, the following two patches
generate the same patch-id, but one is actually malicious:

diff --git a/file1.py b/file1.py
index e574c49..6aa1937 100644
--- a/file1.py
+++ b/file1.py
@@ -1,3 +1,13 @@
 #!/usr/bin/python

+def is_logged_in(cookie):
+    if cookie:
+        print('User is logged in')
+        return True
+
+    return False
+
+if is_logged_in(True):
+    print('You are logged in')
+
 print('Hello!')

This one below is malicious, because is_logged_in() will always return
True:

diff --git a/file1.py b/file1.py
index e574c49..6aa1937 100644
--- a/file1.py
+++ b/file1.py
@@ -1,3 +1,13 @@
 #!/usr/bin/python
 
+def is_logged_in(cookie):
+    if cookie:
+        print('User is logged in')
+    return True
+
+    return False
+
+if is_logged_in(True):
+    print('You are logged in')
+
 print('Hello!')

So, I wouldn't use git-patch-id as a mechanism to look up patches,
except as an auxiliary one.

-K