16052 2007-11-19 10:30:09 -0800 prepare-ChangeLog doesn't report deleted files 2007-11-25 23:07:11 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 kevino ddkilzer aroben ddkilzer oldest_to_newest 61693 0 kevino 2007-11-19 10:30:09 -0800 prepare-ChangeLog currently only reports added or modified files, so any deleted files won't be in the ChangeLog unless manually added. 61694 1 17402 kevino 2007-11-19 10:30:43 -0800 Created attachment 17402 Patch to get prepare-ChangeLog to report deleted files 61743 2 17402 ddkilzer 2007-11-19 22:43:30 -0800 Comment on attachment 17402 Patch to get prepare-ChangeLog to report deleted files Marking r- since this is the wrong fix. The prepare-ChangeLog script already reports removed files for svn. I just realized that the actual bug is that prepare-ChangeLog ignores changes staged to the index for git. For example, if you use "git rm path/to/file" instead of "rm path/to/file", the prepare-ChangeLog script ignores this change since it doesn't use the --cached switch on git-diff. Also, if you git-add changed files before running prepare-ChangeLog, those changes won't be seen, either. Should we be running git-diff with and without "--cached", or should we be looking at the output of git-status and parsing both of its sections? Kevin, are you using svn or git? Does the above explain the issue you're seeing? 61744 3 ddkilzer 2007-11-19 22:44:12 -0800 Confirming issue with git and changes staged to index. 61750 4 kevino 2007-11-19 23:01:25 -0800 (In reply to comment #2) > (From update of attachment 17402 [edit]) > Marking r- since this is the wrong fix. > > The prepare-ChangeLog script already reports removed files for svn. > > I just realized that the actual bug is that prepare-ChangeLog ignores changes > staged to the index for git. For example, if you use "git rm path/to/file" > instead of "rm path/to/file", the prepare-ChangeLog script ignores this change > since it doesn't use the --cached switch on git-diff. > > Also, if you git-add changed files before running prepare-ChangeLog, those > changes won't be seen, either. > > Should we be running git-diff with and without "--cached", or should we be > looking at the output of git-status and parsing both of its sections? > > Kevin, are you using svn or git? Does the above explain the issue you're > seeing? > I'm using svn. I don't have git on my machine, and I removed the files with svn remove path/to/file, so I'm not sure how it could be an issue with git in my case. Are you sure it handles files removed in svn correctly? Because neither the isAdded or isModified functions check for the "D" status, and if those functions don't return true, then how is the file added to @changedFiles? 61753 5 aroben 2007-11-19 23:12:46 -0800 (In reply to comment #2) > Should we be running git-diff with and without "--cached", or should we be > looking at the output of git-status and parsing both of its sections? I think we want to stay away from git-status since it operates over the whole repository. I'm surprised there isn't a way to get git-diff to do what we want in a single invocation. 61754 6 ddkilzer 2007-11-19 23:33:56 -0800 (In reply to comment #4) > I'm using svn. I don't have git on my machine, and I removed the files with svn > remove path/to/file, so I'm not sure how it could be an issue with git in my > case. Are you sure it handles files removed in svn correctly? Because neither > the isAdded or isModified functions check for the "D" status, and if those > functions don't return true, then how is the file added to @changedFiles? I see the issue now. (I'll file a separate bug on the git issue.) If you only have deleted files in the tree, then generateFileList() returns no changes and the script exits. 1. svn rm WebKitTools/mangleme/README 2. ./WebKitTools/Scripts/prepare-ChangeLog WebKitTools To undo these changes (for any new svn users reading this): svn revert WebKitTools/ChangeLog WebKitTools/mangleme/README However, we don't want to add removed (deleted) files to @changed_files since this is used to create a list of added/changed method names for source files. After playing with the script for a while, I think the "No changes found." test is broken. It currently reads: if (!@changed_files && !@conflict_files || !%function_lists) { I think it should read: if (!@changed_files && !@conflict_files && !scalar keys %function_lists) { I believe that should fix the issue. 61756 7 ddkilzer 2007-11-19 23:38:25 -0800 (In reply to comment #5) > I think we want to stay away from git-status since it operates over the whole > repository. I'm surprised there isn't a way to get git-diff to do what we want > in a single invocation. (In reply to comment #6) > I see the issue now. (I'll file a separate bug on the git issue.) Bug 16060 61757 8 ddkilzer 2007-11-19 23:39:56 -0800 (In reply to comment #6) > if (!@changed_files && !@conflict_files && !scalar keys %function_lists) { Or just: > if (!@changed_files && !@conflict_files && !keys %function_lists) { 62349 9 17523 ddkilzer 2007-11-25 22:36:45 -0800 Created attachment 17523 Patch v2 Simpler fix. If there were only removed files, the prepare-ChangeLog script would always report no changes because the check for !%function_lists did not work as expected. We now check for !keys %function_lists instead, and make sure there are no changed files (added, modified), no conflicted files and no removed files. 62361 10 ddkilzer 2007-11-25 23:07:11 -0800 Committed revision 28037. 17402 2007-11-19 10:30:43 -0800 2007-11-25 22:36:45 -0800 Patch to get prepare-ChangeLog to report deleted files prepare-changelog-deletedfiles.patch text/plain 1362 kevino SW5kZXg6IFdlYktpdFRvb2xzL1NjcmlwdHMvcHJlcGFyZS1DaGFuZ2VMb2cKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gV2ViS2l0VG9vbHMvU2NyaXB0cy9wcmVwYXJlLUNoYW5nZUxvZwkocmV2aXNpb24gMjc5MDYp CisrKyBXZWJLaXRUb29scy9TY3JpcHRzL3ByZXBhcmUtQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkp CkBAIC0xMDU0LDcgKzEwNTQsNyBAQAogCiAgICAgICAgICRmaWxlID0gbWFrZUZpbGVQYXRoUmVs YXRpdmUoJGZpbGUpOwogCi0gICAgICAgIGlmIChpc01vZGlmaWVkU3RhdHVzKCRzdGF0dXMpIHx8 IGlzQWRkZWRTdGF0dXMoJHN0YXR1cykpIHsKKyAgICAgICAgaWYgKGlzTW9kaWZpZWRTdGF0dXMo JHN0YXR1cykgfHwgaXNBZGRlZFN0YXR1cygkc3RhdHVzKSB8fCBpc1JlbW92ZWRTdGF0dXMoJHN0 YXR1cykpIHsKICAgICAgICAgICAgIG15IEBjb21wb25lbnRzID0gRmlsZTo6U3BlYy0+c3BsaXRk aXIoJGZpbGUpOwogICAgICAgICAgICAgaWYgKCRjb21wb25lbnRzWzBdIGVxICJMYXlvdXRUZXN0 cyIpIHsKICAgICAgICAgICAgICAgICAkZGlkQ2hhbmdlUmVncmVzc2lvblRlc3RzID0gMTsKQEAg LTEwOTksNiArMTA5OSwxNyBAQAogICAgIHJldHVybiAkc3RhdHVzQ29kZXN7JHN0YXR1c307CiB9 CiAKK3N1YiBpc1JlbW92ZWRTdGF0dXMoJCkKK3sKKyAgICBteSAoJHN0YXR1cykgPSBAXzsKKwor ICAgIG15ICVzdGF0dXNDb2RlcyA9ICgKKyAgICAgICAgIkQiID0+IDEsCisgICAgKTsKKworICAg IHJldHVybiAkc3RhdHVzQ29kZXN7JHN0YXR1c307Cit9CisKIHN1YiBpc0FkZGVkU3RhdHVzKCQp CiB7CiAgICAgbXkgKCRzdGF0dXMpID0gQF87CkluZGV4OiBXZWJLaXRUb29scy9DaGFuZ2VMb2cK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCShyZXZpc2lvbiAyNzkxMCkKKysr IFdlYktpdFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDExIEBACisy MDA3LTExLTE5ICBLZXZpbiBPbGxpdmllciAgPGtldmlub0B0aGVvbGxpdmllcnMuY29tPgorCisg ICAgICAgIE1ha2Ugc3VyZSBwcmVwYXJlLUNoYW5nZUxvZyBoYW5kbGVzIGRlbGV0ZWQgZmlsZXMg YXMgd2VsbC4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIFNjcmlwdHMvcHJlcGFyZS1DaGFuZ2VMb2c6CisKIDIwMDctMTEtMTkgIEFscCBUb2tlciAg PGFscEBhdG9rZXIuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IE1hcmsgUm93ZS4K 17523 2007-11-25 22:36:45 -0800 2007-11-25 22:43:45 -0800 Patch v2 bug-16052-v2.diff text/plain 1166 ddkilzer SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDI4MDM1KQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAKKzIwMDctMTEtMjUgIERhdmlkIEQuIEtpbHplciAg PGRka2lsemVyQHdlYmtpdC5vcmc+CisKKyAgICAgICAgQnVnIDE2MDUyOiBwcmVwYXJlLUNoYW5n ZUxvZyBkb2Vzbid0IHJlcG9ydCBkZWxldGVkIGZpbGVzCisgICAgICAgIDxodHRwOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjA1Mj4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNjcmlwdHMvcHJlcGFyZS1DaGFuZ2VMb2c6IEZp eGVkIGxvZ2ljIHRoYXQgY2hlY2tzIGZvciByZW1vdmVkIGZpbGVzLgorCiAyMDA3LTExLTI1ICBE YXZpZCBLaWx6ZXIgIDxkZGtpbHplckB3ZWJraXQub3JnPgogCiAgICAgICAgIEJ1ZyAxNTg2NDog UmVwbGFjZSBtZXJnZS1jaGFuZ2Vsb2cgd2l0aCByZXNvbHZlLUNoYW5nZUxvZ3MKSW5kZXg6IFdl YktpdFRvb2xzL1NjcmlwdHMvcHJlcGFyZS1DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViS2l0 VG9vbHMvU2NyaXB0cy9wcmVwYXJlLUNoYW5nZUxvZwkocmV2aXNpb24gMjgwMzQpCisrKyBXZWJL aXRUb29scy9TY3JpcHRzL3ByZXBhcmUtQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xMzcs NyArMTM3LDcgQEAgbXkgJGRpZENoYW5nZVJlZ3Jlc3Npb25UZXN0cyA9IDA7CiAKIGdlbmVyYXRl RmlsZUxpc3QoQGNoYW5nZWRfZmlsZXMsIEBjb25mbGljdF9maWxlcywgJWZ1bmN0aW9uX2xpc3Rz KTsKIAotaWYgKCFAY2hhbmdlZF9maWxlcyAmJiAhQGNvbmZsaWN0X2ZpbGVzIHx8ICElZnVuY3Rp b25fbGlzdHMpIHsKK2lmICghQGNoYW5nZWRfZmlsZXMgJiYgIUBjb25mbGljdF9maWxlcyAmJiAh a2V5cyAlZnVuY3Rpb25fbGlzdHMpIHsKICAgICBwcmludCBTVERFUlIgIiAgTm8gY2hhbmdlcyBm b3VuZC5cbiI7CiAgICAgZXhpdCAxOwogfQo=