From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Narebski Subject: Re: [PATCH 2/7] gitweb: Support comparing blobs with different names Date: Fri, 20 Apr 2007 12:34:49 +0200 Message-ID: <200704201234.50134.jnareb@gmail.com> References: <11766699702663-git-send-email-mkoegler@auto.tuwien.ac.at> <11766699701308-git-send-email-mkoegler@auto.tuwien.ac.at> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-2" Content-Transfer-Encoding: 7bit Cc: git@vger.kernel.org To: Martin Koegler X-From: git-owner@vger.kernel.org Fri Apr 20 13:01:53 2007 Return-path: Envelope-to: gcvg-git@gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1Heqrs-0005gn-En for gcvg-git@gmane.org; Fri, 20 Apr 2007 13:01:32 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754452AbXDTLBY (ORCPT ); Fri, 20 Apr 2007 07:01:24 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754447AbXDTLBX (ORCPT ); Fri, 20 Apr 2007 07:01:23 -0400 Received: from wr-out-0506.google.com ([64.233.184.227]:37734 "EHLO wr-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753512AbXDTLBV (ORCPT ); Fri, 20 Apr 2007 07:01:21 -0400 Received: by wr-out-0506.google.com with SMTP id 76so882117wra for ; Fri, 20 Apr 2007 04:01:21 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:from:to:subject:date:user-agent:cc:references:in-reply-to:mime-version:content-disposition:content-type:content-transfer-encoding:message-id; b=ZeX0SXWSTOjpAcIYlCf3MAeYdaHeULuUfBLtTHRfwOsN4rcu+fZaSuRyOydH9JbGQct0sGxQW5jb/ks4rInuTYgIQpYxpPtUiUCNxL7euwg9RieKyBBijbBSdUwRb2l9UYzdjx9ebECjrFGatfOBcXW6hyk+bR/pNRTK6q5QwxA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:from:to:subject:date:user-agent:cc:references:in-reply-to:mime-version:content-disposition:content-type:content-transfer-encoding:message-id; b=U3AIIBBgwUCgA1eJLQMFnMZmNqzk3M5cE75x7oQ80QCHjBFRSjF4wH2EyhU9rqFhqRtCjDgzr9ari270xcB+KD3gzFiPeAhZ9T6Opz1Rbi+XaGngteqslfeOTEhx1Frd4uusFYkc2fu5om89SICgcFZmgOFdBM8N1ys97ihG0J8= Received: by 10.66.220.17 with SMTP id s17mr2515004ugg.1177066880265; Fri, 20 Apr 2007 04:01:20 -0700 (PDT) Received: from host-89-229-25-173.torun.mm.pl ( [89.229.25.173]) by mx.google.com with ESMTP id k2sm4241473ugf.2007.04.20.04.01.18; Fri, 20 Apr 2007 04:01:18 -0700 (PDT) User-Agent: KMail/1.9.3 In-Reply-To: <11766699701308-git-send-email-mkoegler@auto.tuwien.ac.at> Content-Disposition: inline Sender: git-owner@vger.kernel.org Precedence: bulk X-Mailing-List: git@vger.kernel.org Archived-At: Martin Koegler wrote: > Currently, blobdiff can only compare blobs with different file > names, if no hb/hpb parameters are present. > > This patch adds support for comparing two blobs specified by any > combination of hb/f/h and hpb/fp/hp. > > Signed-off-by: Martin Koegler > --- Shouldn't the comment below be also a part of commit message (perhaps changed to use passive form)? > I unified all blobdiff variants and added support for comparing blobs > with different names. > > If h/hp parameter are missing, I need to generate them with > git_get_hash_by_path, as the are needed for the html header, which is > generated before parsing the git-diff output. > > I currently ignore all mode changes, as they are part of the tree. I > don't think that displaying a mode change message justifes two call to > git-ls-tree for each blob diff (Currently it only calls git-ls-tree > for each missing h/hp parameter). I have mixed feelings about this. On the one hand side the blobdiff generation is simplified as we have only one codepath instead of two, and removed codepath in rare cases (not generated by gitweb currently) might return incorrect results in the case of requesting diff between two arbitrary and unrelated files. On the other hand side we do loose some information, contained in extended diff header, like mode changes, renames and such. It's a pity that extended sha-1 syntax to specify blobs, namely the : syntax is opaque, and git-diff machinery sees only the result of it, as if it was called directly with the sha-1 of blob. It would be nice if "git diff : :" took the information about file name and mode (permissions) from and and included such information in extended git diff header. Currently we have: $ git diff -p HEAD^ HEAD -- gitweb/test/hardlink diff --git a/gitweb/test/hardlink b/gitweb/test/hardlink old mode 100644 new mode 100755 but $ git diff -p HEAD^:gitweb/test/hardlink HEAD:gitweb/test/hardlink returns empty diff. I'd rather have core git support for this first before changing how git_blobdiff is implemented in gitweb... -- Jakub Narebski Poland