Hi Peter, Sorry for the delay, it was more complicated than I had imagined :-o
On 28 Sep 2012, at 11:08, "Gary V. Vaughan" <g...@gnu.org> wrote: > On 27 ก.ย. 2012, at 5:03, Peter Rosin <p...@lysator.liu.se> wrote: >> On 2012-09-26 14:57, Peter Rosin wrote: >>> On 2012-09-22 05:31, Gary V. Vaughan wrote: >> >> [Heavily snipped] >> >>> Why not commit the sc_prohibit_test_const_follows_var improvement >>> last, after fixing all violations? >> >> That doesn't make sense, sorry. But the idea would have worked >> initially, before the check first existed. I.e., write the check, >> fixup violations and commit in smaller digestible chunks, then >> finally commit the actual check preventing any new violations from >> creeping in. > > Even then, I'm on the fence, since that doesn't leave a revision to > demonstrate that the syntax-check is finding the violations... on the > other hand, we also don't want to push "broken" commits since that > obviously screws up git bisection. On reflection, I think I prefer to continue to commit a new syntax check rule along with all the fixes necessary to make it pass. Although, I'm horrified by how much unrelated crap crept into 962aa91, and will not be tackling any patches even close to this size by hand in future :( >>>> If not, I will branch before 962aa91, run the script, and then apply >>>> the rest of master to that branch one patch at a time until I arrive >>>> at a diff that I can apply to master itself, rather than using revert >>>> as I did on the current temporary branch. >>> >>> I have to say, given all these difficulties, is it really worth it? >>> Besides, I think >> >> Hmmm, I said that in the wrong context. Your plan above seems like the >> best path forward. I guess I was too "excited" about the bugs and didn't >> read properly, sorry again about my poor communication skills. > > No apologies necessary. I'm working on the revised redo, which will > result in a much smaller patch for master later today or tomorrow. It took me longer than anticipated due to being a lot more work after untangling all those unrelated parts that I've split out on the branch. The new branch gary/reredo-test-operand-order (notice the double re) has everything broken down into digestible chunks. All the differences between that and master now look like improvements upon the original hand rolled version made by my recent scripted revisions, or else outright errors in the original corrected by the script. All the errors you flagged on the list are corrected, as well as several others. Assuming my running 'make dist' doesn't have any regressions compared to master, and unless you have additional problems with Windows using the gary/reredo-test-operand-order branch, I plan to merge the differences back into master in a day or two. >> Some of the noise between master and redo-test-operand-order are a bunch >> of hunks with this pattern: >> - if test bar $op $foo; then >> + if test bar $op $foo ; then > > Nice catch. I already tweaked the script regexs to fix that once, but must > have missed one of the ways for that to happen :( The script was correctly keeping semi-colons with the same spacing as was present prior to running the script. The differences are because I fixed the errant spacing manually in the original patch. I'll write another syntax-check, and fix any remaining offenders after master is fully up to date again. >> BTW, here is another strange-looking hunk from >> "git diff master redo-test-operand-order" >> >> diff --git a/m4/libtool.m4 b/m4/libtool.m4 >> index 4413a6d..8ec9beb 100644 >> --- a/m4/libtool.m4 >> +++ b/m4/libtool.m4 >> @@ -5563,7 +5563,7 @@ _LT_EOF >> ;; >> esac >> >> - if test sni = "$host_vendor"; then >> + if test x$host_vendor = xsni; then >> case $host in >> sysv4 | sysv4.2uw2* | sysv4.3* | sysv5*) >> _LT_TAGVAR(export_dynamic_flag_spec, $1)='$wl-Blargedynsym' >> >> What has caused the difference in this hunk? Why hasn't the script >> caught this instance? And why isn't the syntax-check triggered? >> Are the "missing" quotes the key here? > > Not sure... investigating now. The lack of quotes on both sides was not picked up by my script nor by the syntax check, although I made the manual changes in the original patch. Once the branches are back in sync, I'll beef up the syntax check and fix any new violations it finds. Thanks again for keeping me honest with this one. Please let me know if you'd like me to wait more than a day or two to give you time to review. Cheers, -- Gary V. Vaughan (gary AT gnu DOT org)
