Обсуждение: Weird mangling of a commit log entry in gitweb summary
My recent commit 5e692dcacabd5dbc8ccfb9e37a2d26a574b6dea6
looks like this in "git log":
commit 5e692dcacabd5dbc8ccfb9e37a2d26a574b6dea6
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat Jul 16 12:26:19 2022 -0400
Remove postmaster.c's reset_shared() wrapper function.
In gitweb, it shows that way too if you drill down to the
individual commit, but in the summary
https://git.postgresql.org/gitweb/?p=postgresql.git;a=summary
it looks like
6 hours ago Tom Lane Remove postc's reset_shared() wrapper function. commit | commitdiff | tree
What's up with that?
regards, tom lane
On Sun, Jul 17, 2022 at 1:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > My recent commit 5e692dcacabd5dbc8ccfb9e37a2d26a574b6dea6 > looks like this in "git log": > > commit 5e692dcacabd5dbc8ccfb9e37a2d26a574b6dea6 > Author: Tom Lane <tgl@sss.pgh.pa.us> > Date: Sat Jul 16 12:26:19 2022 -0400 > > Remove postmaster.c's reset_shared() wrapper function. > > In gitweb, it shows that way too if you drill down to the > individual commit, but in the summary > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=summary > > it looks like > > 6 hours ago Tom Lane Remove postc's reset_shared() wrapper function. commit | commitdiff | tree > > What's up with that? I have no idea why but it's clearly getting confused by the single quote. But it doesn't appear to do that in other places where there's a single quote in there. I see nothing strange with the commit message itself, and the cgit view of it shows nothing strange either... -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
On 2022-Jul-17, Magnus Hagander wrote: > On Sun, Jul 17, 2022 at 1:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > 6 hours ago Tom Lane Remove postc's reset_shared() wrapper function. commit | commitdiff | tree > > > > What's up with that? > > I have no idea why but it's clearly getting confused by the single > quote. But it doesn't appear to do that in other places where there's > a single quote in there. I see nothing strange with the commit message > itself, and the cgit view of it shows nothing strange either... How do you know it's the single quote? The problem is not adjacent to that. Maybe it's replacing the string "branch name followed by period" with an empty string. If you go to https://git.postgresql.org/gitweb/?p=postgresql.git;a=shortlog;pg=11 you'll see the commit title for 6c4a8903b93f show as: TAP tests: check for postpid anyway when "pg_ctl start... which was: TAP tests: check for postmaster.pid anyway when "pg_ctl start" fails. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Hi, On Mon, Jul 18, 2022 at 09:34:23AM +0200, Alvaro Herrera wrote: > On 2022-Jul-17, Magnus Hagander wrote: > > > On Sun, Jul 17, 2022 at 1:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > 6 hours ago Tom Lane Remove postc's reset_shared() wrapper function. commit | commitdiff | tree > > > > > > What's up with that? > > > > I have no idea why but it's clearly getting confused by the single > > quote. But it doesn't appear to do that in other places where there's > > a single quote in there. I see nothing strange with the commit message > > itself, and the cgit view of it shows nothing strange either... > > How do you know it's the single quote? The problem is not adjacent to > that. Maybe it's replacing the string "branch name followed by period" > with an empty string. It looks like some gitweb's heuristics to try to reduce the title name: https://github.com/git/git/blob/master/gitweb/gitweb.perl#L3570-L3572 > if (length($title) > 50) { > $title =~ s/(master|www|rsync)\.//; > }
On Mon, Jul 18, 2022 at 10:19 AM Julien Rouhaud <rjuju123@gmail.com> wrote: > > Hi, > > On Mon, Jul 18, 2022 at 09:34:23AM +0200, Alvaro Herrera wrote: > > On 2022-Jul-17, Magnus Hagander wrote: > > > > > On Sun, Jul 17, 2022 at 1:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > > > 6 hours ago Tom Lane Remove postc's reset_shared() wrapper function. commit | commitdiff | tree > > > > > > > > What's up with that? > > > > > > I have no idea why but it's clearly getting confused by the single > > > quote. But it doesn't appear to do that in other places where there's > > > a single quote in there. I see nothing strange with the commit message > > > itself, and the cgit view of it shows nothing strange either... > > > > How do you know it's the single quote? The problem is not adjacent to > > that. Maybe it's replacing the string "branch name followed by period" > > with an empty string. > > It looks like some gitweb's heuristics to try to reduce the title name: > > https://github.com/git/git/blob/master/gitweb/gitweb.perl#L3570-L3572 > > > if (length($title) > 50) { > > $title =~ s/(master|www|rsync)\.//; > > } Hah. I searched that code for a lot of things. but clearly not that one. AFAICT this is not functionality that can be turned off, it's all hardcoded both in what it searches and when it does it. :/ I doubt it's worth forking and maintaining a fork just to handle this situation given how seldom it shows up. -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
On Mon, Jul 18, 2022 at 10:27:47AM +0200, Magnus Hagander wrote: > > AFAICT this is not functionality that can be turned off, it's all > hardcoded both in what it searches and when it does it. :/ > > I doubt it's worth forking and maintaining a fork just to handle this > situation given how seldom it shows up. +1, especially since it was introduced 17 years ago and this is probably the first time someone notice it in our instance.
On 7/18/22 4:37 AM, Julien Rouhaud wrote: > On Mon, Jul 18, 2022 at 10:27:47AM +0200, Magnus Hagander wrote: >> >> AFAICT this is not functionality that can be turned off, it's all >> hardcoded both in what it searches and when it does it. :/ >> >> I doubt it's worth forking and maintaining a fork just to handle this >> situation given how seldom it shows up. > > +1, especially since it was introduced 17 years ago and this is probably the > first time someone notice it in our instance. Maybe it's worth reporting upstream? It seems like it's doing something very opinionated that may not make sense to projects using git. Jonathan
Вложения
On Mon, Jul 18, 2022 at 10:01:37AM -0400, Jonathan S. Katz wrote: > On 7/18/22 4:37 AM, Julien Rouhaud wrote: > > On Mon, Jul 18, 2022 at 10:27:47AM +0200, Magnus Hagander wrote: > > > > > > AFAICT this is not functionality that can be turned off, it's all > > > hardcoded both in what it searches and when it does it. :/ > > > > > > I doubt it's worth forking and maintaining a fork just to handle this > > > situation given how seldom it shows up. > > > > +1, especially since it was introduced 17 years ago and this is probably the > > first time someone notice it in our instance. > > Maybe it's worth reporting upstream? It seems like it's doing something very > opinionated that may not make sense to projects using git. Trying to guess what that code was originally supposed to do (as I can't find anything apart from the "v203" original commit message), I'd say that it was originally thought to shorten addresses in commit messages like Merge master.kernel.org:/... Merge rsync://rsync.kernel.org/... (or other domains) but as written it will easily lead to nonsensical shortened title. Naively, something as simple as adding a leading whitespace in the regex would remove 90% of wrong matches in our repo, and making sure it's not followed by another whitespace would remove all of them AFAICS. I will try to see if upstream would welcome such a change.
Hi, On Mon, Jul 18, 2022 at 11:56:56PM +0800, Julien Rouhaud wrote: > > I will try to see if upstream would welcome such a change. After some discussions it turned out that upstream doesn't have a gitweb instance running anymore, so they preferred to remove the whole title shortening block. The patch just landed on the master branch (1) and should be available in the next release. [1] https://github.com/git/git/commit/75707da4fa4105c174017d079786e5bba79a96f6
On Sat, Aug 6, 2022 at 6:24 AM Julien Rouhaud <rjuju123@gmail.com> wrote: > > Hi, > > On Mon, Jul 18, 2022 at 11:56:56PM +0800, Julien Rouhaud wrote: > > > > I will try to see if upstream would welcome such a change. > > After some discussions it turned out that upstream doesn't have a gitweb > instance running anymore, That's interesting to know indeed. I thought they still had both. But I see now that git itself is purely on github, and the linux kernel seems to only use cgit. > so they preferred to remove the whole title > shortening block. The patch just landed on the master branch (1) and should > be available in the next release. > > [1] https://github.com/git/git/commit/75707da4fa4105c174017d079786e5bba79a96f6 It'll take a long time for that to trickle down to our install, but I think it's still worth just waiting for it instead of maintaining even a backpatch -- due to how little this has been a problem. -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
Hi, On Sat, Aug 06, 2022 at 01:22:38PM +0200, Magnus Hagander wrote: > On Sat, Aug 6, 2022 at 6:24 AM Julien Rouhaud <rjuju123@gmail.com> wrote: > > > > > > After some discussions it turned out that upstream doesn't have a gitweb > > instance running anymore, > > That's interesting to know indeed. I thought they still had both. But > I see now that git itself is purely on github, and the linux kernel > seems to only use cgit. Note that gitweb is still being actively maintained, so I'm assuming it's still popular enough on repo of reasonable size. > > so they preferred to remove the whole title > > shortening block. The patch just landed on the master branch (1) and should > > be available in the next release. > > > > [1] https://github.com/git/git/commit/75707da4fa4105c174017d079786e5bba79a96f6 > > It'll take a long time for that to trickle down to our install, but I > think it's still worth just waiting for it instead of maintaining even > a backpatch -- due to how little this has been a problem. Agreed, we're now aware of it and know it's going to be eventually fixed on our instance.