patch links do not have stable checksums

patch links do not have stable checksums

[ilovezfs]

I'm reporting this as a result of this issue https://github.com/Homebrew/homebrew-core/issues/5353 which was filed with Homebrew regarding cgit.



The cgit patch links do not have stable checksums because of the cgit version signature at the bottom of each patch.



For example, "cgit v1.1-3-g9641"



So whenever a cgit server upgrades its version of cgit the checksums of the contents of all patch links changes.



This compromises the usefulness of cgit patch links for anything other than casual, temporary use.



As a result of this behavior, Homebrew cannot use cgit patches in our patch blocks since each patch block has a url and a checksum, so every time the checksum changes due to the signature change, the patch block is invalidated, and someone must investigate why it changed and whether the content changed in any way other than the signature, and then update the checksum, and open a pull request, and go through CI, and have someone approve and merge the PR. This is a very wasteful use of the time of volunteers on an open source project.



To mitigate this situation, we end up having to vendor all cgit patches in our separate formula-patches repository, which would be entirely unnecessary if the checksums were stable. This is also a very wasteful use of time, but better than morphing checksums of content that's not actually changing.



It would be great if going forward the version signatures were removed from cgit patches so that there are persistent checksums for the patch files across cgit versions, and so that a change in the checksum actually means there was a real content change worth looking into.



Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20161204/15d870d5/attachment.html>

patch links do not have stable checksums

[john]

On Sun, Dec 04, 2016 at 08:45:04AM +0000, ilove zfs wrote:
> I'm reporting this as a result of this issue
> https://github.com/Homebrew/homebrew-core/issues/5353 which was filed
> with Homebrew regarding cgit.
> 
> The cgit patch links do not have stable checksums because of the cgit
> version signature at the bottom of each patch.
> 
> For example, "cgit v1.1-3-g9641"
> 
> So whenever a cgit server upgrades its version of cgit the checksums
> of the contents of all patch links changes.
> 
> This compromises the usefulness of cgit patch links for anything other
> than casual, temporary use.

Going back to the commit that added this feature, 620bb3e (Add plain
patch view, 2007-12-10), it looks like the original intent was for this
to be used for one-off patch application.

The version string seems to be there because git-format-patch uses it,
so I'm including a patch below which allows overriding the patch
signature via the CGit configuration.

I think there is an argument for changing the default to be fixed, but
even if the signature is fixed I don't think we make any guarantee that
the remainder of the output won't change across versions.  For example,
Git currently has experimental heuristics to improve readability of
diffs and if these become the default then I expect CGit's patch output
will change for some diffs.

> As a result of this behavior, Homebrew cannot use cgit patches in our
> patch blocks since each patch block has a url and a checksum, so every
> time the checksum changes due to the signature change, the patch block
> is invalidated, and someone must investigate why it changed and
> whether the content changed in any way other than the signature, and
> then update the checksum, and open a pull request, and go through CI,
> and have someone approve and merge the PR. This is a very wasteful use
> of the time of volunteers on an open source project.

Is there a reason you can't strip the signature before computing the
checksum?


-- >8 --
Subject: [PATCH] patch: allow overriding signature

Add a configuration option to allow specifying the signature displayed
at the bottom of patch output, in a similar way to the "--signature"
option to git-format-patch.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cgit.c       | 2 ++
 cgit.h       | 1 +
 cgitrc.5.txt | 5 +++++
 ui-patch.c   | 5 ++++-
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/cgit.c b/cgit.c
index 1075753..5d0655b 100644
--- a/cgit.c
+++ b/cgit.c
@@ -145,6 +145,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.logo_link = xstrdup(value);
 	else if (!strcmp(name, "module-link"))
 		ctx.cfg.module_link = xstrdup(value);
+	else if (!strcmp(name, "patch-signature"))
+		ctx.cfg.patch_signature = xstrdup(value);
 	else if (!strcmp(name, "strict-export"))
 		ctx.cfg.strict_export = xstrdup(value);
 	else if (!strcmp(name, "virtual-root")) {
diff --git a/cgit.h b/cgit.h
index fbc6c6a..93d11e1 100644
--- a/cgit.h
+++ b/cgit.h
@@ -201,6 +201,7 @@ struct cgit_config {
 	char *logo_link;
 	char *mimetype_file;
 	char *module_link;
+	char *patch_signature;
 	char *project_list;
 	struct string_list readme;
 	char *robots;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 9fcf445..3d99e86 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -331,6 +331,11 @@ noheader::
 	Flag which, when set to "1", will make cgit omit the standard header
 	on all pages. Default value: none. See also: "embedded".
 
+patch-signature::
+	Signature to add at the bottom of patches.  Specify the empty string
+	to omit the signature block completely.
+	Default value: "cgit <version>".
+
 project-list::
 	A list of subdirectories inside of scan-path, relative to it, that
 	should loaded as git repositories. This must be defined prior to
diff --git a/ui-patch.c b/ui-patch.c
index 047e2f9..520e04b 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -90,7 +90,10 @@ void cgit_print_patch(const char *new_rev, const char *old_rev,
 
 	while ((commit = get_revision(&rev)) != NULL) {
 		log_tree_commit(&rev, commit);
-		printf("-- \ncgit %s\n\n", cgit_version);
+		if (!ctx.cfg.patch_signature)
+			printf("-- \ncgit %s\n\n", cgit_version);
+		else if (*ctx.cfg.patch_signature)
+			printf("-- \n%s\n\n", ctx.cfg.patch_signature);
 	}
 
 	fflush(stdout);
-- 
2.12.0.rc2.230.ga28edc07cd

patch links do not have stable checksums

[Jason]

Seems most reasonable to simply keep it something static and not add a nob.

However, John - you raise a good point. Git's diff feature isn't
exactly stable itself, with all sorts of interesting new diffing
strategies being tried out in newer versions. So perhaps this isn't
really an feasible goal in the first place...