git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [BUG/RFC] Raw diff output format (git-diff-tree) and --relative[=<path>] option
Date: Thu, 8 Jul 2010 13:00:17 +0200	[thread overview]
Message-ID: <201007081300.18712.jnareb@gmail.com> (raw)
In-Reply-To: <201007051744.54266.jnareb@gmail.com>

I wanted to get non-recursive raw diff (difftree), but for a given
subdirectory and not starting from root.

In February 2008 Junio C Hamano added support for --relative and 
--relative=<path> options to git-diff:
* cd676a5 (diff --relative: output paths as relative
           to the current subdirectory, 2008-02-12)
* c0cb4a0 (diff --relative: help working in a bare repository,
           2008-02-13)

>From the commit message for cd676a5 (the c0cb4a0 just allows the 
--relative option to say which subdirectory to pretend to be in,
i.e. adds the --relative=<path> version) it looks like this option
was intended for patch (-p) output format.

There was added support also for raw output format, so both 
'git diff-tree' and 'git diff --raw' works with --relative option,
but support for this is buggy, and in my opinion wrong way around.


The '--relative[=<path>]' option works currently like this.  First,
if git command is invoked in subdirectory the diffopts structure
gets set prefix and prefix_length (in init_revisions).  If 
--relative[=<path>] option is passed, git sets RELATIVE_NAME flag,
and if there is argument, prefix is set to it.  Later git removes
prefix (sets it to NULL) if RELATIVE_NAME option is not set.

What's important in this step is that prefix is set without any 
normalization from --relative=<path> argument, while (from what
I understand) if it is set from current directory i.e. with --relative 
option without argument, it is set with trailing slash.  So using
--relative=sub instead of --relative=sub/ might be thought as user
error... but I think here it is lack of robustness in the API.


Then comes the filtering part.  In functions such as diff_change
or stuff_change, if path(s) does not begin with prefix, they are
simply skipped.  This solution limits --relative[=<path>] to work
only with *recursive* (full tree) output, such as patch output format,
or "git diff", or "git diff-tree -r" (and "git diff-tree -t", which
implies "-r").


Last there is filename munging, done using strip_prefix function.
This is done using prefix_length only, and that is the cause of
the bug:
  $ git diff-tree --abbrev -r --raw HEAD --relative=sub
  a3a8425fe5496c61921010cb1e7b455a1f52bb86
  :100644 100644 d90bda0... cefcae0... M	/quux

if one uses '--relative=sub' instead of '--relative=sub/'.


What I'd like to see for the raw output format is to work with 
--relative[=<path>] to work as if <path> was top directory of 
repository.  For example for diff between two trees

  $ git diff-tree A B --relative=sub/

would be equivalent to running

  $ git diff-tree A:sub/ B:sub/

*This* could be done, I think, by modifying diff_tree_sha1 to do a diff 
betweem A:sub/ and B:sub/ (taking 'sub/' from prefix) and unsetting 
prefix (setting prefix to NULL and prefix_length to 0).  But that would 
work only in the case that can be reducted to diff between two tree 
objects.  This wouldn't work for diff in raw output format between tree 
and working area, tree and index, or index and working area.

Is the idea of automagically translating <sha1> into <sha1>:<prefix>
to support --relative / relative=<prefix> well in raw diff output format
a good idea?  Or should I search for another solution.

I also do not know code enough (and it is not simple) to guess how
one would go with the same result for diff between trees, index, and
working area files.

BTW. the approach proposed here has the advantage that for B:<sub>,
if <sub> does not exist in B, we can try to do what 'subtree' merge 
strategy does (and what wholesame directory rename detection did),
namely try to find given directory under different path (like for 
example subtree-merged git-gui and gitk).

-- 
Jakub Narebski
Poland

  reply	other threads:[~2010-07-08 11:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-05  8:15 [BUG] Spurious leading '/' in filename in "git diff --raw --relative=<subdirectory>" Jakub Narebski
2010-07-05 15:44 ` Jakub Narebski
2010-07-08 11:00   ` Jakub Narebski [this message]
2010-07-08 11:41     ` [BUG/RFC] Raw diff output format (git-diff-tree) and --relative[=<path>] option Jeff King
2010-07-08 12:11       ` Jakub Narebski
2010-07-08 12:19       ` Jakub Narebski
2010-07-08 14:23         ` Jeff King
2010-07-08 14:56           ` Jakub Narebski
2010-08-09 14:50             ` Jeff King
2010-08-09 14:59             ` Jeff King

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=201007081300.18712.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).