git rebase chokes on directory -> symlink -> directory

git rebase chokes on directory -> symlink -> directory

[H. Peter Anvin]

The following tree:

http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-newsetup.git;a=summary

... has one commit which changes arch/x86_64/boot from a directory to a
symlink, and another one which changes it back.  Apparently as a result,
git rebase dies horribly; on the first change it requires manual fixup,
but it crashes on the second, with or without -m.

The specific task attempted is to rebase the main branch of that tree
onto the current Linus tree, a989705c4cf6e6c1a339c95f9daf658b4ba88ca8.

	-hpa

Re: git rebase chokes on directory -> symlink -> directory

[Alex Riesen]

H. Peter Anvin, Tue, May 08, 2007 03:08:12 +0200:
> The following tree:
> 
> http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-newsetup.git;a=summary
> 
> ... has one commit which changes arch/x86_64/boot from a directory to a
> symlink, and another one which changes it back.  Apparently as a result,
> git rebase dies horribly; on the first change it requires manual fixup,
> but it crashes on the second, with or without -m.

What kind of manual fixup did you do? I tried to reproduce it, and did
the following:

    git clone --reference ~/linux.git git://git.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-newsetup.git
    cd linux-2.6-newsetup.git
    git rebase a989705c4cf6e6c1a339c95f9daf658b4ba88ca8

It stopped at 'Revert "x86-64: Make arch/x86-64/boot a symlink to
arch/i386/boot"' aka cd312503f8e8a88895b12bf810677406284142e6.
I went on:

    rm arch/x86-64/boot
    git checkout cd312503f8e8a88895b12bf810677406284142e6 arch/x86-64/boot
    git rebase --continue

And then it just continued until all commits were rebased.
I have a very recent git, so maybe that's why it worked.

Re: git rebase chokes on directory -> symlink -> directory

[H. Peter Anvin]

Alex Riesen wrote:

> 
> What kind of manual fixup did you do? I tried to reproduce it, and did
> the following:
> 
>     git clone --reference ~/linux.git git://git.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-newsetup.git
>     cd linux-2.6-newsetup.git
>     git rebase a989705c4cf6e6c1a339c95f9daf658b4ba88ca8
> 
> It stopped at 'Revert "x86-64: Make arch/x86-64/boot a symlink to
> arch/i386/boot"' aka cd312503f8e8a88895b12bf810677406284142e6.
> I went on:
> 
>     rm arch/x86-64/boot
>     git checkout cd312503f8e8a88895b12bf810677406284142e6 arch/x86-64/boot
>     git rebase --continue
> 
> And then it just continued until all commits were rebased.
> I have a very recent git, so maybe that's why it worked.
> 

Mine stops already at the directory -> symlink checkin (the above is the
symlink -> directory one), but your trick of using "git checkout" as a
trick to resolve things helped for both... eventually :-/

Either way, it's still a bug that it stops for either checkin, but it's
not blocking my work anymore.

	-hpa

Re: git rebase chokes on directory -> symlink -> directory

[Alex Riesen]

On 5/9/07, H. Peter Anvin <hpa@zytor.com> wrote:
>
> Mine stops already at the directory -> symlink checkin (the above is the
> symlink -> directory one), but your trick of using "git checkout" as a
> trick to resolve things helped for both... eventually :-/
>

Hmm. What Git version do you have?

> Either way, it's still a bug that it stops for either checkin, ...

Right. And because it is a bug, I'd like to have it fixed.
So, what did you do in that fixup?

Re: git rebase chokes on directory -> symlink -> directory

[H. Peter Anvin]

Alex Riesen wrote:
> On 5/9/07, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> Mine stops already at the directory -> symlink checkin (the above is the
>> symlink -> directory one), but your trick of using "git checkout" as a
>> trick to resolve things helped for both... eventually :-/
>>
> 
> Hmm. What Git version do you have?

Not sure anymore, because I ran a systemwide upgrade late last night.
*Now* I have git-1.5.1.4, but I think I had 1.5.1.2 before.

>> Either way, it's still a bug that it stops for either checkin, ...
> 
> Right. And because it is a bug, I'd like to have it fixed.
> So, what did you do in that fixup?

I'm sorry, I'm not sure I understand the question, in particular, I'm
getting the feeling I'm not sure what "that fixup" refers to.

	-hpa

Re: git rebase chokes on directory -> symlink -> directory

[Alex Riesen]

H. Peter Anvin, Wed, May 09, 2007 18:58:23 +0200:
> >>
> >> Mine stops already at the directory -> symlink checkin (the above is the
> >> symlink -> directory one), but your trick of using "git checkout" as a
> >> trick to resolve things helped for both... eventually :-/
> > 
> > Hmm. What Git version do you have?
> 
> Not sure anymore, because I ran a systemwide upgrade late last night.
> *Now* I have git-1.5.1.4, but I think I had 1.5.1.2 before.

... and the rebase you tried originally works now? Or you didn't try?
There were changes (substantial changes) in this area, so it might be
fixed. If not, I'd like to hear, I personally rely on rebase heavily.

> >> Either way, it's still a bug that it stops for either checkin, ...
> > 
> > Right. And because it is a bug, I'd like to have it fixed.
> > So, what did you do in that fixup?
> 
> I'm sorry, I'm not sure I understand the question, in particular, I'm
> getting the feeling I'm not sure what "that fixup" refers to.
> 

>From your original report:

"git rebase dies horribly; on the first change it requires manual fixup,
but it crashes on the second, with or without -m."

You mentioned that on the first change (I assumed it is the first time
git-rebase stopped, complained, and asked for your help) "it" requires
a "manual fixup". Which I assume you did, as it crashed on "second"
(stop?). The "manual fixup" from the original report, what was it?
Can you remember or find the sequence of commands you did before "it
crashed on second"?

Re: git rebase chokes on directory -> symlink -> directory

[H. Peter Anvin]

Alex Riesen wrote:
>>>> Either way, it's still a bug that it stops for either checkin, ...
>>> Right. And because it is a bug, I'd like to have it fixed.
>>> So, what did you do in that fixup?
>> I'm sorry, I'm not sure I understand the question, in particular, I'm
>> getting the feeling I'm not sure what "that fixup" refers to.
>>
> 
> From your original report:
> 
> "git rebase dies horribly; on the first change it requires manual fixup,
> but it crashes on the second, with or without -m."
> 
> You mentioned that on the first change (I assumed it is the first time
> git-rebase stopped, complained, and asked for your help) "it" requires
> a "manual fixup". Which I assume you did, as it crashed on "second"
> (stop?). The "manual fixup" from the original report, what was it?
> Can you remember or find the sequence of commands you did before "it
> crashed on second"?

Ah, I used your technique of removing everything manually when the
rebase failed, and then doing a "git checkout" of arch/x86_64/boot.
That seemed to work.  I actually logged the commands, but threw out the
log after it worked, figuring it didn't tell you anything new :(

Trying to do a rebase again with 1.5.1.4 and see how it works gives only
one stop instead of two, so it's "halfway there":

: tazenda 14 ; git rebase stock
First, rewinding head to replay your work on top of it...
HEAD is now at a989705... Merge
git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux-2.6

Applying 'x86 setup code rewrite: initial development snapshot'

Wrote tree bf3a4e990978bad7669dac117b0d482e53f1bcd7
Committed: f98b2dc7eff7f2ce9d039af794e7f0062cc42bf9

Applying 'MAINTAINERS: formally take responsibility for the i386 boot code'

Wrote tree 237806033e5c789752aa555a5aaba17c8b650c4c
Committed: e4fb08878f0fe2c2e72665315c35e698ece08101

Applying 'x86 setup: printf.c needs code16gcc.h'

Wrote tree 8be170c7240961ff47d13045412b2f8d66d54e20
Committed: 2a103e2db001d0d6a852861ea209b34ba97ffc6a

Applying 'x86 setup: in tty.c, actually tell it what character to print'

Wrote tree 0e487f781b874a8025e7fcd74e055abeee5ebe2e
Committed: da62daaf237756aab1330f8dc4842ca5e5fa4ac9

Applying 'build: setup sectors doesn't include the boot sector'

Wrote tree 66192f639721713010b65ddb6c40f0d7a1f51a0b
Committed: 5794cf95386551f0f97b7cae877845cbbb3ebfea

Applying 'x86 setup: segment descriptors need to be Present'

Wrote tree a3aa7db9741d1dc57007d97152a337b6532fc588
Committed: de616f57959c3b62a1ac544858a8bd41696d59dd

Applying 'x86 setup: make the video setup code actually do something...'

Wrote tree 9358db854ca73d9f6ac344c9dbc397a9913b4c86
Committed: 13a363f40ff51d61d5c553e529555c32425d7553

Applying 'x86 setup: fix missing semicolon in video-ati.c'

Wrote tree 0bbd82ef2e2bc2128c29ee8f8ca04399b86f8a98
Committed: d948a8ebb7f26f030264153fd801aca5dca47603

Applying 'x86 setup: fix memcmp_[fg]s()'

Wrote tree 368f27ee12eeeebfaad11bfabf6e087638e94b10
Committed: d5a63882787f4a927910307f2e8f3df0534643f6

Applying 'x86 setup: advance one e820 descriptor at a time...'

Wrote tree e7fb596232ed18cbddb6507ce8cb67e09a6326c9
Committed: ccfd63579f5df793701d44c46dd575629eabad85

Applying 'x86 setup: Call INT 15h AX=E820h properly'

Wrote tree bb010d0e1224ccbf4a0e201dab191448e84405d5
Committed: a84a8eab7e7d3708ad9cfdee67981eafe3a21b38

Applying 'x86 setup: remove assembly implementation of putchar and puts'

Wrote tree dc8d970b62d1be70175e04bed26dfa644f521cfd
Committed: 7356f51c11fc0e5f363a5f06df5274a7ccea73f5

Applying 'x86 setup: Sadly, Cirrus removed extended text modes from
their BIOS.'

Wrote tree 48560789f0ed319f77b529ea3f31e11126df6221
Committed: 06a5abda109eeadb34f3087b51e8a1f4cc69c3f3

Applying 'x86 setup: if no specific video mode ID is given, generate one'

Wrote tree 0366a71399c33ca08590aaa67df6438122529867
Committed: 56eb2476b71a917747bf6ea111568f9e5ba85e2c

Applying 'x86 setup: drop video mode range checking'

Wrote tree bd90ee84715e4eff9f1fafe7efc3c6440e6be69e
Committed: 668bf617eb273ddcefb007e3c5ec9bb4aa022894

Applying 'x86 setup: video.c: clean up unused stuff'

Wrote tree 1a2d47f784b5b64e7893a3364cbef1c8ba2a39cd
Committed: f38134d1296f5a09887b79cc6c4b13e34f77f293

Applying 'x86 setup: a20.c: make empty_8042() return status'

Adds trailing whitespace.
.dotest/patch:26:
warning: 1 line adds trailing whitespaces.
Wrote tree 0f46a1846afcaf61d7c4416d7f308ffd9d87d69c
Committed: 18a9e385504b85daa0e293e2a60309e34de4548e

Applying 'x86 setup: Modern ATI cards pass the probe but lacks the modes.'

Wrote tree 775608a82bd3783f5ba5dc00f7b4b7c39eb463ed
Committed: 0ed82f2809ec04d4db243d833e8cb5fcc029cd3e

Applying 'x86 setup: video.c: correct the handling of special mode numbers'

Wrote tree 9dae6d4dcc4bc2b98114a5a3ae6ff88bea7c60c3
Committed: af2b591e9c529b35b4cbb006b417cd6f16b87504

Applying 'x86 setup: remove references to obsolete probes'

Wrote tree f5605b2ab57ef0e52129c1809fa03c4b804b05cd
Committed: 30448725fe01982ab4ce7d28af609783c9d7eb32

Applying 'x86 setup: implement APM BIOS probe'

Wrote tree e46f5c4e318fa5f30ea221b90bdaa301876945cf
Committed: 293ae9fb21ebbfa6c834a016a20037a19b97bc5e

Applying 'x86 setup: clobber registers in keyboard BIOS call'

Wrote tree 98610c0970730f62d05412eaaadcec1528d94a98
Committed: 4b89273f6c78fe28b6f4aea161f64aa7ac2b8724

Applying 'x86 setup: tag functions noreturn; error message on A20 failure'

Adds trailing whitespace.
.dotest/patch:78:
warning: 1 line adds trailing whitespaces.
Wrote tree 605c96e3555b7bbbfd7b06306b225becff4ab09e
Committed: 72bd91360bbe365dc152e39f03229152c1e4f978

Applying 'x86 setup: whitespace cleanup'

Wrote tree 27230a23ba2f8b0aac3dddc264d7f4a28d18f8dc
Committed: 35f72771c466d83cc551bf81dfed22326048080e

Applying 'x86 setup: add CPU feature detect/abort on insufficient featurage'

Adds trailing whitespace.
.dotest/patch:20:setup-y                += header.o main.o mca.o
memory.o pm.o pmjump.o
warning: 1 line adds trailing whitespaces.
Wrote tree 405fde3817d89e7aba618bda965d4bcc24e825d5
Committed: 18e25bde0d246d828931ebd6450c63a4091e0fc1

Applying 'x86 setup: whitespace cleanup'

Wrote tree 8124b48b7b985ae31062356df521462e4420f09a
Committed: 91d417b3def3b38aeba165282746fa82e6de0fad

Applying 'x86 setup: files missing from previous checkin (cpu.c, cpureq.c)'

Wrote tree 2be68b923f0b413701f5f57b93aaca42cc126e49
Committed: a312ada92fdff09e0ca6cf60724353f8f5d3578a

Applying 'x86 setup: remove unused verify_cpu.S'

Wrote tree 7f3ebf06770406f4936ace5aef8a1b20e2676563
Committed: 0eaf9d747d1e0677aef4c762333b8b1988fe5ce3

Applying 'x86 setup: compile with -DSETUP'

Wrote tree 475e43922e02adad5ff1e9e1c77df1ff26cbdcc6
Committed: b843ca098801e962fed08f20ee99fb2f5cbd18fd

Applying 'x86 setup: cpu detection cleanups'

Wrote tree d72cabf9f1812da55506c38ddd88ef5dcd42f535
Committed: 45adb1922193dc5204d5bf07de9e5a8939b2688c

Applying 'x86 setup: remove bogus "static"'

Wrote tree 17281ee44c7ce686366464985b8d1a436d6d7468
Committed: 79ed2a26f0de632d0fb9cf36ccd30d99267975a2

Applying 'x86 setup: use CONFIG_X86_MINIMUM_CPU_MODEL'

Wrote tree 1e3a29b3de3dd86c46d2900485ef8206f948a72d
Committed: ed41358ab168c7e7e9eb1ddff44d995eb339879e

Applying 'x86: Kconfig.cpu: the minimum CPU model is always 3;
WP_WORKS_OK = i486'

Wrote tree d174addf546223baf87598db7673872608f6cb2c
Committed: 84c13f3993b812efbc781ae106fecc0e7478809e

Applying 'x86: make the handling of required features consistent'

Wrote tree c96d44bd1b12f0b4090db2bc60ffec5c54c1c4da
Committed: 48091cdd868350c0c98b0b11592606e850058737

Applying 'x86 setup: use the required masks from <asm/required-features.h>'

Adds trailing whitespace.
.dotest/patch:145:
Adds trailing whitespace.
.dotest/patch:156:
Adds trailing whitespace.
.dotest/patch:190:
Adds trailing whitespace.
.dotest/patch:200:
Adds trailing whitespace.
.dotest/patch:248:
warning: squelched 1 whitespace error
warning: 6 lines add trailing whitespaces.
Wrote tree 7f53ccc902a878e6bdae0446147b79f693bbbfe3
Committed: 22f4301757277e2d542fab95529c664413225efe

Applying 'x86 setup: remove reference to obsolete cpureq.c'

Wrote tree c77da037a2247c8f577e5167d6bfed833484c3a5
Committed: ed056f881d6c0698c233ee888d9c4e798c8ba736

Applying 'x86 setup: apparently $(src) is insufficient, needs
$(srctree)/$(src)'

Wrote tree 53ed308ffc67dd7bbca326facb578bc56091855c
Committed: 530888f2f6f9dceb7a3d1d5e2e5a5c4d56606136

Applying 'x86 setup: bootlin is *so* dead...'

Wrote tree 65574110a6702b6d2616441ce74e5c02bd53cb6c
Committed: f20bb507355d38b871f4bb2034b9303f4b0742e0

Applying 'x86 setup: paranoia: clear the high half of %esp'

Adds trailing whitespace.
.dotest/patch:15:
warning: 1 line adds trailing whitespaces.
Wrote tree 52d253ca3ff8dd7f8394762720501c99614a1b83
Committed: f9040dd40db9f08650201f2f32f56186b31a7dcb

Applying 'x86 setup: add missing linker script'

Wrote tree f9dd325985592248b878c4df0646fbc430e9cdd6
Committed: 877594775c44a72c802f4e8cba9db5db63223aa4

Applying 'x86 setup: cleanups for compatibility with x86-64'

Adds trailing whitespace.
.dotest/patch:125:
warning: 1 line adds trailing whitespaces.
Wrote tree d83528199922e40076dc373cd9d24daba9a15516
Committed: 907963662ce663a8deb3594c6c7f9783de1cbe3b

Applying 'x86-64: add CONFIG_PHYSICAL_ALIGN to match i386'

Wrote tree f6d6abdee2fceed761eef217dcc1c7ed0de9877e
Committed: a015ea48164d9e6a8ab317c26cbe555c3032f699

Applying 'x86-64: <asm/segment.h>: add boot segment descriptors'

Wrote tree e3abd746b0f94b58ed510264a62b123e995f08b6
Committed: 8e00cfafbf108a2801fb0ec8b824962b1e073d55

Applying 'x86-64: fix compilation errors due to required-features.h change'

Wrote tree a1626723ccdd339ac8ef0764c77b2cdccdfb2f26
Committed: 355b0dc242a05f72c7bc1ad8f73fdc6e82a39afc

Applying 'x86-64: verify_cpu.S: use new masks'

Wrote tree 117fbd16776cc9b201a0f76c9f3910cc7c1f8fa6
Committed: 14b6bc6f64844958c7f923ee46ce4e2356e21c08

Applying 'x86: unify <asm/boot.h>'

Wrote tree ce269d002a4645b5ef54b1f8eef086a43498c81e
Committed: d84e3cc9eb73c7feb084b35303e45828917e3115

Applying 'x86: Complete <asm/cpufeature.h> with the union of i386 and
x86-64'

Wrote tree 3f40077f38ceb45312a780390f8d8d5eb98ffe17
Committed: 03d199b61342108a9f37978bec06626710284cd1

Applying 'x86-64: rearrange includes due to unifications and inclusion
from setup'

Space in indent is followed by a tab.
.dotest/patch:44:               "661:\n\tlock; "
warning: 1 line adds trailing whitespaces.
Wrote tree 8f86e02ef672c98ae3519b9b3fabb3701a0ab1b9
Committed: 147328a824c71437259baf9a63fe03762bba74e0

Applying 'x86-64: Make arch/x86-64/boot a symlink to arch/i386/boot'

Wrote tree 9bc7ffd5d07a49c5a83fc8f9a680473995aa6c57
Committed: a859f67570bf62d2a602d27aa34a674e6645ca24

Applying 'x86: fix the definition of struct screen_info'

Wrote tree 258e5053597a572954943cb5b895e4462e6491fb
Committed: 951cf2086f5c2055e93b0db22c1064829bedc74e

Applying 'x86: fix differences between i386 and x86-64 <asm/e820.h>'

Wrote tree 6e4f4ed1de0a6736fb298d4e1adcae3b60f7a151
Committed: 3bec0726b8cfa624492b35659c5a13d484a1bc89

Applying 'i386: change %lu to %u in arch/i386/kernel/e820.h'

Wrote tree b05d8462dd9ce61fb32f61d2c9819877afe6be94
Committed: 7ab473d60f7bda2e7f7341bd9ca88610c5c440c5

Applying 'x86: move the bootparam structure definition into include/'

Wrote tree 3865d940f5be27cf70fe54aff3825b1f6c676a01
Committed: 3b5b95528bcf5487f79e659fe6346377955dd654

Applying 'x86 setup: E820MAX is a definitional constant; no need to use
sizeof hacks'

Wrote tree 6ba2914fb480e88633a7da7700dce60b1e619c22
Committed: 3db11fa860f9b06bf1b50f3fe698f0c6fb9e55ca

Applying 'x86 setup: boot_params.e820_map is just the map, not the
count; adjust'

Wrote tree 75308a18a561335d5a6a3613b51e130f43fcb00c
Committed: 4d6dea0f6c8ec5ac8580bcaee1c8d52f5d35e55a

Applying 'x86 setup: use 0x1e4 as scratch, instead of 0x3c'

Wrote tree 55fe315b3a931bdb3f1ea96ebc5098bcc24b0610
Committed: eca97ac06fa0f8d17b1ec974c890642afea62555

Applying 'x86-64: It appears MTRR isn't a required feature after all.'

Wrote tree 56fc138a5b01818142e200fa35921570270b53ac
Committed: 72c0fc6ecc2661245ae1fc42210a808a0ef63da1

Applying 'Revert "x86-64: Make arch/x86-64/boot a symlink to
arch/i386/boot"'

Adds trailing whitespace.
.dotest/patch:117:FDARGS =
Adds trailing whitespace.
.dotest/patch:350: * Page 0 is deliberately kept safe, since System
Management Mode code in
Adds trailing whitespace.
.dotest/patch:352: * useful for future device drivers that either access
the BIOS via VM86
Adds trailing whitespace.
.dotest/patch:648: *
Adds trailing whitespace.
.dotest/patch:649: * This is a collection of several routines from
gzip-1.0.3
error: arch/x86_64/boot/.gitignore: already exists in working directory
error: arch/x86_64/boot/Makefile: already exists in working directory
error: arch/x86_64/boot/compressed/Makefile: already exists in working
directory
error: arch/x86_64/boot/compressed/head.S: already exists in working
directory
error: arch/x86_64/boot/compressed/misc.c: already exists in working
directory
error: arch/x86_64/boot/compressed/vmlinux.lds: already exists in
working directory
error: arch/x86_64/boot/compressed/vmlinux.scr: already exists in
working directory
error: arch/x86_64/boot/install.sh: already exists in working directory
error: arch/x86_64/boot/mtools.conf.in: already exists in working directory
error: arch/x86_64/boot/tools/.gitignore: already exists in working
directory
error: arch/x86_64/boot/tools/build.c: already exists in working directory
Using index info to reconstruct a base tree...
Adds trailing whitespace.
<stdin>:117:FDARGS =
Adds trailing whitespace.
<stdin>:350: * Page 0 is deliberately kept safe, since System Management
Mode code in
Adds trailing whitespace.
<stdin>:352: * useful for future device drivers that either access the
BIOS via VM86
Adds trailing whitespace.
<stdin>:648: *
Adds trailing whitespace.
<stdin>:649: * This is a collection of several routines from gzip-1.0.3
warning: squelched 26 whitespace errors
warning: 31 lines add trailing whitespaces.
Falling back to patching base and 3-way merge...
fatal: Untracked working tree file
'arch/x86_64/boot/compressed/Makefile' would be overwritten by merge.
Failed to merge in the changes.
Patch failed at 0058.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".
: tazenda 15 ; ls -ld arch/x86_64/boot
lrwxrwxrwx 1 hpa hpa 12 May  9 15:41 arch/x86_64/boot -> ../i386/boot/
: tazenda 16 ; rm -f arch/x86_64/boot
: tazenda 17 ; head -1 .dotest/0058
From cd312503f8e8a88895b12bf810677406284142e6 Mon Sep 17 00:00:00 2001
: tazenda 18 ; git checkout cd312503f8e8a88895b12bf810677406284142e6
arch/x86_64/boot
: tazenda 19 ; ls -ld arch/x86_64/boot
drwxrwxr-x 4 hpa hpa 4096 May  9 15:42 arch/x86_64/boot/
: tazenda 20 ; git rebase --continue

Applying 'Revert "x86-64: Make arch/x86-64/boot a symlink to
arch/i386/boot"'

Wrote tree 454be72b74038289064303704fa095e220539518
Committed: 7f942303bf26e31d10bf96b8d3fb3f1ee50a73a9

Applying 'x86-64: use 0x1b4 as the scratch area in boot_params, not 0x3c'

Wrote tree a9634c1b3aa7bdc5d7de6d8fcdafaef851181f2b
Committed: 3f23a6bd448bc9b00718b412170ee2ed752eec8f

Applying 'x86 setup: share code between i386 and x86-64'

Wrote tree 8397248a3723de8f0b31c6b5ed7f60d968c1631b
Committed: 72fe7620458f84e676dd40fdbe66835135e8a77c

Applying 'x86-64: remove -traditional from AFLAGS'

Wrote tree 5a27b91ebc542b2769347837734890e2043d1b4a
Committed: 2f8b9521f04effda7b8150c4302aff469245c5ef

Applying 'x86 setup: move all VESA-related code into video-vesa.c; add EDID'

Adds trailing whitespace.
.dotest/patch:86:
warning: 1 line adds trailing whitespaces.
Wrote tree e3289a29203434e4d683254f2aee077ddd4ae63c
Committed: f579842ed1a0dbabadc5359a7490c6a990bc7cb6

Applying 'x86 setup: allow setting of VESA graphics modes; cleanups'

Adds trailing whitespace.
.dotest/patch:68:
warning: 1 line adds trailing whitespaces.
Wrote tree 44eebedb0cb6cb40a10aec011103d0271a5e3fd3
Committed: e9cbeb161b71ac237d9bf8c8afd15f9a35eedb20

Applying 'x86 setup: whitespace cleanup'

Wrote tree eec8d62c5c9db76d918815f113d802a7a3d0d7f5
Committed: e3c55080c3db6dd3a9836fe7cd99bbdbb98bf84d

Applying 'x86 setup: implement screen contents save/restore'

Wrote tree f135c21ddd2b6af70f97c1380dbcd460267ec177
Committed: 53be00bc3919c2c565776bb408ecdc0ea5dbc261

Applying 'x86 setup: coppyright rPath, Inc.'

Wrote tree 47f7505f0436b527839190ee26029d520f17d25e
Committed: 2bf2c30e18fef98061ec0272e957d8faa387b6a0

Applying 'i386 boot protocol: boot loaders should allow more heap for
bzImage'

Wrote tree 4c5790a4a6ccf697e94729c3f5dec8d4d34a3dbc
Committed: 7439514e936c899639c6731f29a4988e7b66aa36

Applying 'x86 setup: actually check the end of the heap.'

Wrote tree 3ed25ec4913189031ccfd5368969cfd77a1fbece
Committed: ab7cd52538086dc095289a981c9177e956abf354

Applying 'x86 setup: when watching the setup size, take the stack into
account'

Wrote tree 34fa356d84aaa5241a281a8b01f04a059b5b353b
Committed: f3691d413e883e956213fc45cc132f7f76d7d3b7

Applying 'x86 setup: Factor out the environment-independent part of the
CPU check.'

Adds trailing whitespace.
.dotest/patch:239:
Adds trailing whitespace.
.dotest/patch:263:
warning: 2 lines add trailing whitespaces.
Wrote tree bae2277d4c4d40ba7b8fc11a25342ff4c023ef49
Committed: 85a238ecc2371cde07117d34524c71f9c96bc2f7

Applying 'x86 setup: be more paranoid about the stack setup in header.S'

Wrote tree 1b5fa9f3d37d361e6b47bb4e7142ecbe89eb552c
Committed: 58643e76ef2630ae5e7b0541c5fd0e8183141a10

Applying 'x86 setup: compile with -fomit-frame-pointer'

Wrote tree 3cd438bea06b27992940ce53fc56f16737d77c74
Committed: b36cd32f74a8be03dc23cd20b65c5c08dfae50ae

Applying 'x86 setup: remove double nesting of a20_test()'

Wrote tree 941b5ec12d8a1e82949ad0cd3932537f91eb5e8d
Committed: 53cdb8049013d6f72d3cfb5be0bd9999fbff2a97

Applying 'x86 setup: remove code moved from cpucheck.c -> cpu.c'

Wrote tree d83b020d216f92186026208576b5170b61ffd382
Committed: d5afa72f8eb28cc1e245b321dfa19fb620970900

Applying 'x86 setup: swap cpu.c and cpucheck.c; rename functions'

Adds trailing whitespace.
.dotest/patch:489:
Adds trailing whitespace.
.dotest/patch:512:
warning: 2 lines add trailing whitespaces.
Wrote tree 41d79ce765e1174464c47bff61c4330e23bf6382
Committed: f2c63bb8e16d9b3c67ad1fdbc7ae1f5e18286662

Applying 'x86 setup: add -fno-stack-protector; other Makefile fixes'

Wrote tree 65c5d385ff3bc54b65881629f72e4583c69a737e
Committed: 7ea7f8dbb4103d43b90fb6cc6f32c923b1f658e8

Applying 'x86 setup: add missing file "bitops.h" missing from previous
checkins'

Wrote tree 01beebe04609804ef88762e794fdd71daff89bf1
Committed: 7eaa98edb81e9937128ce342f626177ee8a68941
: tazenda 21 ;

Re: git rebase chokes on directory -> symlink -> directory

[Junio C Hamano]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Applying 'Revert "x86-64: Make arch/x86-64/boot a symlink to
> arch/i386/boot"'
>
> Adds trailing whitespace.
> .dotest/patch:117:FDARGS =
> Adds trailing whitespace.
> .dotest/patch:350: * Page 0 is deliberately kept safe, since System
> Management Mode code in
> Adds trailing whitespace.
> .dotest/patch:352: * useful for future device drivers that either access
> the BIOS via VM86
> Adds trailing whitespace.
> .dotest/patch:648: *
> Adds trailing whitespace.
> .dotest/patch:649: * This is a collection of several routines from
> gzip-1.0.3
> error: arch/x86_64/boot/.gitignore: already exists in working directory
> error: arch/x86_64/boot/Makefile: already exists in working directory
> error: arch/x86_64/boot/compressed/Makefile: already exists in working
> directory
> error: arch/x86_64/boot/compressed/head.S: already exists in working
> directory
> error: arch/x86_64/boot/compressed/misc.c: already exists in working
> directory
> error: arch/x86_64/boot/compressed/vmlinux.lds: already exists in
> working directory
> error: arch/x86_64/boot/compressed/vmlinux.scr: already exists in
> working directory
> error: arch/x86_64/boot/install.sh: already exists in working directory
> error: arch/x86_64/boot/mtools.conf.in: already exists in working directory
> error: arch/x86_64/boot/tools/.gitignore: already exists in working
> directory
> error: arch/x86_64/boot/tools/build.c: already exists in working directory

Ahh.

 * The tree state before this patch is applied has arch/x86_64/boot
   as a symlink pointing at ../i386/boot/

 * The patch tries to remove arch/x86_64/boot symlink, and
   create bunch of files there: .gitignore, Makefile, etc.

This is unfortunately a bit deeper than just git-rebase.  You
exposed a nasty corner case problem in the stock git-apply,
probably one of the most important tool the kernel project uses
every day.

git-apply tries to be careful while applying patches; it never
touches the working tree until it is convinced that the patch
would apply cleanly.  One of the check it does is that when it
knows a path is going to be created by the patch (in your
example, arch/x86_64/boot/.gitignore), it runs lstat() on the
path to make sure it does not exist.

This leads to a false alarm.  Because we do not touch the
working tree before all the check passes, when we try to make
sure that arch/x86_64/boot/.gitignore does not exist yet, we
haven't removed the arch/x86_64/boot symlink.  The lstat() check
ends up seeing arch/i386/boot/.gitignore through the
yet-to-be-removed symlink, and says "Hey, you already have a
file there, but what you fed me is a patch to create a new
file. I am not going to clobber what you have in the working
tree."

We have similar checks to see a file we are going to modify does
exist and match the preimage of the diff, which is done by
directly opening and reading the file.

For a file we are going to delete, we make sure that it does
exist and matches what is going to be removed (a removal patch
records the full preimage, so we check what you have in your
working tree matches it in full -- otherwise we would risk
losing your local changes), which again is done by directly
opening and reading the file.

These checks need to be adjusted so that they are not fooled by
symlinks in the middle.

 - To make sure something does not exist, first lstat().  If it
   does not exist, it does not, so be happy.  If it _does_, we
   might be getting fooled by a symlink in the middle, so break
   leading paths and see if there are symlinks involved.  When
   we are checking for a path a/b/c/d, if any of a, a/b, a/b/c
   is a symlink, then a/b/c/d does _NOT_ exist, for the purpose
   of our test.

   This would fix this particular case you saw, and would not
   add extra overhead in the usual case.

 - To make sure something already exists, first lstat().  If it
   does not exist, barf (up to this, we already do).  Even if it
   does seem to exist, we might be getting fooled by a symlink
   in the middle, so make sure leading paths are not symlinks.

   This would make the normal codepath much more expensive for
   deep trees, which is a bit worrisome.

When the code writes out the result after the checks pass, we
first delete all the paths that are going to be deleted, and
also delete all the paths that are being modified.  Then in the
second pass, we create all the paths that are being created and
also modified by writing their final contents out, while
creating leading directories as needed.  Because of this, a
patch that removes a symlink and then makes it a directory to
hold new files would first remove the symlink and then create
the directory at the right location and deposit newly created
files there.  So I do not expect any change is needed in the
writeout codepath.

By the way, I noticed a few things while diagnosing this.

 * It was very considerate of you to leave "rebase-1" branch for
   my postmortem in the repository.  You know what are needed
   for debugging very well.

 * git-rebase with -m is dog slow.  There were people who
   advocated to make it the default, but they probably are
   either working in a very small project, or working on a
   filesystem that even git-apply is slow that the speed
   difference does not matter to them.

Re: git rebase chokes on directory -> symlink -> directory

[Johannes Sixt]

Junio C Hamano wrote:
>  * git-rebase with -m is dog slow.  There were people who
>    advocated to make it the default, but they probably are
>    either working in a very small project, or working on a
>    filesystem that even git-apply is slow that the speed
>    difference does not matter to them.

Heh... you name it. But just yesterday rebase -m was a life saver for me
because of its rename detection.

-- Hannes

Re: git rebase chokes on directory -> symlink -> directory

[Shawn O. Pearce]

Junio C Hamano <junkio@cox.net> wrote:
>  * git-rebase with -m is dog slow.  There were people who
>    advocated to make it the default, but they probably are
>    either working in a very small project, or working on a
>    filesystem that even git-apply is slow that the speed
>    difference does not matter to them.

That would have been me saying make -m default.  Because on git.git
I don't really see a huge performance difference on my Mac, and the
bigger projects that I work on are on Cygwin, where *everything*
is slow as hell.

The few places there that I do need rebase, I actually also need
the rename detection that am -3 or rebase -m would do, so I just
wind up paying the penalty anyway.  Or really I just get burned
because I run the op without the magic "do what I really need"
flag, and it dies, and I have to reset and start it again, which
just takes longer than if -3 or -m was the default.

But that's not the situation everyone else has, so its reasonable
that -m ain't the default.  ;-)

-- 
Shawn.

Re: git rebase chokes on directory -> symlink -> directory

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>>  * git-rebase with -m is dog slow.  There were people who
>>    advocated to make it the default, but they probably are
>>    either working in a very small project, or working on a
>>    filesystem that even git-apply is slow that the speed
>>    difference does not matter to them.
> ...
> But that's not the situation everyone else has, so its reasonable
> that -m ain't the default.  ;-)

Well, that is not the conclusion you should be drawing from
this. If rebase -m is 10x slower than without -m in cases where
the rename handling does not matter, there is something wrong.

And what is wrong in this case is that the unpack-trees tree
merging code, which is used everywhere in git to do branch
switching and merges, is way too inefficient.

When merge-recursive is instructed to merge another tree with
the current tree using an ancestor, while taking the index into
account, it basically does the three-way tree-level merge one
path at a time, even when subdirectory at quite high level
matches identically across three trees.

The situation is the same for switching branches.  If two
branches of the kernel project (22k files spread across 1300
directories) differ at a file at the toplevel (e.g. v2.6.21
which changes only Makefile), we still read the index, the
current tree, and the other branch, and match all 22k files one
by one to compute the resulting index entry, by first removing
the current index entry and then stuffing the result entry in
the index, all the while trashing the cache-tree.  Then we
recompute all 1300 tree objects and write them out, even though
we should be able to notice that none of the toplevel 17
subdirectories have changed, and all we have to do is to rehash
one blob and recompute only one tree object at the toplevel.  We
boast how lightweight git branches are and how fast switching
between two branches is, but that's a serious lie.  If done
properly, we should be able to switch branches in a time roughly
proportional to the number of paths different between the
branches.  Currently, the time is proportional to the size of
the tree, no matter how small the change between trees are.

git-apply, which is used by rebase without -m, is optimized to
make it proportional to the size of the change.  It obviously
knows to only touch the affected paths (because the patch does
not talk about unaffected paths) and leave the others intact,
but also avoids expensive tree recomputation for unaffected
directories, by properly maintaining the cache-tree data in the
index.

IIRC, Linus said unpack-trees was beyond repair several months
ago, and I tend to agree with him.  Currently the first thing
unpack-trees does is to discard cache-tree from the index,
because the code does not properly invalidate affected paths,
and it is probably way too cumbersome to add it to various
places the code modifies the index (I haven't looked at it
recently, so maybe somebody can try it and prove me wrong).

My gut feeling is that we may be better off redoing the tree
level merge infrastructure from scratch, and make a new one that
is optimized for trees with small differences.  There is a
prototype code called test-para in 'pu' that implements such a
multi-tree walk, and also we've had its precursor (by Linus)
called git-merge-tree in 'master' for quite a long time, but
unfortunately neither has recently seen any activity.

Re: git rebase chokes on directory -> symlink -> directory

[Linus Torvalds]

On Thu, 10 May 2007, Junio C Hamano wrote:
> 
> And what is wrong in this case is that the unpack-trees tree
> merging code, which is used everywhere in git to do branch
> switching and merges, is way too inefficient.

Yes, it certainly could be speeded up, but on the other hand:

 - it's certainly not really "horribly bad" in most normal operations.

   In merging, it tends to be fast enough, and in checkouts the real 
   expense is checking out the tree, and the unpack-trees part really 
   isn't a big deal.

 - the only case it shows up is really just when you script things, and 
   compare it to just applying a patch, which is fundamnetally easier.

So I think the reason smarter tree merging hasn't gotten the love and 
attention to make it really a *lot* faster is simply that it's already 
quite fast enough for most people, and the "rebase using proper merges" is 
probably the only case where you can really see the difference.

I'd certainly love for the tree unpacking to handle all the "hey, whole 
sub-tree is identical" cases, but I can also see why it's not getting a 
lot of traction. It just hasn't been painful enough, and we're already too 
damn fast. 

Fixing the tree unpacking to do the trees in parallel (so that you don't 
have to shuffle the arrays and take a huge performance hit there) was 
enough to basically make all normal issues go away. 

That said, wouldn't it potentially be quite easy in "unpack_trees_rec()" 
to just notice when all the trees are identical, and just not recurse at 
all in that case (or - alternatively - recurse, but on entry, just exit 
quickly?)

		Linus

Re: git rebase chokes on directory -> symlink -> directory

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> That said, wouldn't it potentially be quite easy in "unpack_trees_rec()" 
> to just notice when all the trees are identical, and just not recurse at 
> all in that case (or - alternatively - recurse, but on entry, just exit 
> quickly?)

In the threeway case, it should be able to notice that the trees
of the ancestor and the other match, which is the only case we
can safely say that the result will be our tree (aka HEAD) and
we won't mess with the index nor worry about local modifications
made only to the working tree.  The case in which all trees
match, regardless of the number of trees involved, should work
the same way.

However, in the threeway case, we still need to make sure that
the index matches the HEAD even when we know the result will be
taken from HEAD, as otherwise we would end up including the
local "git add" made earlier in the merge result.  We need to
make sure the index matches HEAD for such a subdirectory in that
case before skipping it.

What is troublesome is that kind of policy logic is not supposed
to happen in unpack_trees_rec() in the current code structure;
instead, that decision is to be made by o->fn().

So, I am not sure if it is that quite easy.

[PATCH] apply: do not get confused by symlinks in the middle

[Junio C Hamano]

HPA noticed that git-rebase fails when changes involve symlinks
in the middle of the hierarchy.  Consider:

 * The tree state before the patch is applied has arch/x86_64/boot
   as a symlink pointing at ../i386/boot/

 * The patch tries to remove arch/x86_64/boot symlink, and
   create bunch of files there: .gitignore, Makefile, etc.

git-apply tries to be careful while applying patches; it never
touches the working tree until it is convinced that the patch
would apply cleanly.  One of the check it does is that when it
knows a path is going to be created by the patch, it runs
lstat() on the path to make sure it does not exist.

This leads to a false alarm.  Because we do not touch the
working tree before all the check passes, when we try to make
sure that arch/x86_64/boot/.gitignore does not exist yet, we
haven't removed the arch/x86_64/boot symlink.  The lstat() check
ends up seeing arch/i386/boot/.gitignore through the
yet-to-be-removed symlink, and says "Hey, you already have a
file there, but what you fed me is a patch to create a new
file. I am not going to clobber what you have in the working
tree."

We have similar checks to see a file we are going to modify does
exist and match the preimage of the diff, which is done by
directly opening and reading the file.

For a file we are going to delete, we make sure that it does
exist and matches what is going to be removed (a removal patch
records the full preimage, so we check what you have in your
working tree matches it in full -- otherwise we would risk
losing your local changes), which again is done by directly
opening and reading the file.

These checks need to be adjusted so that they are not fooled by
symlinks in the middle.

 - To make sure something does not exist, first lstat().  If it
   does not exist, it does not, so be happy.  If it _does_, we
   might be getting fooled by a symlink in the middle, so break
   leading paths and see if there are symlinks involved.  When
   we are checking for a path a/b/c/d, if any of a, a/b, a/b/c
   is a symlink, then a/b/c/d does _NOT_ exist, for the purpose
   of our test.

   This would fix this particular case you saw, and would not
   add extra overhead in the usual case.

 - To make sure something already exists, first lstat().  If it
   does not exist, barf (up to this, we already do).  Even if it
   does seem to exist, we might be getting fooled by a symlink
   in the middle, so make sure leading paths are not symlinks.

   This would make the normal codepath much more expensive for
   deep trees, which is a bit worrisome.

This patch implements the first side of the check "making sure
it does not exist".  The latter "making sure it exists" check is
not done yet, so applying the patch in reverse would still
fail, but we have to start from somewhere.

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

 * This should help the particular rebase go beyond the 0058
   patch.  While developing the new t4122 test I noticed that
   read-tree -m -u (aka git-checkout) seems to have a very
   similar problem, but I haven't dug into that yet.

 builtin-apply.c                 |   69 +++++++++++++++++++++++++++++++++-----
 t/t4122-apply-symlink-inside.sh |   57 ++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 9 deletions(-)
 create mode 100755 t/t4122-apply-symlink-inside.sh

diff --git a/builtin-apply.c b/builtin-apply.c
index f94d0db..38d20ef 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2009,6 +2009,63 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
 	return 0;
 }
 
+static int has_symlink_component(const char *new_name)
+{
+	char path[PATH_MAX];
+	const char *sp, *ep;
+	char *dp;
+
+	sp = new_name;
+	dp = path;
+
+	while (1) {
+		size_t len;
+		struct stat st;
+
+		ep = strchr(sp, '/');
+		if (!ep)
+			break;
+		len = ep - sp;
+		if (PATH_MAX <= dp + len - path + 2)
+			return 0; /* new name is longer than that??? */
+		memcpy(dp, sp, len);
+		dp[len] = 0;
+
+		if (lstat(path, &st))
+			return 0; /* why? we already lstat() new_name successfully. */
+		if (S_ISLNK(st.st_mode))
+			return 1;
+
+		dp[len++] = '/';
+		dp = dp + len;
+		sp = ep + 1;
+	}
+	return 0;
+}
+
+static int check_to_create_blob(const char *new_name, int ok_if_exists)
+{
+	struct stat nst;
+	if (!lstat(new_name, &nst)) {
+		if (S_ISDIR(nst.st_mode) || ok_if_exists)
+			return 0;
+		/*
+		 * A leading component of new_name might be a symlink
+		 * that is going to be removed with this patch, but
+		 * still pointing at somewhere that has the path.
+		 * In such a case, path "new_name" does not exist as
+		 * far as git is concerned.
+		 */
+		if (has_symlink_component(new_name))
+			return 0;
+
+		return error("%s: already exists in working directory", new_name);
+	}
+	else if ((errno != ENOENT) && (errno != ENOTDIR))
+		return error("%s: %s", new_name, strerror(errno));
+	return 0;
+}
+
 static int check_patch(struct patch *patch, struct patch *prev_patch)
 {
 	struct stat st;
@@ -2095,15 +2152,9 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)
 		    !ok_if_exists)
 			return error("%s: already exists in index", new_name);
 		if (!cached) {
-			struct stat nst;
-			if (!lstat(new_name, &nst)) {
-				if (S_ISDIR(nst.st_mode) || ok_if_exists)
-					; /* ok */
-				else
-					return error("%s: already exists in working directory", new_name);
-			}
-			else if ((errno != ENOENT) && (errno != ENOTDIR))
-				return error("%s: %s", new_name, strerror(errno));
+			int err = check_to_create_blob(new_name, ok_if_exists);
+			if (err)
+				return err;
 		}
 		if (!patch->new_mode) {
 			if (0 < patch->is_new)
diff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh
new file mode 100755
index 0000000..37c9a9f
--- /dev/null
+++ b/t/t4122-apply-symlink-inside.sh
@@ -0,0 +1,57 @@
+#!/bin/sh
+
+test_description='apply to deeper directory without getting fooled with symlink'
+. ./test-lib.sh
+
+lecho () {
+	for l_
+	do
+		echo "$l_"
+	done
+}
+
+test_expect_success setup '
+
+	mkdir -p arch/i386/boot arch/x86_64 &&
+	lecho 1 2 3 4 5 >arch/i386/boot/Makefile &&
+	ln -s ../i386/boot arch/x86_64/boot &&
+	git add . &&
+	test_tick &&
+	git commit -m initial &&
+	git branch test &&
+
+	rm arch/x86_64/boot &&
+	mkdir arch/x86_64/boot &&
+	lecho 2 3 4 5 6 >arch/x86_64/boot/Makefile &&
+	git add . &&
+	test_tick &&
+	git commit -a -m second &&
+
+	git format-patch --binary -1 --stdout >test.patch
+
+'
+
+test_expect_success apply '
+
+	git checkout test &&
+	git reset --hard && #### checkout seems to be buggy
+	git diff --exit-code test &&
+	git diff --exit-code --cached test &&
+	git apply --index test.patch
+
+'
+
+test_expect_success 'check result' '
+
+	git diff --exit-code master &&
+	git diff --exit-code --cached master &&
+	test_tick &&
+	git commit -m replay &&
+	T1=$(git rev-parse "master^{tree}") &&
+	T2=$(git rev-parse "HEAD^{tree}") &&
+	test "z$T1" = "z$T2"
+
+'
+
+test_done
+
-- 
1.5.2.rc3.706.g498a

Re: git rebase chokes on directory -> symlink -> directory

[Junio C Hamano]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Mine stops already at the directory -> symlink checkin (the above is the
> symlink -> directory one), but your trick of using "git checkout" as a
> trick to resolve things helped for both... eventually :-/

I've tried to redo your rebase using:

	apply: do not get confused by symlinks in the middle

patch on top of 'master'.  It successfully run through the end.
After rebasing f1bb07af ("rebase-1" in your repository) on to
a989705 (near the tip of Linus), I did

	git diff --stat --summary a989705...f1bb07af
        git diff --stat --summary a989705...HEAD

(that is, "show me the change since the merge base") and the
results from these two diffs match exactly.

So I think I can declare victory for now ;-).

However.

I usually have "[apply] whitespace = strip" in my ~/.gitconfig,
but during this verification run, I disabled it to keep rebase
from falling back to 3-way merge using merge-recursive.  If I
turn it on, rebase still fails and I strongly suspect "rebase
-m" would fail the same way, although I haven't tried it (it
takes too much time).

I'll be somewhat busy this weekend, so I would welcome anybody
else beating me to fixing the problem in merge-recursive.

Re: git rebase chokes on directory -> symlink -> directory

[H. Peter Anvin]

Junio C Hamano wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
>> Mine stops already at the directory -> symlink checkin (the above is the
>> symlink -> directory one), but your trick of using "git checkout" as a
>> trick to resolve things helped for both... eventually :-/
> 
> I've tried to redo your rebase using:
> 
> 	apply: do not get confused by symlinks in the middle
> 
> patch on top of 'master'.  It successfully run through the end.
> 
> So I think I can declare victory for now ;-).
> 

YAY!  Huge thanks!

	-hpa

Re: git rebase chokes on directory -> symlink -> directory

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> However.
>
> I usually have "[apply] whitespace = strip" in my ~/.gitconfig,
> but during this verification run, I disabled it to keep rebase
> from falling back to 3-way merge using merge-recursive.  If I
> turn it on, rebase still fails and I strongly suspect "rebase
> -m" would fail the same way, although I haven't tried it (it
> takes too much time).
>
> I'll be somewhat busy this weekend, so I would welcome anybody
> else beating me to fixing the problem in merge-recursive.

Yuck.  merge-recursive does seem to have problem with D/F
conflict in general; I suspect this is not limited to cases that
involve symbolic links.

First, the setup.

$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-newsetup.git/
$ cd linux-2.6-newsetup.git
$ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/
$ git branch other b2ad90f4969226fe8cf3edc5330711ed5fc20105
$ git branch ancestor other^1
$ git reset --hard ancestor
$ echo >>Makefile ; git add Makefile ; git commit -m 'change one'
$ git branch try

To see that the underlying read-tree is working as expected (and
do not have to be debugged), we can first try the usual three-way
read tree:

$ git read-tree -m -u ancestor HEAD other
$ git ls-files -u --abbrev arch/x86-64

This would show...

    120000 ad3f146 3       arch/x86_64/boot
    100644 495f20c 1       arch/x86_64/boot/.gitignore
    100644 495f20c 2       arch/x86_64/boot/.gitignore
    100644 ee6f650 1       arch/x86_64/boot/Makefile
    100644 ee6f650 2       arch/x86_64/boot/Makefile
    ...

The current HEAD (the one that we tagged as 'try') has the
directory arch/x86_64/boot/, but all of its files are unchanged
from the "common ancestor", so they all have identical stages 1
and 2, with stage 3 missing.  The other tree has boot/ as a
symlink.  These "One-side removes other side does not touch" and
"one-side adds" cases are left unmerged by "read-tree -m -u", as
that is how merge-recursive can find renames to begin with.

Now we've seen what the read-tree (which is the same machinery
used as git_merge_trees() in mege-recursive) does with these
three trees, let's see how merge-recursive finishes this off:

    $ git reset --hard try
    $ git merge-recursive ancestor -- HEAD other
    CONFLICT (directory/file): There is a directory with name arch/x86_64/boot in HEAD. Added arch/x86_64/boot as arch/x86_64/boot~other
    Removed arch/x86_64/boot/.gitignore
    Removed arch/x86_64/boot/Makefile
    Removed arch/x86_64/boot/bootsect.S
    Removed arch/x86_64/boot/compressed/Makefile
    Removed arch/x86_64/boot/compressed/head.S
    Removed arch/x86_64/boot/compressed/misc.c
    Removed arch/x86_64/boot/compressed/vmlinux.lds
    Removed arch/x86_64/boot/compressed/vmlinux.scr
    Removed arch/x86_64/boot/install.sh
    Removed arch/x86_64/boot/mtools.conf.in
    Removed arch/x86_64/boot/setup.S
    Removed arch/x86_64/boot/tools/.gitignore
    Removed arch/x86_64/boot/tools/build.c

There is no rename, so rename detection does not interfere, but
the D/F conflict detection code in merge-recursive thinks that
arch/x86_64/boot is a directory in one tree (yes, it is true in
the original tree, but it goes away as all files under it), and
a non-directory in another.  And instead of replacing the boot/
directory that becomes empty with a new symlink boot/, it
creates the boot~funny-name symlink and fails the operation.

When it actually is checking out arch/x86_64/boot out of the
resolved index, it should notice that (1) arch/x86_64/boot
directory is unnecessary to house anything in the resulting
index anymore, and that (2) there is no locally created
untracked file that is 'precious' [*1*].  Then it can rmdir()
and create the boot/ symlink in its place.  To fix this, it may
be necessary to update its checkout code to perform the "remove
first then create" two-pass process git-apply does.

It is unfortunate that for merge-recursive it is probably too
cumbersome to always do the right thing, but I think it should
at least notice that arch/x86_64/boot whose files all disappear
does not conflict with creation of the new symlink.