47863 2010-10-18 16:57:05 -0700 scm.py should be able tell us what revisions made changes to a given file 2010-10-18 18:38:16 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 abarth abarth commit-queue eric oldest_to_newest 295841 0 abarth 2010-10-18 16:57:05 -0700 scm.py should be able tell us what revisions made changes to a given file 295843 1 71101 abarth 2010-10-18 16:58:28 -0700 Created attachment 71101 Patch 295850 2 71101 eric 2010-10-18 17:15:49 -0700 Comment on attachment 71101 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71101&action=review Looks OK. Testing could be better. r- for no error-case testing. Otherwise looks fine. > WebKitTools/Scripts/webkitpy/common/checkout/scm.py:249 > + self._subclass_must_implement() using a helper function to raise confuses pylint. I had it in an earlier version of scm.py, but then moved to copy/paste exception throwing instead. Someone must have added it back. > WebKitTools/Scripts/webkitpy/common/checkout/scm.py:434 > + revisions = [] Any time you find yourself doing this, it may be cleaner as a list-comprehension. :) > WebKitTools/Scripts/webkitpy/common/checkout/scm.py:437 > + match = re.search('^r(?P<revision>\d+) ', line) If you worry this is hot we could store this in a class variable as a compiled regexp. > WebKitTools/Scripts/webkitpy/common/checkout/scm.py:440 > + if not match: > + continue > + revisions.append(int(match.group('revision'))) So you coudl break this out into a self._revision_from_line(line) and then this function is a simple list-comprehension. :) I guess it would have to be revisions = [self._revision_from_line(line) for line in lines] return [revision for revision in revisions if revision] # filter out None from non-revision lines of output. I'm not sure that's actually cleaner, but maybe I'm missing a slick way to do the filtering. > WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py:356 > + self.assertEqual(self.scm.revisions_changing_file("test_file"), [5, 4, 3, 2]) Are there other interesting cases to test? What happens if you pass a non-existant file? What about a file which hasnt' been changed in the latest revision? 295857 3 71101 eric 2010-10-18 17:25:18 -0700 Comment on attachment 71101 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71101&action=review > WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py:357 > + self.assertRaises(ScriptError, self.scm.revisions_changing_file, "non_existent_file") Um. Clearly I'm blind. OK. 295892 4 71101 commit-queue 2010-10-18 18:38:11 -0700 Comment on attachment 71101 Patch Clearing flags on attachment: 71101 Committed r70014: <http://trac.webkit.org/changeset/70014> 295893 5 commit-queue 2010-10-18 18:38:16 -0700 All reviewed patches have been landed. Closing bug. 71101 2010-10-18 16:58:28 -0700 2010-10-18 18:38:10 -0700 Patch bug-47863-20101018165826.patch text/plain 5002 abarth ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCA5ZjY4ZTNlZDExNzVhNTI0Y2MyOTI3MzJlZjlmNThjMTJiNzNmZjkyLi40MTI3ODdk MGRjOGFjODgzNjY4MWFmNmJlZTFiZjdhYTlkODgyMzU2IDEwMDY0NAotLS0gYS9XZWJLaXRUb29s cy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAK KzIwMTAtMTAtMTggIEFkYW0gQmFydGggIDxhYmFydGhAd2Via2l0Lm9yZz4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBzY20ucHkgc2hvdWxkIGJlIGFi bGUgdGVsbCB1cyB3aGF0IHJldmlzaW9ucyBtYWRlIGNoYW5nZXMgdG8gYSBnaXZlbiBmaWxlCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00Nzg2MworCisg ICAgICAgIExvb2sgYWdhaW4sIHlvdXIgU0NNLnB5IGNhbiBub3cgbG9nIGZpbGVzLgorCisgICAg ICAgICogU2NyaXB0cy93ZWJraXRweS9jb21tb24vY2hlY2tvdXQvc2NtLnB5OgorICAgICAgICAq IFNjcmlwdHMvd2Via2l0cHkvY29tbW9uL2NoZWNrb3V0L3NjbV91bml0dGVzdC5weToKKwogMjAx MC0xMC0xOCAgQW5kZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCiAgICAgICAg IEZpeCBidWlsZC4KZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29t bW9uL2NoZWNrb3V0L3NjbS5weSBiL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29tbW9u L2NoZWNrb3V0L3NjbS5weQppbmRleCA3OTNkOTZkM2VmYzJlMWIyZDZlYWQzMWRkODI0MTBmM2Q0 ZWU5MDE1Li42YzJjMTg5MzNkZmE1MDE2NWE5YjEwMjcxZWM3NTNhM2NhNWFjMDZhIDEwMDY0NAot LS0gYS9XZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9jaGVja291dC9zY20ucHkK KysrIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9jb21tb24vY2hlY2tvdXQvc2NtLnB5 CkBAIC0yNDUsNyArMjQ1LDEwIEBAIGNsYXNzIFNDTToKICAgICBkZWYgY2hhbmdlZF9maWxlcyhz ZWxmLCBnaXRfY29tbWl0PU5vbmUpOgogICAgICAgICBzZWxmLl9zdWJjbGFzc19tdXN0X2ltcGxl bWVudCgpCiAKLSAgICBkZWYgY2hhbmdlZF9maWxlc19mb3JfcmV2aXNpb24oc2VsZik6CisgICAg ZGVmIGNoYW5nZWRfZmlsZXNfZm9yX3JldmlzaW9uKHNlbGYsIHJldmlzaW9uKToKKyAgICAgICAg c2VsZi5fc3ViY2xhc3NfbXVzdF9pbXBsZW1lbnQoKQorCisgICAgZGVmIHJldmlzaW9uc19jaGFu Z2luZ19maWxlKHNlbGYsIHBhdGgsIGxpbWl0PTUpOgogICAgICAgICBzZWxmLl9zdWJjbGFzc19t dXN0X2ltcGxlbWVudCgpCiAKICAgICBkZWYgYWRkZWRfZmlsZXMoc2VsZik6CkBAIC00MjcsNiAr NDMwLDE2IEBAIGNsYXNzIFNWTihTQ00pOgogICAgICAgICBzdGF0dXNfY29tbWFuZCA9IFsic3Zu IiwgImRpZmYiLCAiLS1zdW1tYXJpemUiLCAiLWMiLCByZXZpc2lvbl0KICAgICAgICAgcmV0dXJu IHNlbGYucnVuX3N0YXR1c19hbmRfZXh0cmFjdF9maWxlbmFtZXMoc3RhdHVzX2NvbW1hbmQsIHNl bGYuX3N0YXR1c19yZWdleHAoIkFDRE1SIikpCiAKKyAgICBkZWYgcmV2aXNpb25zX2NoYW5naW5n X2ZpbGUoc2VsZiwgcGF0aCwgbGltaXQ9NSk6CisgICAgICAgIHJldmlzaW9ucyA9IFtdCisgICAg ICAgIGxvZ19jb21tYW5kID0gWydzdm4nLCAnbG9nJywgJy0tcXVpZXQnLCAnLS1saW1pdD0lcycg JSBsaW1pdCwgcGF0aF0KKyAgICAgICAgZm9yIGxpbmUgaW4gc2VsZi5ydW4obG9nX2NvbW1hbmQs IGN3ZD1zZWxmLmNoZWNrb3V0X3Jvb3QpLnNwbGl0bGluZXMoKToKKyAgICAgICAgICAgIG1hdGNo ID0gcmUuc2VhcmNoKCdecig/UDxyZXZpc2lvbj5cZCspICcsIGxpbmUpCisgICAgICAgICAgICBp ZiBub3QgbWF0Y2g6CisgICAgICAgICAgICAgICAgY29udGludWUKKyAgICAgICAgICAgIHJldmlz aW9ucy5hcHBlbmQoaW50KG1hdGNoLmdyb3VwKCdyZXZpc2lvbicpKSkKKyAgICAgICAgcmV0dXJu IHJldmlzaW9ucworCiAgICAgZGVmIGNvbmZsaWN0ZWRfZmlsZXMoc2VsZik6CiAgICAgICAgIHJl dHVybiBzZWxmLnJ1bl9zdGF0dXNfYW5kX2V4dHJhY3RfZmlsZW5hbWVzKHNlbGYuc3RhdHVzX2Nv bW1hbmQoKSwgc2VsZi5fc3RhdHVzX3JlZ2V4cCgiQyIpKQogCkBAIC02NTMsNiArNjY2LDEwIEBA IGNsYXNzIEdpdChTQ00pOgogICAgICAgICBjb21taXRfaWQgPSBzZWxmLmdpdF9jb21taXRfZnJv bV9zdm5fcmV2aXNpb24ocmV2aXNpb24pCiAgICAgICAgIHJldHVybiBzZWxmLl9jaGFuZ2VzX2Zp bGVzX2Zvcl9jb21taXQoY29tbWl0X2lkKQogCisgICAgZGVmIHJldmlzaW9uc19jaGFuZ2luZ19m aWxlKHNlbGYsIHBhdGgsIGxpbWl0PTUpOgorICAgICAgICBjb21taXRfaWRzID0gc2VsZi5ydW4o WyJnaXQiLCAibG9nIiwgIi0tcHJldHR5PWZvcm1hdDolSCIsICItJXMiICUgbGltaXQsIHBhdGhd KS5zcGxpdGxpbmVzKCkKKyAgICAgICAgcmV0dXJuIG1hcChzZWxmLnN2bl9yZXZpc2lvbl9mcm9t X2dpdF9jb21taXQsIGNvbW1pdF9pZHMpCisKICAgICBkZWYgY29uZmxpY3RlZF9maWxlcyhzZWxm KToKICAgICAgICAgIyBXZSBkbyBub3QgbmVlZCB0byBwYXNzIGRlY29kZV9vdXRwdXQgZm9yIHRo aXMgZGlmZiBjb21tYW5kCiAgICAgICAgICMgYXMgd2UncmUgcGFzc2luZyAtLW5hbWUtc3RhdHVz IHdoaWNoIGRvZXMgbm90IG91dHB1dCBhbnkgZGF0YS4KQEAgLTY4OCw2ICs3MDUsOSBAQCBjbGFz cyBHaXQoU0NNKToKICAgICAgICAgICAgIHJhaXNlIFNjcmlwdEVycm9yKG1lc3NhZ2U9J0ZhaWxl ZCB0byBmaW5kIGdpdCBjb21taXQgZm9yIHJldmlzaW9uICVzLCB5b3VyIGNoZWNrb3V0IGxpa2Vs eSBuZWVkcyBhbiB1cGRhdGUuJyAlIHJldmlzaW9uKQogICAgICAgICByZXR1cm4gZ2l0X2NvbW1p dAogCisgICAgZGVmIHN2bl9yZXZpc2lvbl9mcm9tX2dpdF9jb21taXQoc2VsZiwgY29tbWl0X2lk KToKKyAgICAgICAgcmV0dXJuIGludChzZWxmLnJ1bihbJ2dpdCcsICdzdm4nLCAnZmluZC1yZXYn LCBjb21taXRfaWRdKS5yc3RyaXAoKSkKKwogICAgIGRlZiBjb250ZW50c19hdF9yZXZpc2lvbihz ZWxmLCBwYXRoLCByZXZpc2lvbik6CiAgICAgICAgICIiIlJldHVybnMgYSBieXRlIGFycmF5IChz dHIoKSkgY29udGFpbmluZyB0aGUgY29udGVudHMKICAgICAgICAgb2YgcGF0aCBAIHJldmlzaW9u IGluIHRoZSByZXBvc2l0b3J5LiIiIgpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93 ZWJraXRweS9jb21tb24vY2hlY2tvdXQvc2NtX3VuaXR0ZXN0LnB5IGIvV2ViS2l0VG9vbHMvU2Ny aXB0cy93ZWJraXRweS9jb21tb24vY2hlY2tvdXQvc2NtX3VuaXR0ZXN0LnB5CmluZGV4IDg3ZDU1 MzliNTg0Y2UwMzc4YjFiODJmZGMzMzRiMTJjNzBhN2RjOWMuLmVkOGYwMDk3MGEyODlmY2JhM2M5 MzNmMmYzODlmZTg4NGQ4Y2EyNWMgMTAwNjQ0Ci0tLSBhL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Vi a2l0cHkvY29tbW9uL2NoZWNrb3V0L3NjbV91bml0dGVzdC5weQorKysgYi9XZWJLaXRUb29scy9T Y3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9jaGVja291dC9zY21fdW5pdHRlc3QucHkKQEAgLTM1Miw2 ICszNTIsMTAgQEAgY2xhc3MgU0NNVGVzdCh1bml0dGVzdC5UZXN0Q2FzZSk6CiAgICAgICAgIHNl bGYuYXNzZXJ0UmFpc2VzKFNjcmlwdEVycm9yLCBzZWxmLnNjbS5jb250ZW50c19hdF9yZXZpc2lv biwgInRlc3RfZmlsZTIiLCAyKQogICAgICAgICBzZWxmLmFzc2VydFJhaXNlcyhTY3JpcHRFcnJv ciwgc2VsZi5zY20uY29udGVudHNfYXRfcmV2aXNpb24sICJkb2VzX25vdF9leGlzdCIsIDIpCiAK KyAgICBkZWYgX3NoYXJlZF90ZXN0X3JldmlzaW9uc19jaGFuZ2luZ19maWxlKHNlbGYpOgorICAg ICAgICBzZWxmLmFzc2VydEVxdWFsKHNlbGYuc2NtLnJldmlzaW9uc19jaGFuZ2luZ19maWxlKCJ0 ZXN0X2ZpbGUiKSwgWzUsIDQsIDMsIDJdKQorICAgICAgICBzZWxmLmFzc2VydFJhaXNlcyhTY3Jp cHRFcnJvciwgc2VsZi5zY20ucmV2aXNpb25zX2NoYW5naW5nX2ZpbGUsICJub25fZXhpc3RlbnRf ZmlsZSIpCisKICAgICBkZWYgX3NoYXJlZF90ZXN0X2NvbW1pdHRlcl9lbWFpbF9mb3JfcmV2aXNp b24oc2VsZik6CiAgICAgICAgIHNlbGYuYXNzZXJ0RXF1YWwoc2VsZi5zY20uY29tbWl0dGVyX2Vt YWlsX2Zvcl9yZXZpc2lvbigzKSwgZ2V0cGFzcy5nZXR1c2VyKCkpICAjIENvbW1pdHRlciAiZW1h aWwiIHdpbGwgYmUgdGhlIGN1cnJlbnQgdXNlcgogCkBAIC02OTYsNiArNzAwLDkgQEAgUTFkVEJ4 MEFBQUI0Mml0ZzRHbGdZSmpHd01ERHlPRE14TUR3MzRHQmdRRUFKUFFESkE9PQogICAgIGRlZiB0 ZXN0X2NvbnRlbnRzX2F0X3JldmlzaW9uKHNlbGYpOgogICAgICAgICBzZWxmLl9zaGFyZWRfdGVz dF9jb250ZW50c19hdF9yZXZpc2lvbigpCiAKKyAgICBkZWYgdGVzdF9yZXZpc2lvbnNfY2hhbmdp bmdfZmlsZShzZWxmKToKKyAgICAgICAgc2VsZi5fc2hhcmVkX3Rlc3RfcmV2aXNpb25zX2NoYW5n aW5nX2ZpbGUoKQorCiAgICAgZGVmIHRlc3RfY29tbWl0dGVyX2VtYWlsX2Zvcl9yZXZpc2lvbihz ZWxmKToKICAgICAgICAgc2VsZi5fc2hhcmVkX3Rlc3RfY29tbWl0dGVyX2VtYWlsX2Zvcl9yZXZp c2lvbigpCiAKQEAgLTExOTksNiArMTIwNiw5IEBAIGNsYXNzIEdpdFNWTlRlc3QoU0NNVGVzdCk6 CiAgICAgZGVmIHRlc3RfY29udGVudHNfYXRfcmV2aXNpb24oc2VsZik6CiAgICAgICAgIHNlbGYu X3NoYXJlZF90ZXN0X2NvbnRlbnRzX2F0X3JldmlzaW9uKCkKIAorICAgIGRlZiB0ZXN0X3Jldmlz aW9uc19jaGFuZ2luZ19maWxlKHNlbGYpOgorICAgICAgICBzZWxmLl9zaGFyZWRfdGVzdF9yZXZp c2lvbnNfY2hhbmdpbmdfZmlsZSgpCisKICAgICBkZWYgdGVzdF9hZGRlZF9maWxlcyhzZWxmKToK ICAgICAgICAgc2VsZi5fc2hhcmVkX3Rlc3RfYWRkZWRfZmlsZXMoKQogCg==