I'm missing isofs.h

In a current tree, using git-pasky-0.7:

bix:/usr/src/git26> cat .git/tags/v2.6.12-rc3 
a2755a80f40e5794ddc20e00f781af9d6320fafb
bix:/usr/src/git26> git diff -r v2.6.12-rc3|grep isofs.h
+#include "isofs.h"
 #include "zisofs.h"
+#include "isofs.h"
+#include "isofs.h"
+#include "isofs.h"
 #include "zisofs.h"
+#include "isofs.h"
+#include "isofs.h"
+#include "isofs.h"
+#include "isofs.h"

That diff should have included the addition of the new isofs.h, but it isn't there.

On Tue, Apr 26, 2005 at 09:43:38PM -0700, Andrew Morton wrote:

It looks like git-pasky-0.7 doesn't include the following commit, but there are also several other diff and merge related fixes that were added since then.

Jan

commit 65bc81d6fef619d7aadc5c7116be52860539f17a tree 9adb399af84228740555d732732983b7a02b019d parent 93256315b2444601a35484f4fb76cd5723284201 author Petr Baudis <pasky@ucw.cz> Sat, 23 Apr 2005 18:05:07 -0700 committer Linus Torvalds <torvalds@ppc970.osdl.org> Sat, 23 Apr 2005 18:05:07 -0700

    [PATCH] Fix broken diff-cache output on added files

    Added files were errorneously reported with the - prefix by diff-cache,
    obviously leading to great confusion.

    Signed-off-by: Petr Baudis <pasky@ucw.cz>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Dear diary, on Wed, Apr 27, 2005 at 02:58:44PM CEST, I got a letter where Jan Harkes <jaharkes@cs.cmu.edu> told me that...

Why do you think it doesn't include it? I can see the fix in the code.

On Wed, Apr 27, 2005 at 03:58:41PM +0200, Petr Baudis wrote:

I looked through the output of cg-log, which I thought had at least some ordering, and that commit showed up as more recent than the pasky-0.7 entry. It looks like the same change is also part of pasky-0.7, but with a different commit-id. Sorry about the confusion.

In any case, when I use
    cg-diff -r a2755a80f40e5794ddc20e00f781af9d6320fafb: | grep isofs.h

the missing file does show up,
    ...
    Index: fs/isofs/isofs.h
    +++ fd1621a8c03331bd78abfe52c8c385977d0a9729/fs/isofs/isofs.h (mode:100644 sha1:9ce7b51fb6141ea6b82d85687d490c74755591fb)
    ...

so either I'm missing some subtle command line error (missing ':' after the tag-id?) or the problem was fixed by some other change. So I looked through the logs to see if there was anything obvious and the commit I mentioned looked promising.

Jan

On Wed, Apr 27, 2005 at 12:43:51PM -0400, Jan Harkes wrote:

Looks like that actually is the problem, when I run cg-diff -r, but leave out the ':' the final output does not include the added isofs.h file.

Jan

Andrew Morton wrote:

I'm seeing unexplained behaviour using the above technique, and I'm also seeing fs/isofs/isofs.h as missing, along with seven other changes.

I'm using the latest cogito release: [steven@spc0 COGITO]$ cg-version cogito-0.8 (3e0fb979cc7541506ec660ab66b83d8120da6d57)

I updated my linux-2.6 repo with cg-update origin, and then created a current linux-2.6 tree using cg-export. I diffed that exported tree with 2.6.12-rc3 and saved the result as "a.diff".

I created another diff using Andrew's technique using cg-diff and saved that to "b.diff".

I had expected that a.diff and b.diff to be the same, but they are not, and the AWOL file fs/isofs/isofs.h is among the missing using Andrew's technique.

Here are the details.

Steven

[steven@spc0 linux-2.6]$ cat .git/HEAD e8108c98dd6d65613fa0ec9d2300f89c48d554bf

[steven@spc0 linux-2.6]$ fsck-cache --tags tagged commit a2755a80f40e5794ddc20e00f781af9d6320fafb (v2.6.12-rc3) in 0397236d43e48e821cce5bbe6a80a1a56bb7cc3a tagged commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 (v2.6.12-rc2) in 9e734775f7c22d2f89943ad6c745571f1930105f expect dangling commits - potential heads - due to lack of head information dangling commit e8108c98dd6d65613fa0ec9d2300f89c48d554bf

[steven@spc0 linux-2.6]$ cg-export ../linux-2.6-current [steven@spc0 linux-2.6]$ cg-diff -r v2.6.12-rc3 >../b.diff [steven@spc0 linux-2.6]$ cd ..

[steven@spc0 COGITO]$ diff -urN linux-2.6.12-rc3 linux-2.6-current >a.diff #note that linux-2.6.12-rc3 was created by patch from kernel.org.

[steven@spc0 COGITO]$ diffstat a.diff >a.diffstat
[steven@spc0 COGITO]$ diffstat b.diff >b.diffstat
[steven@spc0 COGITO]$ tail -n 1 a.diffstat
  199 files changed, 3083 insertions(+), 1601 deletions(-)
[steven@spc0 COGITO]$ tail -n 1 b.diffstat
  191 files changed, 2539 insertions(+), 1540 deletions(-)
[steven@spc0 COGITO]$ diff -u a.diffstat b.diffstat
--- a.diffstat  2005-04-27 09:07:04.000000000 -0600
+++ b.diffstat  2005-04-27 09:07:14.000000000 -0600

Steven Cole wrote:

Jan Harkes has found the problem to be a missing ':' at the end of the tag.

Steven

Dear diary, on Wed, Apr 27, 2005 at 06:43:38AM CEST, I got a letter where Andrew Morton <akpm@osdl.org> told me that...

Ouch.

Well, using -r v2.6.12-rc3: is a workable workaround, but this is a problem. With the trailing :, you are diffing against your latest commit, whilst without the trailing :, you are diffing against your working tree. ;-)

The problem is in how the latter is implemented:

        export GIT_INDEX_FILE=$(mktemp -t gitdiff.XXXXXX)
        cp .git/index $GIT_INDEX_FILE
        read-tree -m $(tree-id "$id1")
        update-cache --refresh
        tree=$(tree-id "$id1")
	diff-cache -r -z $tree | xargs -0 cg-Xdiffdo ...

So, we are recording our adds to the index cache, but here we use a different one with the adds are not recorded - so diff-cache won't catch them.

So I fixed this by doing diff-cache --cached and updating the tmp index by the +- entries. Pushed out, thanks.

On Thu, 28 Apr 2005, Petr Baudis wrote:

Umm. 

Why do you create the new index file in the first place?

If you're diffing against the current working tree, you should just use your current index file, no?

And to get the difference between an old tree and the current working tree, you should just need to do

	diff-cache -r -z $tree

and you're done.

In other words, that temporary index file really isn't needed in the "diff-cache" world. It can diff the current index against _any_ old tree.

And together with Junio's stuff from today, you can literally just do

	diff-cache -p $tree

and you're done - it diffs any release "$tree" against the current state.

And if you want to diff against the current head (rather than current working state), a simple

	diff-tree -p $tree $(cat .git/HEAD)

should do it.

			Linus

Dear diary, on Thu, Apr 28, 2005 at 02:19:07AM CEST, I got a letter where Linus Torvalds <torvalds@osdl.org> told me that...

Oops, you are of course right. This was a stupid leftover from the pre-diff-cache days, and since then I never looked at this code from sufficient distance to see it. ;-)

Actually, I can't; the patch generator is not on par with mine yet. It does not show modes and does not indicate file adds/removals by /dev/null - basically, I need something cg-patch can eat (and it should be backwards compatible). I think throwing the sha1 hashes away will not harm; I got used to the Index: field and === marker, but I don't care if I loose it.

PB> Dear diary, on Thu, Apr 28, 2005 at 02:19:07AM CEST, I got a letter PB> where Linus Torvalds <torvalds@osdl.org> told me that...

PB> Actually, I can't; the patch generator is not on par with mine yet.

That's what GIT_EXTERNAL_DIFF is there for.

PB> Actually, I can't; the patch generator is not on par with mine yet. PB> It does not show modes and does not indicate file adds/removals by PB> /dev/null - basically, I need something cg-patch can eat (and it should PB> be backwards compatible). I think throwing the sha1 hashes away will not PB> harm; I got used to the Index: field and === marker, but I don't care if PB> I loose it.

I've looked at what cg-Xdiffdo does. From the above paragraph, I sense that it does more than what cg-patch requires, so I took a look at cg-patch, too.

Can you help me verify if I understand the requirements cg-patch has on its input correctly?

 - Follow the convention of showing newly added files with
   "--- /dev/null" and removed files with "+++ /dev/null";

 - Label matches this Perl regexp:

     m|^(---|\+\+\+)\s+[^/]+\/(\S+)\s+.*mode:([0-7]{3,}).*/|

   and you only care about sign ($1), filename ($2) and mode ($3).

To illustrate, cg-Xdiffdo generates something like:

 (modified files)
 --- FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF/fs/ext3/Makefile  (mode:0644)
 +++ EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE/fs/ext3/Makefile  (mode:0664)

 (deleted files)
 --- FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF/fs/ext3/Makefile  (mode:0644)
 +++ /dev/null  (tree:EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE)

 (added files)
 --- /dev/null  (tree:EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE)
 +++ FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF/fs/ext3/Makefile  (mode:0644)

but they could be like the following to satisfy cg-patch:

 (modified files)
 --- a/fs/ext3/Makefile  (mode:0644)
 +++ b/fs/ext3/Makefile  (mode:0664)

 (deleted files)
 --- a/fs/ext3/Makefile  (mode:0644)
 +++ /dev/null

 (added files)
 --- /dev/null
 +++ b/fs/ext3/Makefile  (mode:0644)

Is my understanding correct? If so it should not be too much work to generate something like it from within the builtin stuff.

Provided if that is what the kernel folks can live with (I do see why the tool wants the mode bits, but it is unusual to see non-timestamp strings after filenames).

Linus & Andrew, is the above (second) format acceptable for the kernel work?

This patch changes the way the default arguments to diff are built when diff-cache and friends are invoked with -p and there is no GIT_EXTERNAL_DIFF environment variable. It attempts to be more cg-patch friendly by:

 - Showing diffs against /dev/null to denote added or removed
   files;

 - Showing file modes for existing files as a comment after the
   diff label.

Unfortunately with this change GIT_DIFF_CMD customization cannot be supported easily anymore, so it has been dropped. GIT_DIFF_OPTS customization to change diffs from unified to context is still there, though.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c | 56 ++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 36 insertions(+), 20 deletions(-)

# - 04/27 21:50 diff.c clean up temporary file. # + 04/27 23:18 Attempt to minimally be compatible with cg-Xdiffdo. --- k/diff.c (mode:100644) +++ l/diff.c (mode:100644)

Dear diary, on Thu, Apr 28, 2005 at 07:27:59AM CEST, I got a letter where Junio C Hamano <junkio@cox.net> told me that...

Yes; that was what the last sentence was about. ;-)

Yes.

Yes..

Yes, perfectly.

There's no reason not to get the timestamps too if you can; just put them after the attributes. They aren't in the diff now either.

I need the mode bits to set the mode right, surprisingly. :-) Yes, in part it is a leftover from the old times when we didn't just track the execute bit; I don't know if it is worth changing this.

Petr Baudis wrote:

Actually, I like having the full mode bits in there. "git" actually can be useful as a more general capability for keeping careful track of an entire tree that's NOT just source code (e.g., your entire home directory tree, so you can replicate it across machines in its current state).

I can easily imagine an option flag that stores "modes as they really are", and another that says "use the modes as they are stored". Add some support for symlinks, and you could do quite a bit. Timestamps would be ducky, too, for the same reason.

--- David A. Wheeler

On Wed, 27 Apr 2005, Junio C Hamano wrote:

The only thing my stuff needs is that it's "-p1" format, but I don't care if the prefix is the sha1 tree-name, or "a/" and "b/" or anything else (I think the current thing that the built-in stuff defaults to is a bit strange. "k/" and "l/"? I understand "a/" and "b/", and I'd even get "x/" and "y/" or "old/" and "new/", but starting counting at "l" is strange ;)

		Linus

LT> On Wed, 27 Apr 2005, Junio C Hamano wrote:

LT> The only thing my stuff needs is that it's "-p1" format, ...

Is /dev/null convention OK with you? I know it is OK for patch and cg-patch, both of which have built-in knowledge of the convention, but other tools may get confused. Here is an example of diffstat getting confused:

    $ jit-snap -v linus-mirror:0 Makefile comm-z.c
    # - [PATCH] diff-tree -p implies diff-tree -p -r
    # + JIT: indent help text from jit-snap.
    --- k/Makefile  (mode:100644)
    +++ l/Makefile  (mode:100644)
    @@ -7,7 +7,8 @@
     # BREAK YOUR LOCAL DIFFS! show-diff and anything us.....
     # break unless your underlying filesystem supports .....
    ...
    --- /dev/null
    +++ l/comm-z.c  (mode:100644)
    @@ -0,0 +1,101 @@
    +#include <stdio.h>
    +#include <string.h>
    ...

    $ jit-snap -v linus-mirror:0 Makefile comm-z.c | diffstat
    Makefile   |   15 +++++++--
    l/comm-z.c |  101 ++++++++++++++++++++++++++++++++++.....
     2 files changed, 114 insertions(+), 2 deletions(-)

LT> ... but I don't care LT> if the prefix is the sha1 tree-name, or "a/" and "b/" or anything else (I LT> think the current thing that the built-in stuff defaults to is a bit LT> strange. "k/" and "l/"? I understand "a/" and "b/", and I'd even get "x/" LT> and "y/" or "old/" and "new/", but starting counting at "l" is strange ;)

I am _very_ glad somebody finally has noticed and voiced the puzzlement. It is meant to be a gentle reminder that the tool's primary purpose is to support development of l-k ;-).

I do not mind if you told me to change them to a/ or b/ (or do it yourself---it is a single character change in the source) if you want.

On Thu, 28 Apr 2005, Junio C Hamano wrote:

Absolutely. In fact, I prefer it, but I end up just using standard "patch -p1" in the end, so..

diffstat is _way_ too easily confused by various things. I've seen it claim "no files" just because the diff had some headers that confused it. And yes, you should always tell it to use "-p1" to get the right pathnames, otherwise it does nonsensical things (if all the diffs happen to be in "drivers/usb/" it ends up deciding that that's just a common prefix, and won't actually show it at all).

However, I'm surprised that it's confused by /dev/null. Usually the confusion comes from the stuff _after_ the name (ie adding the "mode" etc is what I'd have expected to confuse it).

One way to un-confuse diffstat is to add the "Index: " line. I'm not actually much of a fan of Index: lines myself, and I'd rather somebody fixed diffstat, but they _do_ work around diffstat problems.

		Linus

Minor nit on Cogito: I think cg-update should default to "origin", not the head, if you leave it unspecified. Instead, add an option flag to specify the HEAD. The origin seems (to me) to be a MUCH more common situation (and thus the better default).

--- David A. Wheeler

Dear diary, on Thu, Apr 28, 2005 at 02:32:32AM CEST, I got a letter where "David A. Wheeler" <dwheeler@dwheeler.com> told me that...

Actually, I wasn't too happy with the current update-to-HEAD special case. Sure, it's similar to SVN, but SVN's concepts are totally different here, and this special case wart (which does really do something entirely different than normal cg-update) is one of the Cogito-related shadows in my mind. What about moving this special case to something like

	cg-restore

and changing the defaulting of update and pull back to 'origin'? I think people do this cg-update without arguments so seldom that changing this now shouldn't hurt much, right?

Another thing is to UI-wise maintain clear difference between cg-cancel and cg-restore. Do you think the names are distinctive enough? Any better naming ideas?

Thanks,

Petr Baudis wrote:

How about making the restore thing a special case of cg-cancel instead? "Restore deleted files", and "restore deleted and modified files and unseek" are similar enough that people will now where to look. Something like "cg-cancel -C" (for careful), that only restores deleted files would do it, I think.

/dan

I said:

Petr Baudis replied:

Sounds like we're in agreement! Once the special case goes
away, cg-update in both concept & code essentially becomes:
  cg-pull ${1:-origin} && cg-merge
which has the wonderful advantage of being really, really
easy to explain.  ("cg-update ALWAYS pulls, then merges").

 > I think people do this cg-update without arguments so seldom
 > that changing this now shouldn't hurt much, right?

Absolutely!  Indeed, I find myself doing:
  cg-update {wait for something to happen} {oops} cg-update origin

...

Good names for these operations seem to be tough to find. "cg-cancel" seems odd anyway; you'd think you could "cancel" a commit and then the commit would stop existing (not true!).

I looked at a thesaurus; other options to cancel & restore include: revert, recover, retrieve, reclaim, reclaim, undo. You could even use the names cg-recover-deleted to recover deleted files (what cg-update does now without parameters), and use cg-cancel-edits or cg-cancel-changes to make clearer commands. But in the end I have a different idea, hold on...

elsewhere Dan Holmsand said:
 >How about making the restore thing a special case of cg-cancel instead?
 >"Restore deleted files", and "restore deleted and modified files and
 >unseek" are similar enough that people will now where to look.
 >Something like "cg-cancel -C" (for careful), that only restores deleted
 >files would do it, I think.

There's a big risk of not including the "-C" and suddenly losing everthing. Since there's NO way to recover these files, a somewhat safer interface would probably be a better idea. But merging the concepts may make sense if we can find a single command name that would help people figure this out.

How about "cg-revert" or "cg-restore"? The word "revert" is even in the comments for cg-cancel, but now it makes sense to "revert" or "restore" the existence of a file (whereas it's really odd to "cancel" a file deletion).

A serious problem with cg-cancel (and previous cg-undo) is big data loss, no recovery, of your recent work.... if it's going to have less & more drastic operations, I'd sure hate for the drastic operation to be the default. There's also missing functionality currently: often I want to revert to the unedited state for just a single file, or just restore a single file. So, how about this:

cg-revert [FILE...] or
cg-revert [-d|--deleted]|[-a|--all]
   Reverts some/all files back to the HEAD's state, eliminating changes

   If given a list of 1 or more files, this reverts just the named files
   to the HEAD state. If they were deleted, they are restored;
   if they were edited, their edits are PERMANENTLY LOST.
   If they haven't changed, nothing changes and there is no error.

   If given -d or --deleted, it reverts all deleted files.
   If given -a or --all, it reverts all files
   (everything), resuling in loss of all edits and removals.

How's that for a reasonable UI, replacing both cg-cancel and cg-update's current no-parameter functionality?

--- David A. Wheeler

David A. Wheeler wrote:

That's very good (and much better than my idea).

/dan