174551 2017-07-15 17:43:27 -0700 [Scripts] Make svn-create-patch work better when called in sub directories 2017-07-17 10:22:15 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=167164 P2 Normal --- 1 sam sam ap buildbot cdumez darin dbates jbedard lforschler simon.fraser thorton oldest_to_newest 1329256 0 sam 2017-07-15 17:43:27 -0700 [Scripts] Make svn-create-patch work better when called in sub directories 1329257 1 315573 sam 2017-07-15 17:47:05 -0700 Created attachment 315573 Patch 1329260 2 sam 2017-07-15 17:52:24 -0700 Ugh, my changelog is terrible. The real bug this is fixing is that when you move/rename a file, and then use svn-create-patch to create a patch for it, you currently have to be at the root of the svn tree. This fixes things so you can once again do it from the subdirectory. 1329279 3 315573 darin 2017-07-15 20:13:07 -0700 Comment on attachment 315573 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315573&action=review > Tools/Scripts/svn-create-patch:287 > + # svn may output explanitory lines describing more detail about a file change Spelling error here: explanatory > Tools/Scripts/svn-create-patch:289 > + next if $line =~ / +>/; I think you want a ^ at the start of the regular expression here, before the space. > Tools/Scripts/svn-create-patch:378 > + print STDERR "Performing `svn diff -r 0:${sourceRevision} ${escapedSourceFile} | tail -n +5`\n" if $verbose; I would not use backquotes in STDERR. That’s perl syntax to mean "execute this and turn the result into a string", but doesn’t seem right for the logging. Maybe just normal \" quotes? > Tools/Scripts/svn-create-patch:380 > print `svn diff -r 0:${sourceRevision} ${escapedSourceFile} | tail -n +5`; It’s surprising that we use print combined with backquotes here. Kind of a peculiar way to run a command and let the output go to STDOUT as normal. I think there are better perl idioms for this. 1329294 4 sam 2017-07-15 20:45:17 -0700 Committed revision 219540. 1329296 5 sam 2017-07-15 20:46:27 -0700 (In reply to Darin Adler from comment #3) > Comment on attachment 315573 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=315573&action=review > > > Tools/Scripts/svn-create-patch:287 > > + # svn may output explanitory lines describing more detail about a file change > > Spelling error here: explanatory Fixed. > > > Tools/Scripts/svn-create-patch:289 > > + next if $line =~ / +>/; > > I think you want a ^ at the start of the regular expression here, before the > space. Yup. Fixed. > > > Tools/Scripts/svn-create-patch:378 > > + print STDERR "Performing `svn diff -r 0:${sourceRevision} ${escapedSourceFile} | tail -n +5`\n" if $verbose; > > I would not use backquotes in STDERR. That’s perl syntax to mean "execute > this and turn the result into a string", but doesn’t seem right for the > logging. Maybe just normal \" quotes? Switched to quotes. > > > Tools/Scripts/svn-create-patch:380 > > print `svn diff -r 0:${sourceRevision} ${escapedSourceFile} | tail -n +5`; > > It’s surprising that we use print combined with backquotes here. Kind of a > peculiar way to run a command and let the output go to STDOUT as normal. I > think there are better perl idioms for this. Switch to the more common idiom on the file of using open/close, and implemented tail with a loop and counter. Thanks for the review. 315573 2017-07-15 17:47:05 -0700 2017-07-15 20:13:07 -0700 Patch bug-174551-20170715174858.patch text/plain 6415 sam SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDIxOTUzOSkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDI0IEBACisyMDE3LTA3LTE1ICBTYW0gV2VpbmlnICA8c2FtQHdlYmtpdC5vcmc+CisKKyAg ICAgICAgW1NjcmlwdHNdIE1ha2Ugc3ZuLWNyZWF0ZS1wYXRjaCB3b3JrIGJldHRlciB3aGVuIGNh bGxlZCBpbiBzdWIgZGlyZWN0b3JpZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTE3NDU1MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIC0gRml4ZXMgbWFudWZhY3R1cmVQYXRjaEZvckFkZGl0aW9uV2l0aEhp c3RvcnkgdG8gdXNlIHRoZSBjb3JyZWN0IHBhdGgKKyAgICAgICAgICBieSBlbnN1cmluZyB0aGF0 IHRoZSBwcmVmaXggaXMgYXBwbGllZCBhcyBpcyBkb25lIGluIGdlbmVyYXRlRGlmZi4KKyAgICAg ICAgLSBTaWxlbmNlIG91dHB1dCBvZiBleHBsYW5hdG9yeSBsaW5lcyBmcm9tIHN2biBzdGF0ICgi ID4gbW92ZSB0byAuLi4iKQorICAgICAgICAgIHRoYXQgc2hvdyB1cCBpbiBTVERFUlIgd2hlbiBy dW5uaW5nIHN2bi1jcmVhdGUtcGF0Y2guCisgICAgICAgIC0gQWRkIHZlcmJvc2UgbG9nZ2luZyB0 aGF0IEkgZm91bmQgdXNlZnVsIHdoaWxlIGRlYnVnZ2luZyB0aGlzIGlzc3VlLgorCisgICAgICAg ICogU2NyaXB0cy9zdm4tY3JlYXRlLXBhdGNoOgorICAgICAgICAoZmluZEJhc2VVcmwpOgorICAg ICAgICAoZmluZE1pbWVUeXBlKToKKyAgICAgICAgKGZpbmRTb3VyY2VGaWxlQW5kUmV2aXNpb24p OgorICAgICAgICAoZ2VuZXJhdGVEaWZmKToKKyAgICAgICAgKGdlbmVyYXRlRmlsZUxpc3QpOgor ICAgICAgICAobWFudWZhY3R1cmVQYXRjaEZvckFkZGl0aW9uV2l0aEhpc3RvcnkpOgorCiAyMDE3 LTA3LTE1ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNvbT4KIAogICAgICAgICBV bnJldmlld2VkIGJ1aWxkIGZpeCBhZnRlciByMjE5NTM3LgpJbmRleDogVG9vbHMvU2NyaXB0cy9z dm4tY3JlYXRlLXBhdGNoCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL1NjcmlwdHMvc3ZuLWNyZWF0ZS1w YXRjaAkocmV2aXNpb24gMjE5NTM3KQorKysgVG9vbHMvU2NyaXB0cy9zdm4tY3JlYXRlLXBhdGNo CSh3b3JraW5nIGNvcHkpCkBAIC02NSw3ICs2NSw3IEBAIHN1YiBnZW5lcmF0ZURpZmYoJCQpOwog c3ViIGdlbmVyYXRlRmlsZUxpc3QoJFwlKTsKIHN1YiBodW5rSGVhZGVyTGluZVJlZ0V4Rm9yRmls ZSgkKTsKIHN1YiBpc0JpbmFyeU1pbWVUeXBlKCQpOwotc3ViIG1hbnVmYWN0dXJlUGF0Y2hGb3JB ZGRpdGlvbldpdGhIaXN0b3J5KCQpOworc3ViIG1hbnVmYWN0dXJlUGF0Y2hGb3JBZGRpdGlvbldp dGhIaXN0b3J5KCQkKTsKIHN1YiBudW1lcmljY21wKCQkKTsKIHN1YiBvdXRwdXRCaW5hcnlDb250 ZW50KCQpOwogc3ViIHBhdGNocGF0aGNtcCgkJCk7CkBAIC03OSwxNSArNzksMTcgQEAgJEVOVnsn TENfQUxMJ30gPSAnQyc7CiBteSAkc2hvd0hlbHA7CiBteSAkY2hlY2tXZWJLaXRTdHlsZSA9IDA7 CiBteSAkaWdub3JlQ2hhbmdlbG9ncyA9IDA7CitteSAkdmVyYm9zZSA9IDA7CiBteSAkZGV2TnVs bCA9IEZpbGU6OlNwZWMtPmRldm51bGwoKTsKIAogbXkgJHJlc3VsdCA9IEdldE9wdGlvbnMoCiAg ICAgImhlbHAiICAgICAgID0+IFwkc2hvd0hlbHAsCiAgICAgImlnbm9yZS1jaGFuZ2Vsb2dzIiA9 PiBcJGlnbm9yZUNoYW5nZWxvZ3MsCiAgICAgInN0eWxlISIgPT4gXCRjaGVja1dlYktpdFN0eWxl LAorICAgICJ2ZXJib3NlIiA9PiBcJHZlcmJvc2UsCiApOwogaWYgKCEkcmVzdWx0IHx8ICRzaG93 SGVscCkgewotICAgIHByaW50IFNUREVSUiBiYXNlbmFtZSgkMCkgLiAiIFstaHwtLWhlbHBdIFst LWlnbm9yZS1jaGFuZ2Vsb2dzXSBbLS1bbm8tXXN0eWxlXSBbc3ZuZGlyMSBbc3ZuZGlyMiAuLi5d XVxuIjsKKyAgICBwcmludCBTVERFUlIgYmFzZW5hbWUoJDApIC4gIiBbLWh8LS1oZWxwXSBbLXZ8 LS12ZXJib3NlXSBbLS1pZ25vcmUtY2hhbmdlbG9nc10gWy0tW25vLV1zdHlsZV0gW3N2bmRpcjEg W3N2bmRpcjIgLi4uXV1cbiI7CiAgICAgZXhpdCAxOwogfQogCkBAIC0xMDAsNiArMTAyLDEzIEBA IGZvciBteSAkcGF0aCAoa2V5cyAlcGF0aHMpIHsKICAgICBnZW5lcmF0ZUZpbGVMaXN0KCRwYXRo LCAlZGlmZkZpbGVzKTsKIH0KIAoraWYgKCR2ZXJib3NlKSB7CisgICAgcHJpbnQgU1RERVJSICJM aXN0IG9mIGZpbGVzIGZvciBwYXRjaDpcbiI7CisgICAgZm9yIG15ICRmaWxlIChzb3J0IHBhdGNo cGF0aGNtcCB2YWx1ZXMgJWRpZmZGaWxlcykgeworICAgICAgICBwcmludCBTVERFUlIgIiAgIiAu ICRmaWxlLT57cGF0aH0gLiAiXG4iOworICAgIH0KK30KKwogbXkgJHN2blJvb3QgPSBkZXRlcm1p bmVTVk5Sb290KCk7CiBteSAkcHJlZml4ID0gY2hkaXJSZXR1cm5pbmdSZWxhdGl2ZVBhdGgoJHN2 blJvb3QpOwogCkBAIC0xNTcsNiArMTY2LDkgQEAgc3ViIGZpbmRCYXNlVXJsKCQpCiAgICAgbXkg KCRpbmZvUGF0aCkgPSBAXzsKICAgICBteSAkYmFzZVVybDsKICAgICBteSAkZXNjYXBlZEluZm9Q YXRoID0gZXNjYXBlU3VidmVyc2lvblBhdGgoJGluZm9QYXRoKTsKKyAgICAKKyAgICBwcmludCBT VERFUlIgIlBlcmZvcm1pbmcgYHN2biBpbmZvICckZXNjYXBlZEluZm9QYXRoJ2BcbiIgaWYgJHZl cmJvc2U7CisgICAgCiAgICAgb3BlbiBJTkZPLCAic3ZuIGluZm8gJyRlc2NhcGVkSW5mb1BhdGgn IHwiIG9yIGRpZTsKICAgICB3aGlsZSAoPElORk8+KSB7CiAgICAgICAgIGlmICgvXlVSTDogKC4r PylbXHJcbl0qJC8pIHsKQEAgLTE3Miw2ICsxODQsOSBAQCBzdWIgZmluZE1pbWVUeXBlKCQ7JCkK ICAgICBteSAoJGZpbGUsICRyZXZpc2lvbikgPSBAXzsKICAgICBteSAkYXJncyA9ICRyZXZpc2lv biA/ICItLXJldmlzaW9uICRyZXZpc2lvbiIgOiAiIjsKICAgICBteSAkZXNjYXBlZEZpbGUgPSBl c2NhcGVTdWJ2ZXJzaW9uUGF0aCgkZmlsZSk7CisKKyAgICBwcmludCBTVERFUlIgIlBlcmZvcm1p bmcgYHN2biBwcm9wZ2V0IHN2bjptaW1lLXR5cGUgJGFyZ3MgJyRlc2NhcGVkRmlsZScgMj4gJGRl dk51bGxgXG4iIGlmICR2ZXJib3NlOworCiAgICAgb3BlbiBQUk9QR0VULCAic3ZuIHByb3BnZXQg c3ZuOm1pbWUtdHlwZSAkYXJncyAnJGVzY2FwZWRGaWxlJyAyPiAkZGV2TnVsbCB8IiBvciBkaWU7 CiAgICAgbXkgJG1pbWVUeXBlID0gPFBST1BHRVQ+OwogICAgIGNsb3NlIFBST1BHRVQ7CkBAIC0y MDMsNiArMjE4LDkgQEAgc3ViIGZpbmRTb3VyY2VGaWxlQW5kUmV2aXNpb24oJCkKICAgICBteSAk c291cmNlRmlsZTsKICAgICBteSAkc291cmNlUmV2aXNpb247CiAgICAgbXkgJGVzY2FwZWRGaWxl ID0gZXNjYXBlU3VidmVyc2lvblBhdGgoJGZpbGUpOworICAgIAorICAgIHByaW50IFNUREVSUiAi UGVyZm9ybWluZyBgc3ZuIGluZm8gJyRlc2NhcGVkRmlsZSdgXG4iIGlmICR2ZXJib3NlOworICAg IAogICAgIG9wZW4gSU5GTywgInN2biBpbmZvICckZXNjYXBlZEZpbGUnIHwiIG9yIGRpZTsKICAg ICB3aGlsZSAoPElORk8+KSB7CiAgICAgICAgIGlmICgvXkNvcGllZCBGcm9tIFVSTDogKC4rPylb XHJcbl0qJC8pIHsKQEAgLTIyNywxMSArMjQ1LDE0IEBAIHN1YiBnZW5lcmF0ZURpZmYoJCQpCiAg ICAgbXkgJHBhdGNoID0gIiI7CiAgICAgbXkgJGlzQWRkaXRpb25XaXRoSGlzdG9yeSA9ICRmaWxl RGF0YS0+e21vZGlmaWNhdGlvblR5cGV9IGVxICJhZGRpdGlvbldpdGhIaXN0b3J5IjsKICAgICBp ZiAoJGlzQWRkaXRpb25XaXRoSGlzdG9yeSkgewotICAgICAgICBtYW51ZmFjdHVyZVBhdGNoRm9y QWRkaXRpb25XaXRoSGlzdG9yeSgkZmlsZURhdGEpOworICAgICAgICBtYW51ZmFjdHVyZVBhdGNo Rm9yQWRkaXRpb25XaXRoSGlzdG9yeSgkZmlsZURhdGEsICRwcmVmaXgpOwogICAgIH0KIAogICAg IG15ICRkaWZmT3B0aW9ucyA9IGRpZmZPcHRpb25zRm9yRmlsZSgkZmlsZSk7CiAgICAgbXkgJGVz Y2FwZWRGaWxlID0gZXNjYXBlU3VidmVyc2lvblBhdGgoJGZpbGUpOworCisgICAgcHJpbnQgU1RE RVJSICJQZXJmb3JtaW5nIGBzdm4gZGlmZiAtLWRpZmYtY21kIGRpZmYgLXggLSRkaWZmT3B0aW9u cyAnJGVzY2FwZWRGaWxlJ2BcbiIgaWYgJHZlcmJvc2U7CisKICAgICBvcGVuIERJRkYsICJzdm4g ZGlmZiAtLWRpZmYtY21kIGRpZmYgLXggLSRkaWZmT3B0aW9ucyAnJGVzY2FwZWRGaWxlJyB8IiBv ciBkaWU7CiAgICAgd2hpbGUgKDxESUZGPikgewogICAgICAgICAkcGF0Y2ggLj0gJF87CkBAIC0y NTYsMTAgKzI3NywxNyBAQCBzdWIgZ2VuZXJhdGVGaWxlTGlzdCgkXCUpCiAgICAgbXkgJGVzY2Fw ZWRTdGF0UGF0aCA9IGVzY2FwZVN1YnZlcnNpb25QYXRoKCRzdGF0UGF0aCk7CiAgICAgbXkgQGRl bGV0ZWRGaWxlczsKIAorICAgIHByaW50IFNUREVSUiAiUGVyZm9ybWluZyBgc3ZuIHN0YXQgJyRl c2NhcGVkU3RhdFBhdGgnYFxuIiBpZiAkdmVyYm9zZTsKKwogICAgIG9wZW4gU1RBVCwgInN2biBz dGF0ICckZXNjYXBlZFN0YXRQYXRoJyB8IiBvciBkaWU7CiAgICAgd2hpbGUgKG15ICRsaW5lID0g PFNUQVQ+KSB7CiAgICAgICAgICMgc3ZuIG1heSBvdXRwdXQgYSBkaWZmZXJlbnQgRU9MIHNlcXVl bmNlIHRoYW4gJC8sIHNvIGF2b2lkIGNob21wLgogICAgICAgICAkbGluZSA9fiBzL1tcclxuXSsk Ly9nOworCisgICAgICAgICMgc3ZuIG1heSBvdXRwdXQgZXhwbGFuaXRvcnkgbGluZXMgZGVzY3Jp YmluZyBtb3JlIGRldGFpbCBhYm91dCBhIGZpbGUgY2hhbmdlCisgICAgICAgICMgZS5nICI+IG1v dmVkIHRvIGZvby9iYXIuY3BwIi4gRm9yIG5vdyB3ZSBpZ25vcmUgdGhlc2UgbGluZXMuCisgICAg ICAgIG5leHQgaWYgJGxpbmUgPX4gLyArPi87CisKICAgICAgICAgbXkgJHN0YXQ7CiAgICAgICAg IG15ICRwYXRoOwogICAgICAgICBpZiAoaXNTVk5WZXJzaW9uMTZPck5ld2VyKCkpIHsKQEAgLTMy NywyMiArMzU1LDI4IEBAIHN1YiBpc0JpbmFyeU1pbWVUeXBlKCQpCiAgICAgcmV0dXJuIDE7CiB9 CiAKLXN1YiBtYW51ZmFjdHVyZVBhdGNoRm9yQWRkaXRpb25XaXRoSGlzdG9yeSgkKQorc3ViIG1h bnVmYWN0dXJlUGF0Y2hGb3JBZGRpdGlvbldpdGhIaXN0b3J5KCQkKQogewotICAgIG15ICgkZmls ZURhdGEpID0gQF87Ci0gICAgbXkgJGZpbGUgPSAkZmlsZURhdGEtPntwYXRofTsKKyAgICBteSAo JGZpbGVEYXRhLCAkcHJlZml4KSA9IEBfOworCisgICAgbXkgJGZpbGUgPSBGaWxlOjpTcGVjLT5j YXRkaXIoJHByZWZpeCwgJGZpbGVEYXRhLT57cGF0aH0pOworICAgIG15ICRzb3VyY2VGaWxlID0g RmlsZTo6U3BlYy0+Y2F0ZGlyKCRwcmVmaXgsICRmaWxlRGF0YS0+e3NvdXJjZUZpbGV9KTsKKyAg ICBteSAkc291cmNlUmV2aXNpb24gPSAkZmlsZURhdGEtPntzb3VyY2VSZXZpc2lvbn07CisKICAg ICBwcmludCAiSW5kZXg6ICR7ZmlsZX1cbiI7CiAgICAgcHJpbnQgIj0iIHggNjcsICJcbiI7Ci0g ICAgbXkgJHNvdXJjZUZpbGUgPSAkZmlsZURhdGEtPntzb3VyY2VGaWxlfTsKLSAgICBteSAkc291 cmNlUmV2aXNpb24gPSAkZmlsZURhdGEtPntzb3VyY2VSZXZpc2lvbn07CiAgICAgcHJpbnQgIi0t LSAke2ZpbGV9XHQocmV2aXNpb24gJHtzb3VyY2VSZXZpc2lvbn0pXHQoZnJvbSAke3NvdXJjZUZp bGV9OiR7c291cmNlUmV2aXNpb259KVxuIjsKICAgICBwcmludCAiKysrICR7ZmlsZX1cdCh3b3Jr aW5nIGNvcHkpXG4iOworCiAgICAgaWYgKCRmaWxlRGF0YS0+e2lzQmluYXJ5fSkgewogICAgICAg ICBwcmludCAiXG5DYW5ub3QgZGlzcGxheTogZmlsZSBtYXJrZWQgYXMgYSBiaW5hcnkgdHlwZS5c biI7CiAgICAgICAgIG15ICRtaW1lVHlwZSA9IGZpbmRNaW1lVHlwZSgkZmlsZSwgJHNvdXJjZVJl dmlzaW9uKTsKICAgICAgICAgcHJpbnQgInN2bjptaW1lLXR5cGUgPSAke21pbWVUeXBlfVxuXG4i OwogICAgIH0gZWxzZSB7CiAgICAgICAgIG15ICRlc2NhcGVkU291cmNlRmlsZSA9IGVzY2FwZVN1 YnZlcnNpb25QYXRoKCRzb3VyY2VGaWxlKTsKKyAgICAgICAgCisgICAgICAgIHByaW50IFNUREVS UiAiUGVyZm9ybWluZyBgc3ZuIGRpZmYgLXIgMDoke3NvdXJjZVJldmlzaW9ufSAke2VzY2FwZWRT b3VyY2VGaWxlfSB8IHRhaWwgLW4gKzVgXG4iIGlmICR2ZXJib3NlOworICAgICAgICAKICAgICAg ICAgcHJpbnQgYHN2biBkaWZmIC1yIDA6JHtzb3VyY2VSZXZpc2lvbn0gJHtlc2NhcGVkU291cmNl RmlsZX0gfCB0YWlsIC1uICs1YDsKICAgICB9CiB9Cg==