List for cgit developers and users
 help / color / mirror / Atom feed
From: john at keeping.me.uk (John Keeping)
Subject: patch links do not have stable checksums
Date: Sat, 18 Feb 2017 17:32:19 +0000	[thread overview]
Message-ID: <20170218173219.GC1577@john.keeping.me.uk> (raw)
In-Reply-To: <8ec92b94-13ec-45ec-b8f7-66d1165ccd8e@me.com>

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



  reply	other threads:[~2017-02-18 17:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-04  8:45 ilovezfs
2017-02-18 17:32 ` john [this message]
2017-02-18 21:50   ` Jason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170218173219.GC1577@john.keeping.me.uk \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).