Skip to contentSkip to conversation

fix compare symlink against readlink not data

PATCH In review Kay Sievers 4 participants 9 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix compare symlink against readlink not data

Fix update-cache to compare the blob of a symlink against the link-target and not the file it points to. Also ignore all permissions applied to links.
Thanks to Greg for recognizing this while he added our list of symlinks back to the udev repository.

---

diff-files.c +1 -1
@@ -111,7 +111,7 @@ int main(int argc, char **argv)
 			continue;
 		}
  
-		if (stat(ce->name, &st) < 0) {
+		if (lstat(ce->name, &st) < 0) {
 			if (errno != ENOENT) {
 				perror(ce->name);
 				continue;
read-cache.c +3 -3
@@ -16,6 +16,9 @@ int cache_match_stat(struct cache_entry 
 	switch (ntohl(ce->ce_mode) & S_IFMT) {
 	case S_IFREG:
 		changed |= !S_ISREG(st->st_mode) ? TYPE_CHANGED : 0;
+		/* We consider only the owner x bit to be relevant for "mode changes" */
+		if (0100 & (ntohl(ce->ce_mode) ^ st->st_mode))
+			changed |= MODE_CHANGED;
 		break;
 	case S_IFLNK:
 		changed |= !S_ISLNK(st->st_mode) ? TYPE_CHANGED : 0;
@@ -43,9 +46,6 @@ int cache_match_stat(struct cache_entry 
 	if (ce->ce_uid != htonl(st->st_uid) ||
 	    ce->ce_gid != htonl(st->st_gid))
 		changed |= OWNER_CHANGED;
-	/* We consider only the owner x bit to be relevant for "mode changes" */
-	if (0100 & (ntohl(ce->ce_mode) ^ st->st_mode))
-		changed |= MODE_CHANGED;
 	if (ce->ce_dev != htonl(st->st_dev) ||
 	    ce->ce_ino != htonl(st->st_ino))
 		changed |= INODE_CHANGED;
update-cache.c +44 -5

76 lines not on this page · raw message

#1 lore raw
Quoted 5 lines · Kay Sievers

Fix update-cache to compare the blob of a symlink against the link-target and not the file it points to. Also ignore all permissions applied to links.
Thanks to Greg for recognizing this while he added our list of symlinks back to the udev repository.

Hm, even with this patch applied (it's in Linus's tree right now), I still get the following with a clean checked out udev tree:

 $ cg-diff
 Index: test/sys/block/cciss!c0d0/device
 ===================================================================
 Index: test/sys/block/rd!c0d0/device
 ===================================================================
 Index: test/sys/block/sda/device
 ===================================================================
 Index: test/sys/bus/pci/devices/0000:00:09.0
 ===================================================================
 Index: test/sys/bus/pci/devices/0000:00:1e.0
 ===================================================================
 Index: test/sys/bus/pci/devices/0000:02:05.0
 ===================================================================
 Index: test/sys/bus/pci/drivers/aic7xxx/0000:02:05.0
 ===================================================================
 Index: test/sys/bus/scsi/devices/0:0:0:0
 ===================================================================
 Index: test/sys/bus/scsi/drivers/sd/0:0:0:0
 ===================================================================
 Index: test/sys/bus/usb-serial/devices/ttyUSB0
 ===================================================================
 Index: test/sys/bus/usb-serial/drivers/PL-2303/ttyUSB0
 ===================================================================
 Index: test/sys/bus/usb/devices/3-0:1.0
 ===================================================================
 Index: test/sys/bus/usb/devices/3-1
 ===================================================================
 Index: test/sys/bus/usb/devices/3-1:1.0
 ===================================================================
 Index: test/sys/bus/usb/devices/usb3
 ===================================================================
 Index: test/sys/bus/usb/drivers/hub/3-0:1.0
 ===================================================================
 Index: test/sys/bus/usb/drivers/pl2303/3-1:1.0
 ===================================================================
 Index: test/sys/bus/usb/drivers/usb/3-1
 ===================================================================
 Index: test/sys/bus/usb/drivers/usb/usb3
 ===================================================================
 Index: test/sys/class/tty/ttyUSB0/device
 ===================================================================
 Index: test/sys/devices/pci0000:00/0000:00:09.0/usb3/3-1/ttyUSB0/driver
 ===================================================================

Any idea?

thanks,

greg k-h

#2 lore raw
Quoted 7 lines · Greg KH
Quoted 5 lines · Kay Sievers

Fix update-cache to compare the blob of a symlink against the link-target and not the file it points to. Also ignore all permissions applied to links.
Thanks to Greg for recognizing this while he added our list of symlinks back to the udev repository.

Quoted 5 lines

Hm, even with this patch applied (it's in Linus's tree right now), I still get the following with a clean checked out udev tree:

 $ cg-diff
 Index: test/sys/block/cciss!c0d0/device
 ===================================================================

I can't reproduce this. Are you sure, that the git-core binaries are called and not the cogito ones?

git-update-cache --refresh
git-diff-cache -r HEAD
from the core-git should print nothing.

Thanks,
Kay

#3 lore raw
Quoted 14 lines · Kay Sievers
Quoted 7 lines · Greg KH
Quoted 5 lines · Kay Sievers

Fix update-cache to compare the blob of a symlink against the link-target and not the file it points to. Also ignore all permissions applied to links.
Thanks to Greg for recognizing this while he added our list of symlinks back to the udev repository.

Quoted 5 lines

Hm, even with this patch applied (it's in Linus's tree right now), I still get the following with a clean checked out udev tree:

 $ cg-diff
 Index: test/sys/block/cciss!c0d0/device
 ===================================================================
Quoted 6 lines

I can't reproduce this. Are you sure, that the git-core binaries are called and not the cogito ones?

git-update-cache --refresh
git-diff-cache -r HEAD
from the core-git should print nothing.

Odd. If I reclone the whole tree from the udev kernel.org tree, then it works just fine. If I create a new copy of my local tree, I still have the same problem. Diffing the trees shows no difference in the objects at all...

Can you add a symlink to a local tree and see if you can duplicate this?

thanks,

greg k-h

#4 lore raw
Quoted 22 lines · Greg KH
Quoted 14 lines · Kay Sievers
Quoted 7 lines · Greg KH
Quoted 5 lines · Kay Sievers

Fix update-cache to compare the blob of a symlink against the link-target and not the file it points to. Also ignore all permissions applied to links.
Thanks to Greg for recognizing this while he added our list of symlinks back to the udev repository.

Quoted 5 lines

Hm, even with this patch applied (it's in Linus's tree right now), I still get the following with a clean checked out udev tree:

 $ cg-diff
 Index: test/sys/block/cciss!c0d0/device
 ===================================================================
Quoted 6 lines

I can't reproduce this. Are you sure, that the git-core binaries are called and not the cogito ones?

git-update-cache --refresh
git-diff-cache -r HEAD
from the core-git should print nothing.

Quoted 6 lines

Odd. If I reclone the whole tree from the udev kernel.org tree, then it works just fine. If I create a new copy of my local tree, I still have the same problem. Diffing the trees shows no difference in the objects at all...

Can you add a symlink to a local tree and see if you can duplicate this?

Works as it should be.
What happens when you throw away your old .git/index with?
git-read-tree HEAD

Kay

#5 lore raw
In reply to Greg KH
Quoted 5 lines · Greg KH

Odd. If I reclone the whole tree from the udev kernel.org tree, then it works just fine. If I create a new copy of my local tree, I still have the same problem. Diffing the trees shows no difference in the objects at all...

You've not updated your cache.

Guys, remember this command:

git-diff-files

Just like that, with no arguments. It shows you what is different in your cache. If you get a lot of output, it means that your index file isn't up-to-date.

The other magic command is

	git-update-cache --refresh

and you need to do that after you merge.

If you use cogito, and cogito doesn't refresh after pulls etc, that would be a cogito bug. But if you do things like "cp -a" of a git tree, and you forget to refresh the cache in the new tree, than that is _your_ bug..

Linus

#6 lore raw
Quoted 15 lines · Linus Torvalds
Quoted 5 lines · Greg KH

Odd. If I reclone the whole tree from the udev kernel.org tree, then it works just fine. If I create a new copy of my local tree, I still have the same problem. Diffing the trees shows no difference in the objects at all...

You've not updated your cache.

Guys, remember this command:

git-diff-files

Quoted 7 lines

Just like that, with no arguments. It shows you what is different in your cache. If you get a lot of output, it means that your index file isn't up-to-date.

The other magic command is

	git-update-cache --refresh

Damm, I still was using update-cache and checkout-cache from an old git version. That was my problem.

Sorry for the noise, it works just fine.

thanks,

greg k-h

#7 lore raw
In reply to Greg KH
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"GKH" == Greg KH <[email protected]> writes:

Quoted 2 lines · Kay Sievers
Quoted 2 lines

Thanks to Greg for recognizing this while he added our list of symlinks back to the udev repository.

GKH> Hm, even with this patch applied (it's in Linus's tree right now), I GKH> still get the following with a clean checked out udev tree:

GKH>  $ cg-diff
GKH>  Index: test/sys/block/cciss!c0d0/device
GKH>  ===================================================================
GKH>  Index: test/sys/block/rd!c0d0/device
GKH>  ===================================================================

GKH> Any idea?

I do not use Cogito but probably it is this piece of code in cg-Xdiffdo. It is assuming that a valid SHA1 means it can diff against the filesystem object that resides there. It used to be the case before symlinks but not anymore.

    mkbanner () {
            loc=$1; treeid=$2; fname=$3; mode=$4; sha1=$5;

            if [ "$sha1" != "0000000000000000000000000000000000000000" ]; then
                    git-cat-file blob $sha1 >$loc
            else
                    ln -s "$(pwd)/$fname" "$loc"
                    sha1="!"
            fi

Maybe changing the if to (I'm writing this in e-mail editor so completely untested) something like:

    if expr "$sha1" : '0*$' >/dev/null ||
       expr "$mode" : '.*120000$' >/dev/null
    then
        git-cat-file blob "$sha1" >$loc

would help?

Also could you try the low-level git command, git-diff-cache -p, against the tree you are comparing? The built-in diff stuff might get this wrong too.

#8 lore raw
Quoted 4 lines · Junio C Hamano

Also could you try the low-level git command, git-diff-cache -p, against the tree you are comparing? The built-in diff stuff might get this wrong too.

No, 'git-diff-cache -r HEAD' shows me:

$ git-diff-cache -r HEAD
*120000->100644 blob    2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a      test/sys/block/cciss!c0d0/device
*120000->100644 blob    2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a      test/sys/block/rd!c0d0/device
*120000->100644 blob    2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a      test/sys/block/sda/device
*120000->100644 blob    1c776568bdc9dc750addd0885dded6b008a44460->1c776568bdc9dc750addd0885dded6b008a44460      test/sys/bus/pci/devices/0000:00:09.0
*120000->100644 blob    e000c77614a23ad57fed284bd007ed7c1cb7872e->e000c77614a23ad57fed284bd007ed7c1cb7872e      test/sys/bus/pci/devices/0000:00:1e.0
*120000->100644 blob    630d35bf617944a4ba6afc90ca5176cb342a2662->630d35bf617944a4ba6afc90ca5176cb342a2662      test/sys/bus/pci/devices/0000:02:05.0
*120000->100644 blob    bd644e0e9d0c2f289bc4a3e3a034d528d5d671cc->bd644e0e9d0c2f289bc4a3e3a034d528d5d671cc      test/sys/bus/pci/drivers/aic7xxx/0000:02:05.0
*120000->100644 blob    ebb65b3bac36ef935a55a7f1010e4d3a242188eb->ebb65b3bac36ef935a55a7f1010e4d3a242188eb      test/sys/bus/scsi/devices/0:0:0:0
*120000->100644 blob    239003f712dd9112171e635a44160da1898f5996->239003f712dd9112171e635a44160da1898f5996      test/sys/bus/scsi/drivers/sd/0:0:0:0
*120000->100644 blob    b7733a68e08e564300212a22c9f81888c12bb55a->b7733a68e08e564300212a22c9f81888c12bb55a      test/sys/bus/usb-serial/devices/ttyUSB0
*120000->100644 blob    177f109e4899cf4008b9413933392d4f07832fdc->177f109e4899cf4008b9413933392d4f07832fdc      test/sys/bus/usb-serial/drivers/PL-2303/ttyUSB0
*120000->100644 blob    9137978832942ecce572d376f14244c1588748a2->9137978832942ecce572d376f14244c1588748a2      test/sys/bus/usb/devices/3-0:1.0
*120000->100644 blob    e47b4d58c4e5406bdba3ea1384c0c3efe007b8f6->e47b4d58c4e5406bdba3ea1384c0c3efe007b8f6      test/sys/bus/usb/devices/3-1
*120000->100644 blob    f519185eb36af29f79ca89d4b3d51011756b6837->f519185eb36af29f79ca89d4b3d51011756b6837      test/sys/bus/usb/devices/3-1:1.0
*120000->100644 blob    fb1919e7c9794ce31a257b50621f71f6f4f8bdef->fb1919e7c9794ce31a257b50621f71f6f4f8bdef      test/sys/bus/usb/devices/usb3
*120000->100644 blob    2bc160c20cd950c52e34d4bab30e1e25d6f4df34->2bc160c20cd950c52e34d4bab30e1e25d6f4df34      test/sys/bus/usb/drivers/hub/3-0:1.0
*120000->100644 blob    49d32d5abd7e26766f4c905f1d4edf1e28f8b322->49d32d5abd7e26766f4c905f1d4edf1e28f8b322      test/sys/bus/usb/drivers/pl2303/3-1:1.0
*120000->100644 blob    03c76193e99a93c7ff45c9ac986d2bc8e0706b0b->03c76193e99a93c7ff45c9ac986d2bc8e0706b0b      test/sys/bus/usb/drivers/usb/3-1
*120000->100644 blob    61dc52a61345178c8c171ecfe96df9646af2f16c->61dc52a61345178c8c171ecfe96df9646af2f16c      test/sys/bus/usb/drivers/usb/usb3
*120000->100644 blob    b7733a68e08e564300212a22c9f81888c12bb55a->b7733a68e08e564300212a22c9f81888c12bb55a      test/sys/class/tty/ttyUSB0/device
*120000->100644 blob    9ff2c81f529a95bd93ddaf66de6a72c74166c268->9ff2c81f529a95bd93ddaf66de6a72c74166c268      test/sys/devices/pci0000:00/0000:00:09.0/usb3/3-1/ttyUSB0/driver

thanks,

greg k-h