Pull request: Use userdiff configuration

Pull request: Use userdiff configuration

[me]

(I don't mean to spam, but I can't see this initial message on the list, so I'm re-sending. Apologies if it's extraneous.)

Hi Lars and friends,

Yesterday I set up cgit for my company's private repositories, and have it working great. We mainly have Objective-C projects, and I found that cgit wasn't using the same userdiff driver as git does. This affects the display of the funcname in diff contexts. git-diff gives this:

diff --git a/Library/LISortManagedObject.m b/Library/LISortManagedObject.m
index d78d17d..8a8255e 100644
--- a/Library/LISortManagedObject.m
+++ b/Library/LISortManagedObject.m
@@ -36,6 +36,7 @@ + (NSArray *)defaultSortDescriptorsArray;
[ changes inside the +defaultSortDescriptorsArray method ]

I have to explicitly tell git to use the objc diff header, through gitattributes:
*.m diff=objc
(the patterns for which are built in to git, in userdiff.c).

For non-built-in diff types, you can also specify a "textconv" filter, through which both versions are piped before diffing. I spent some time today making one that has nice output for Cocoa's serialized interface files (.xib).

So I have some patches to cgit to respect both the userdiff xfuncname and textconv. I made them relative to v0.9, and have rebased it onto master. The branch is here: <https://github.com/jmah/cgit/commits/userdiff>


And in case anyone uses them, my xib textconv filter is here: <https://github.com/jmah/xibition>



Jonathon Mah
me at JonathonMah.com

Pull request: Use userdiff configuration

[hjemli]

On Tue, Apr 5, 2011 at 13:38, Jonathon Mah <me at jonathonmah.com> wrote:
> I have to explicitly tell git to use the objc diff header, through gitattributes:
> *.m diff=objc
> (the patterns for which are built in to git, in userdiff.c).
>
> For non-built-in diff types, you can also specify a "textconv" filter, through which
> both versions are piped before diffing. I spent some time today making one that
> has nice output for Cocoa's serialized interface files (.xib).
>
> So I have some patches to cgit to respect both the userdiff xfuncname and
> textconv. I made them relative to v0.9, and have rebased it onto master. The branch
> is here: <https://github.com/jmah/cgit/commits/userdiff>
>

Thanks. The changes looks good, but I'm a bit concerned about textconv
security. Maybe this feature should be disabled by default?

-- 
larsh

Pull request: Use userdiff configuration

[me]

Lars,

On 2011-04-07, at 03:30, Lars Hjemli wrote:

>> So I have some patches to cgit to respect both the userdiff xfuncname and
>> textconv.
> 
> Thanks. The changes looks good, but I'm a bit concerned about textconv
> security. Maybe this feature should be disabled by default?


Yes, I would suggest that. To clarify, this allows people with repo write access to instruct cgit to run an arbitrary command.

So I think I'll look at making it a per-repo setting (also with a global), defaulting to off. Another idea (that I don't like as much): We could restrict textconv values to those in the system-wide git config.

I'll try to make some time for this by tomorrow.



Jonathon Mah
me at JonathonMah.com

Pull request: Use userdiff configuration

[me]

Lars,

On 2011-04-07, at 03:30, Lars Hjemli wrote:

> On Tue, Apr 5, 2011 at 13:38, Jonathon Mah <me at jonathonmah.com> wrote:
> 
>> So I have some patches to cgit to respect both the userdiff xfuncname and
>> textconv.
> 
> Thanks. The changes looks good, but I'm a bit concerned about textconv
> security. Maybe this feature should be disabled by default?

I rethought things a bit a decided to change my approach. Along with trying to minimize configuration options, while I was adding the textconv support in the first place, I had some serious performance issues. The textconv filter took around 2 seconds per file, so diff pages stalled for a while. But much worse, I initially tried "enable-log-linecount" on, and the line stats were holding up the log page for ~40 seconds. So once my textconv filter was good, I turned on "cachetextconv" in the git config.

I've re-written the patch so cgit will use the cached textconv blobs. If a cached blob is not available for at least one side of a diff, the diff will instead be of the files on disk. This way, cgit will never execute an external handler, and so will never be held up by a slow one.

If repo owners want textconv, they can turn on cachetextconv and set a post-receive hook to update the cache.

(A not-particularly-efficient, but trivial post-receive hook is:)
while read oldrev newrev ref; do
	git log -u $newrev &>/dev/null
done



Jonathon Mah
me at JonathonMah.com

Pull request: Use userdiff configuration

[me]

On 2011-04-04, at 01:33, Jonathon Mah wrote:

> [?]
> So I have some patches to cgit to respect both the userdiff xfuncname and textconv. I made them relative to v0.9, and have rebased it onto master. The branch is here: <https://github.com/jmah/cgit/commits/userdiff>

I have also added a fix for the default value of module-link. It's in the same branch (userdiff).



Jonathon Mah
me at JonathonMah.com