[RFC/WIP] range-diff: show old/new blob OIDs in comments

[RFC/WIP] range-diff: show old/new blob OIDs in comments

[Eric Wong]

(WIP, mostly stream-of-concious notes + reasoning)

When using "git format-patch --range-diff", the pre and
post-image blob OIDs are in each email, while the exact
commit OIDs are rarely shared via emails (only the tip
commit from "git request-pull").

These blob OIDs make it easy to search for or lookup the
full emails which create them, or the blob itself once
it's fetched via git.

public-inbox indexes and allows querying specifically for blob
OIDs via dfpre:/dfpost: since June 2017.  As of Jan 2019,
public-inbox also supports recreating blobs out of patch emails
(querying internally with dfpre:/dfpost: and doing "git apply")

Searching on these blob OIDs also makes it easier to find
previous versions of the patch sets using any mail search
engine.

Future changes to public-inbox may allow generating custom
diffs out of any blobs it can find or recreate.

Most of this is pretty public-inbox-specific and would've
made some future changes to public-inbox much easier....
(if we had this from the start of range-diff).

Unfortunately, it won't help with cases where range-diffs
are already published, but range-diff isn't too old.

I'm also still learning my way around git's C internals, but
using patch.{old,new}_oid_prefix seems alright...

FIXME: tests, t3206 needs updating

Not-signed-off-by: Eric Wong <e@80x24.org>
---
 range-diff.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 7fed5a3b4b..85d2f1f58f 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -118,13 +118,24 @@ static int read_patches(const char *range, struct string_list *list)
 				die(_("could not parse git header '%.*s'"), (int)len, line);
 			strbuf_addstr(&buf, " ## ");
 			if (patch.is_new > 0)
-				strbuf_addf(&buf, "%s (new)", patch.new_name);
+				strbuf_addf(&buf, "%s (new %s)",
+						patch.new_name,
+						patch.new_oid_prefix);
 			else if (patch.is_delete > 0)
-				strbuf_addf(&buf, "%s (deleted)", patch.old_name);
+				strbuf_addf(&buf, "%s (deleted %s)",
+						patch.old_name,
+						patch.old_oid_prefix);
 			else if (patch.is_rename)
-				strbuf_addf(&buf, "%s => %s", patch.old_name, patch.new_name);
+				strbuf_addf(&buf, "%s => %s (%s..%s)",
+						patch.old_name,
+						patch.new_name,
+						patch.old_oid_prefix,
+						patch.new_oid_prefix);
 			else
-				strbuf_addstr(&buf, patch.new_name);
+				strbuf_addf(&buf, "%s (%s..%s)",
+						patch.new_name,
+						patch.old_oid_prefix,
+						patch.new_oid_prefix);
 
 			free(current_filename);
 			if (patch.is_delete > 0)

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Johannes Schindelin]

Hi Eric,


On Thu, 17 Oct 2019, Eric Wong wrote:

> (WIP, mostly stream-of-concious notes + reasoning)
>
> When using "git format-patch --range-diff", the pre and
> post-image blob OIDs are in each email, while the exact
> commit OIDs are rarely shared via emails (only the tip
> commit from "git request-pull").
>
> These blob OIDs make it easy to search for or lookup the
> full emails which create them, or the blob itself once
> it's fetched via git.
>
> public-inbox indexes and allows querying specifically for blob
> OIDs via dfpre:/dfpost: since June 2017.  As of Jan 2019,
> public-inbox also supports recreating blobs out of patch emails
> (querying internally with dfpre:/dfpost: and doing "git apply")
>
> Searching on these blob OIDs also makes it easier to find
> previous versions of the patch sets using any mail search
> engine.
>
> Future changes to public-inbox may allow generating custom
> diffs out of any blobs it can find or recreate.
>
> Most of this is pretty public-inbox-specific and would've
> made some future changes to public-inbox much easier....
> (if we had this from the start of range-diff).
>
> Unfortunately, it won't help with cases where range-diffs
> are already published, but range-diff isn't too old.

I guess your patch won't hurt.

As to recreating blobs from mails: Wow. That's quite a length you're
going, and I think it is a shame that you have to. If only every
contribution came accompanied with a pullable branch in a public
repository.

Instead, we will have to rely on your centralized, non-distributed
service...

Ciao,
Dscho

>
> I'm also still learning my way around git's C internals, but
> using patch.{old,new}_oid_prefix seems alright...
>
> FIXME: tests, t3206 needs updating
>
> Not-signed-off-by: Eric Wong <e@80x24.org>
> ---
>  range-diff.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/range-diff.c b/range-diff.c
> index 7fed5a3b4b..85d2f1f58f 100644
> --- a/range-diff.c
> +++ b/range-diff.c
> @@ -118,13 +118,24 @@ static int read_patches(const char *range, struct string_list *list)
>  				die(_("could not parse git header '%.*s'"), (int)len, line);
>  			strbuf_addstr(&buf, " ## ");
>  			if (patch.is_new > 0)
> -				strbuf_addf(&buf, "%s (new)", patch.new_name);
> +				strbuf_addf(&buf, "%s (new %s)",
> +						patch.new_name,
> +						patch.new_oid_prefix);
>  			else if (patch.is_delete > 0)
> -				strbuf_addf(&buf, "%s (deleted)", patch.old_name);
> +				strbuf_addf(&buf, "%s (deleted %s)",
> +						patch.old_name,
> +						patch.old_oid_prefix);
>  			else if (patch.is_rename)
> -				strbuf_addf(&buf, "%s => %s", patch.old_name, patch.new_name);
> +				strbuf_addf(&buf, "%s => %s (%s..%s)",
> +						patch.old_name,
> +						patch.new_name,
> +						patch.old_oid_prefix,
> +						patch.new_oid_prefix);
>  			else
> -				strbuf_addstr(&buf, patch.new_name);
> +				strbuf_addf(&buf, "%s (%s..%s)",
> +						patch.new_name,
> +						patch.old_oid_prefix,
> +						patch.new_oid_prefix);
>
>  			free(current_filename);
>  			if (patch.is_delete > 0)
>
>

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Konstantin Ryabitsev]

On Tue, Oct 22, 2019 at 09:18:35PM +0200, Johannes Schindelin wrote:
>As to recreating blobs from mails: Wow. That's quite a length you're
>going, and I think it is a shame that you have to. If only every
>contribution came accompanied with a pullable branch in a public
>repository.

Trouble is, those public repositories are transient and may be gone soon 
after the pull request is performed. The goal is to be able to 
reconstruct full code history prior to when it is merged into the 
official repository.

>Instead, we will have to rely on your centralized, non-distributed
>service...

It's actually neither, because mirroring and distributing public-inbox 
repositories is already easy, and we hope to make it easier in the near 
future.

-K

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Eric Wong]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi Eric,
> 
> 
> On Thu, 17 Oct 2019, Eric Wong wrote:
> 
> > (WIP, mostly stream-of-concious notes + reasoning)
> >
> > When using "git format-patch --range-diff", the pre and
> > post-image blob OIDs are in each email, while the exact
> > commit OIDs are rarely shared via emails (only the tip
> > commit from "git request-pull").
> >
> > These blob OIDs make it easy to search for or lookup the
> > full emails which create them, or the blob itself once
> > it's fetched via git.
> >
> > public-inbox indexes and allows querying specifically for blob
> > OIDs via dfpre:/dfpost: since June 2017.  As of Jan 2019,
> > public-inbox also supports recreating blobs out of patch emails
> > (querying internally with dfpre:/dfpost: and doing "git apply")
> >
> > Searching on these blob OIDs also makes it easier to find
> > previous versions of the patch sets using any mail search
> > engine.
> >
> > Future changes to public-inbox may allow generating custom
> > diffs out of any blobs it can find or recreate.
> >
> > Most of this is pretty public-inbox-specific and would've
> > made some future changes to public-inbox much easier....
> > (if we had this from the start of range-diff).
> >
> > Unfortunately, it won't help with cases where range-diffs
> > are already published, but range-diff isn't too old.
> 
> I guess your patch won't hurt.

Cool, will update tests and resend.

> As to recreating blobs from mails: Wow. That's quite a length you're
> going, and I think it is a shame that you have to. If only every
 contribution came accompanied with a pullable branch in a public
> repository.

What Konstantin said about git repos being transient.
It wasn't too much work to recreate those blobs from
scratch since git-apply has done it since 2005.
Just add search :)

We could get around transient repos with automatic mirroring
bots which never deletes or overwrites anything published.
That includes preserving pre-force-push data in case of
force pushes.

> Instead, we will have to rely on your centralized, non-distributed
> service...

I'm curious how you came to believe that, since that's the
opposite of what public-inbox has always been intended to be.

The only thing that's centralized and not reproducible by
mirrors is the domain name (and I also have Tor .onion mirrors
with no dependency on ICAAN).  Memorable naming is a tricky
problem in decentralized systems, though...

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Junio C Hamano]

Eric Wong <e@80x24.org> writes:

> What Konstantin said about git repos being transient.
> It wasn't too much work to recreate those blobs from
> scratch since git-apply has done it since 2005.

;-)

> We could get around transient repos with automatic mirroring
> bots which never deletes or overwrites anything published.
> That includes preserving pre-force-push data in case of
> force pushes.
>
>> Instead, we will have to rely on your centralized, non-distributed
>> service...
>
> I'm curious how you came to believe that, since that's the
> opposite of what public-inbox has always been intended to be.

I think the (mis)perception comes from the fact that the website and
the newsfeed you give are both too easy to use and directly attract
end users, instead of enticing them to keep their own mirrors for
offline use.

Thanks for injecting dose of sanity.

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > I guess your patch won't hurt.
> 
> Cool, will update tests and resend.

Turns out the t3206 "trivial reordering" case is way too noisy.
I might need to change diff.c to support this case; will update
in a few days or week.

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Johannes Schindelin]

Hi,

On Wed, 23 Oct 2019, Junio C Hamano wrote:

> Eric Wong <e@80x24.org> writes:
>
> > What Konstantin said about git repos being transient.
> > It wasn't too much work to recreate those blobs from
> > scratch since git-apply has done it since 2005.
>
> ;-)
>
> > We could get around transient repos with automatic mirroring
> > bots which never deletes or overwrites anything published.
> > That includes preserving pre-force-push data in case of
> > force pushes.
> >
> >> Instead, we will have to rely on your centralized, non-distributed
> >> service...
> >
> > I'm curious how you came to believe that, since that's the
> > opposite of what public-inbox has always been intended to be.
>
> I think the (mis)perception comes from the fact that the website and
> the newsfeed you give are both too easy to use and directly attract
> end users, instead of enticing them to keep their own mirrors for
> offline use.
>
> Thanks for injecting dose of sanity.

Maybe your dose of sanity can inject a statement about the case when
public-inbox.org/git differs from a mirror, and not in a
fast-forwardable way? What is the authoritative source of truth, then?

Ciao,
Dscho

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Eric Wong]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 23 Oct 2019, Junio C Hamano wrote:
> > Eric Wong <e@80x24.org> writes:
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > >> Instead, we will have to rely on your centralized, non-distributed
> > >> service...
> > >
> > > I'm curious how you came to believe that, since that's the
> > > opposite of what public-inbox has always been intended to be.
> >
> > I think the (mis)perception comes from the fact that the website and
> > the newsfeed you give are both too easy to use and directly attract
> > end users, instead of enticing them to keep their own mirrors for
> > offline use.
> >
> > Thanks for injecting dose of sanity.
> 
> Maybe your dose of sanity can inject a statement about the case when
> public-inbox.org/git differs from a mirror, and not in a
> fast-forwardable way? What is the authoritative source of truth, then?

Why does authoritative source of truth matter?  My
anti-authoritarian ethos is what drew me to DVCS in the first
place.

If senders want to attest to the integrity of their messages;
they can sign, and/or publish a copy/log of their sent messages
on their homepage/social media/whatever.  That's up to THEM,
not anybody else.

If somebody wants to fork public-inbox.org/git and run
public-inbox-watch from their own Maildir, they're more than
welcome to.

If somebody wants to write their own importers since they don't
like the code I write, they are more than welcome to.  There's
already mail-archive.com, marc.info, news.gmane.org (which
public-inbox.org/git forked from) and some others.

Going farther, if people want to fork entire mailing lists and
communities, they should be able to do so.  I don't like mail
subscriber lists being centralized on any host, either.

I have never, ever asked anybody to trust me or public-inbox;
in fact, I've stated the opposite and will continue to do so.

Re: [RFC/WIP] range-diff: show old/new blob OIDs in comments

[Johannes Schindelin]

Hi Eric,

On Fri, 25 Oct 2019, Eric Wong wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Wed, 23 Oct 2019, Junio C Hamano wrote:
> > > Eric Wong <e@80x24.org> writes:
> > > > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > >> Instead, we will have to rely on your centralized, non-distributed
> > > >> service...
> > > >
> > > > I'm curious how you came to believe that, since that's the
> > > > opposite of what public-inbox has always been intended to be.
> > >
> > > I think the (mis)perception comes from the fact that the website and
> > > the newsfeed you give are both too easy to use and directly attract
> > > end users, instead of enticing them to keep their own mirrors for
> > > offline use.
> > >
> > > Thanks for injecting dose of sanity.
> >
> > Maybe your dose of sanity can inject a statement about the case when
> > public-inbox.org/git differs from a mirror, and not in a
> > fast-forwardable way? What is the authoritative source of truth, then?
>
> Why does authoritative source of truth matter?  My
> anti-authoritarian ethos is what drew me to DVCS in the first
> place.
>
> If senders want to attest to the integrity of their messages;
> they can sign, and/or publish a copy/log of their sent messages
> on their homepage/social media/whatever.  That's up to THEM,
> not anybody else.
>
> If somebody wants to fork public-inbox.org/git and run
> public-inbox-watch from their own Maildir, they're more than
> welcome to.

I am _more_ than happy to rely on public-inbox.org/git. And I will never
kid myself about relying on a central service, is all.

> If somebody wants to write their own importers since they don't
> like the code I write, they are more than welcome to.  There's
> already mail-archive.com, marc.info, news.gmane.org (which
> public-inbox.org/git forked from) and some others.
>
> Going farther, if people want to fork entire mailing lists and
> communities, they should be able to do so.  I don't like mail
> subscriber lists being centralized on any host, either.
>
> I have never, ever asked anybody to trust me or public-inbox;
> in fact, I've stated the opposite and will continue to do so.

Well, too bad. I trust you, Eric. I do trust you and will probably
continue to trust you because I don't expect you to do anything, ever,
to break that trust. So far, you haven't disappointed me even a single
time, and we've concurrently been Git contributors for, sheesh, has it
already been almost 14 years? I have benefitted from your work greatly,
mostly via `git svn` in the olden days, and I hope that I could return
the favor every once in a while.

Without public-inbox.org/git, GitGitGadget would not be possible. My
scripts to map commits to mails and vice versa (mirrored to
https://github.com/gitgitgadget/git as `refs/notes/mail-to-commit` and
`commit-to-mail`) would remain a pipe dream of mine.

(Yes, yes, there are holes in that mapping, but even if I only have to
look up manually one out of 30 mails when I want to comment on a
specific commit, that already saves me so much time, not to mention
nerves.)

So please understand that I am deeply grateful that you came up with
these projects, in particular with public-inbox. It is a life saver. I
might not share all of your philosophy regarding centralized vs
decentralized, even so, what you did helps me multiple times every
single day.

Therefore: a heart-felt Thank You, I owe you more than one,
Dscho