{"thread":{"id":"873","subject":"git bug?","startedAt":"2005-06-09T06:54:37Z","lastAt":"2005-06-09T08:15:41Z","messageCount":5,"participants":["Jeff Garzik","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"4757","messageId":"42A7E7AD.5030108@pobox.com","threadId":"873","inReplyTo":null,"subject":"git bug?","fromName":"Jeff Garzik","fromEmail":"jgarzik@pobox.com","sentAt":"2005-06-09T06:54:37Z","receivedAt":"2005-06-09T06:54:37Z","isPatch":false,"sender":{"key":"jgarzik@pobox.com","avatar":null},"body":"\nJust upgraded to the latest git, pulled the latest Linux kernel tree, \nand made a local modification. Here is the strange git-diff-cache output:\n\n> [jgarzik@pretzel libata-dev]$ git-diff-cache -p HEAD \n> diff --git a/arch/arm/mm/minicache.c b/arch/arm/mm/minicache.c\n> deleted file mode 100644\n> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c\n> --- a/drivers/scsi/libata-core.c\n> +++ b/drivers/scsi/libata-core.c\n> @@ -3059,8 +3059,6 @@ static void __ata_qc_complete(struct ata\n> struct ata_port *ap = qc->ap;\n> unsigned int tag, do_clear = 0;\n> \n> - WARN_ON(!assert_spin_locked(&ap->host_set->lock));\n> -\n> if (likely(qc->flags & ATA_QCFLAG_ACTIVE)) {\n> assert(ap->queue_depth);\n> ap->queue_depth--;\n\nThe libata-core part is correct, the arch/arm/mm part is not.\n\nI pulled using unmodified, upstream git scripts. No sugar added. \nHere's what the repo looks like:\n\n> [jgarzik@pretzel libata-dev]$ git-rev-list --pretty HEAD ^master | git-shortlog \n> Jeff Garzik:\n> Merge /spare/repo/linux-2.6/\n> Automatic merge of /spare/repo/linux-2.6/.git branch HEAD\n> \n> Jens Axboe:\n> libata: fix spinlock bug introduced by NCQ code\n> libata: ncq support update\n> libata-scsi: better placement of cmd completion on err\n> SATA NCQ support\n\nIt's mirroring to kernel.org right now, at \nrsync.kernel.org://...jgarzik/libata-dev.git if somebody wants to poke \nat it. Top of tree is d032ec9048ff82a704b96b93cfd6f2e8e3a06b19 (when \nfully uploaded and mirrored).\n\n\tJeff\n\n\n\n"},{"id":"4762","messageId":"42A7F21C.2040709@pobox.com","threadId":"873","inReplyTo":"42A7E7AD.5030108@pobox.com","subject":"Re: git bug?","fromName":"Jeff Garzik","fromEmail":"jgarzik@pobox.com","sentAt":"2005-06-09T07:39:08Z","receivedAt":"2005-06-09T07:39:08Z","isPatch":false,"sender":{"key":"jgarzik@pobox.com","avatar":null},"body":"Jeff Garzik wrote:\n> \n> Just upgraded to the latest git, pulled the latest Linux kernel tree, \n> and made a local modification. Here is the strange git-diff-cache output:\n> \n>> [jgarzik@pretzel libata-dev]$ git-diff-cache -p HEAD diff --git \n>> a/arch/arm/mm/minicache.c b/arch/arm/mm/minicache.c\n>> deleted file mode 100644\n>> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c\n>> --- a/drivers/scsi/libata-core.c\n>> +++ b/drivers/scsi/libata-core.c\n>> @@ -3059,8 +3059,6 @@ static void __ata_qc_complete(struct ata\n>> struct ata_port *ap = qc->ap;\n>> unsigned int tag, do_clear = 0;\n>> \n>> - WARN_ON(!assert_spin_locked(&ap->host_set->lock));\n>> -\n>> if (likely(qc->flags & ATA_QCFLAG_ACTIVE)) {\n>> assert(ap->queue_depth);\n>> ap->queue_depth--;\n> \n> \n> The libata-core part is correct, the arch/arm/mm part is not.\n> \n> I pulled using unmodified, upstream git scripts. No sugar added. Here's \n> what the repo looks like:\n> \n>> [jgarzik@pretzel libata-dev]$ git-rev-list --pretty HEAD ^master | \n>> git-shortlog Jeff Garzik:\n>> Merge /spare/repo/linux-2.6/\n>> Automatic merge of /spare/repo/linux-2.6/.git branch HEAD\n>>\n>> Jens Axboe:\n>> libata: fix spinlock bug introduced by NCQ code\n>> libata: ncq support update\n>> libata-scsi: better placement of cmd completion on err\n>> SATA NCQ support\n> \n> \n> It's mirroring to kernel.org right now, at \n> rsync.kernel.org://...jgarzik/libata-dev.git if somebody wants to poke \n> at it. Top of tree is d032ec9048ff82a704b96b93cfd6f2e8e3a06b19 (when \n> fully uploaded and mirrored).\n\n\nWell, after switching branches and switching back, the behavior is gone. \n \n\n\tJeff\n\n\n"},{"id":"4769","messageId":"7vzmu0otg8.fsf@assigned-by-dhcp.cox.net","threadId":"873","inReplyTo":"7vy89kq96o.fsf@assigned-by-dhcp.cox.net","subject":"Re: git bug?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T07:55:19Z","receivedAt":"2005-06-09T07:55:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff, I assume you are talking about this part:\n\n>> diff --git a/arch/arm/mm/minicache.c b/arch/arm/mm/minicache.c\n>> deleted file mode 100644\n>> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c\n\nthat does not have anything between \"deleted file...\" and the\nnext diff about libata-core.c.\n\nIn short, this is not really a \"bug\", but I am open to\nsuggestions for improvements (but you need to talk Linus into\naccepting the changes, because changing this would also affect\nhis \"git-apply\" program as well).\n\nThe current \"diff-patch\" format is generated this way:\n\n (0) if a modification is between different kind of objects\n (e.g. a regular file changed to a symlink), then the\n following steps are done twice; one \"deletion\" of the\n original, and another \"creation\" of the result.\n\n (1) \"diff --git a/oldname b/newname\" line is shown first and it\n is always shown. Unless it is a rename/copy diff, oldname\n and newname always match. We never say /dev/null on this\n line to represent creation/deletion.\n\n (2) Next, optional \"diff --git extended headers\" appear. See\n apply.c for the list of things to expect.\n\n (3) Next, the output from \"diff -u -L -L\"\n between the two file contents follows. Here, oldname and\n newname are usually of the form \"a/oldname\" and\n \"b/oldname\", but we _do_ follow /dev/null convention for\n file creation and deletion here. That is, you would see\n\n --- /dev/null\n +++ frotz.c\n @@ ...\n\n for a file creation, and the opposite for a file deletion.\n\n Unfortunately, if \"diff\" finds nothing to report, you would\n not even see these ---/+++ lines. I think that is what\n puzzled you.\n\nBy the way, this is all documented (?) behaviour, reported first\nby Linus in this message.\n\n Message-ID: \n From: Linus Torvalds \n Subject: git full diff output issues..\n Date: Thu, 26 May 2005 12:19:21 -0700 (PDT)\n Cc: Git Mailing List \n\n While testing my \"git-apply\" thing (coming along quite nicely, thanks for\n asking), I've hit a case that is nasty to parse.\n\n This is from the 2.6.12-rc4 -> 2.6.12-rc5 patch:\n\n diff --git a/arch/um/kernel/checksum.c b/arch/um/kernel/checksum.c\n deleted file mode 100644\n diff --git a/arch/um/kernel/initrd.c b/arch/um/kernel/initrd.c\n new file mode 100644\n --- /dev/null\n +++ b/arch/um/kernel/initrd.c\n @@ -0,0 +1,78 @@\n\n and the magic here is that deleted file that was empty to begin with, so \n it didn't have a patch, just a note on deletion.\n\n Why is that nasty? Because we don't have the file _name_ in any good \n format. The filename only exists int he \"diff --git\" header, and that one \n has the space-parsing issue, which makes it less than optimal.\n\n (the rest omitted)\n\nWe had a handful of back-and-forth back then and the resolution\nwas this:\n\n Message-ID: \n From: Linus Torvalds \n Subject: Re: git full diff output issues..\n Date: Thu, 26 May 2005 13:33:26 -0700 (PDT)\n Cc: Junio C Hamano , Git Mailing List \n\n (some omitted)\n\n However, I ended up just validating the name parsing by making sure that \n when I parse the \"git --diff\" line, I only take the name if I can see it \n being the same for both the old and the new. ...\n\n (the rest omitted)\n\nBy the way, this is not just \"deleting empty files\". Any change\nthat does not involve content changes you would see something\nsimilar to what you found.\n\n Message-ID: \n From: Linus Torvalds \n Subject: Re: git full diff output issues..\n Date: Sun, 5 Jun 2005 08:11:02 -0700 (PDT)\n Cc: git@vger.kernel.org\n\n (some omitted)\n\n The only case that was special was literally the \"same name, no content \n changes, new mode\" case, which looked like\n\n diff --git a/oldname.c b/oldname.c\n new mode 100755\n old mode 100644\n\n and thus _only_ had the name in the (normally ambiguous wrt whitepsace) \n header line.\n\n"},{"id":"4767","messageId":"42A7F769.2050800@pobox.com","threadId":"873","inReplyTo":"7vzmu0otg8.fsf@assigned-by-dhcp.cox.net","subject":"Re: git bug?","fromName":"Jeff Garzik","fromEmail":"jgarzik@pobox.com","sentAt":"2005-06-09T08:01:45Z","receivedAt":"2005-06-09T08:01:45Z","isPatch":false,"sender":{"key":"jgarzik@pobox.com","avatar":null},"body":"Junio C Hamano wrote:\n> Jeff, I assume you are talking about this part:\n> \n> \n>>>diff --git a/arch/arm/mm/minicache.c b/arch/arm/mm/minicache.c\n>>>deleted file mode 100644\n>>>diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c\n> \n> \n> that does not have anything between \"deleted file...\" and the\n> next diff about libata-core.c.\n> \n> In short, this is not really a \"bug\", but I am open to\n> suggestions for improvements (but you need to talk Linus into\n> accepting the changes, because changing this would also affect\n> his \"git-apply\" program as well).\n\nIt's a bug and a new behavior.\n\n'git-diff-cache -p HEAD' should not show files that I have not modified.\n\nThis screws up\n\tgit-diff-cache -p HEAD | diffstat -p1 | awk '{print $1}'\nfor example.\n\n\tJeff\n\n\n"},{"id":"4763","messageId":"7vmzq0osia.fsf@assigned-by-dhcp.cox.net","threadId":"873","inReplyTo":"42A7F769.2050800@pobox.com","subject":"Re: git bug?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T08:15:41Z","receivedAt":"2005-06-09T08:15:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"JG\" == Jeff Garzik writes:\n\nJG> 'git-diff-cache -p HEAD' should not show files that I have not modified.\nJG> This screws up\nJG> \tgit-diff-cache -p HEAD | diffstat -p1 | awk '{print $1}'\nJG> for example.\n\nAgreed 100%. If there is a file exists (rather, if the git\nthinks the file is there), then diff-cache should not say it is\ndeleted. That would be wrong.\n\nSorry for not having a crystal ball to poke into your terminal,\nso let me get something straight. I am still unsure the problem\nyou are reporting is coming from diff-cache bug, or somehow\nyesterday's (well it is 01:13AM where I sit ;-)) merge updates\ntrashed your work tree or index file, so these questions would\nhelp me tell which. When \"diff-cache\" reports the file\n\"deleted\":\n\n - Do you have that arch/arm/mm/minicache.c file in your working\n directory?\n\n - Does git-ls-files know about the file? Does the path show in:\n git-ls-files --stage | grep arch/arm/mm/minicache.c\n\n - Does the HEAD tree have the path as well? Does it show in:\n git-ls-tree -r HEAD | grep arch/arm/mm/minicache.c\n\n\n"}]}