Thanks for the quick fix, Andreas! Is there any way to alert the user to issue 2? Maybe if you run the fixer from the interactive style checker and nothing changes we can report that to the user?
I'll look into making a backup and hopefully post a patch on RB soon. Cheers, Jason On Fri, Jul 1, 2016 at 10:19 AM Andreas Sandberg <[email protected]> wrote: > Hi Jason, > > I have committed a fix for 1 and the issue below. > > 2 is a bit trickier. The main problem is that you don’t always have enough > context when running the include checker since it’s restricted to the lines > affected by the commit. IIRC, there are a couple of cases where it sorts > the includes and when it compares the new file and old file, it can’t find > any differences to the lines in the commit. In that case, the style hook > won’t complain, but running the checker manually would. > > I think it would make sense to create a backup whenever we decide we need > to modify a source file. This is less of an issue in git since the index is > never modified (i.e., the style checker never stages its modifications). > Unfortunately, I don’t think I’ll find time to implement this anytime soon, > especially since it is unlikely to affect our internal flow. > > Regards, > Andreas > > On 01/07/2016, 15:40, "Jason Lowe-Power" <[email protected]> wrote: > > Another error that I just ran into. I accidentally used tabs instead of > spaces and got this error. > > chinook:gem5>hg qref >> invalid whitespace in /afs/ >> cs.wisc.edu/p/multifacet/users/powerjg/gem5/configs/common/HMC.py:281 >> invalid whitespace in /afs/ >> cs.wisc.edu/p/multifacet/users/powerjg/gem5/configs/common/HMC.py:288 >> invalid whitespace in /afs/ >> cs.wisc.edu/p/multifacet/users/powerjg/gem5/configs/common/HMC.py:292 >> invalid whitespace in /afs/ >> cs.wisc.edu/p/multifacet/users/powerjg/gem5/configs/common/HMC.py:296 >> invalid whitespace in /afs/ >> cs.wisc.edu/p/multifacet/users/powerjg/gem5/configs/common/HMC.py:300 >> invalid whitespace in /afs/ >> cs.wisc.edu/p/multifacet/users/powerjg/gem5/configs/common/HMC.py:311 >> (a)bort, (i)gnore, or (f)ix? f >> Traceback (most recent call last): >> File "/u/p/o/powerjg/multifacet/gem5/util/hgstyle.py", line 182, in >> check_style >> return do_check_style(ui, repo, **args) >> File "/u/p/o/powerjg/multifacet/gem5/util/hgstyle.py", line 165, in >> do_check_style >> if verifier.apply(joinpath(repo.root, fname), mod_regions): >> File "/u/p/o/powerjg/multifacet/gem5/util/style/verifiers.py", line >> 154, in apply >> self.fix(filename, regions) >> File "/u/p/o/powerjg/multifacet/gem5/util/style/verifiers.py", line >> 219, in fix >> line = self.fix_line(line, language=lang) >> File "/u/p/o/powerjg/multifacet/gem5/util/style/verifiers.py", line >> 275, in fix_line >> newline += ' ' * (tabsize - len(newline) % tabsize) >> NameError: global name 'tabsize' is not defined >> warning: pre-qrefresh.style hook failed > > > After this error the file from line 282 onwards is deleted. Luckily I > didn't make too many changes before losing my work this time! > > Thanks, > Jason > > > On Fri, Jul 1, 2016 at 9:22 AM, Jason Lowe-Power <[email protected]> > wrote: > >> Hi Andreas and everyone, >> >> I've been having some issues with the style scripts ever since Andreas >> updated them a while back. The scary part about these problems is that >> sometimes they end up with deleting all of the text from the offending >> file! I haven't lost any work, yet, but I've come close a couple of times, >> being saved only by my text editor's buffer. >> >> Here are a few examples of the issues: >> 1) I applied http://reviews.gem5.org/r/3132/ to a clean version of gem5 >> and ran "hg qpush" which resulted in the following: >> >> chinook:gem5>hg qpush >>> applying systemc_stats.patch >>> invalid spacing after if/while/for in /afs/ >>> cs.wisc.edu/p/multifacet/users/powerjg/gem5/util/systemc/stats.cc:67 >>> invalid spacing after if/while/for in /afs/ >>> cs.wisc.edu/p/multifacet/users/powerjg/gem5/util/systemc/stats.cc:117 >>> invalid spacing after if/while/for in /afs/ >>> cs.wisc.edu/p/multifacet/users/powerjg/gem5/util/systemc/stats.cc:150 >>> (a)bort, (i)gnore, or (f)ix? f >>> Traceback (most recent call last): >>> File "/u/p/o/powerjg/multifacet/gem5/util/hgstyle.py", line 182, in >>> check_style >>> return do_check_style(ui, repo, **args) >>> File "/u/p/o/powerjg/multifacet/gem5/util/hgstyle.py", line 165, in >>> do_check_style >>> if verifier.apply(joinpath(repo.root, fname), mod_regions): >>> File "/u/p/o/powerjg/multifacet/gem5/util/style/verifiers.py", line >>> 154, in apply >>> self.fix(filename, regions) >>> File "/u/p/o/powerjg/multifacet/gem5/util/style/verifiers.py", line >>> 219, in fix >>> line = self.fix_line(line, language=lang) >>> File "/u/p/o/powerjg/multifacet/gem5/util/style/verifiers.py", line >>> 352, in fix_line >>> new_line = _any_control.sub(r'\1 (', line) >>> NameError: global name '_any_control' is not defined >>> transaction abort! >>> rollback completed >>> done >>> >>> >>> abort: pretxncommit.style hook failed >> >> >> I think this always happens when there is an if() without a space, but I >> haven't spent enough time looking into it to be sure. >> >> 2) The style fixer and the style checker don't agree on how to order >> header files. I think the style checker wants the header file for the .cc >> file to be the first header, but the style fixer does not do this. I could >> be wrong about this issue, though. I've just noticed that I can run the >> style checker, tell it to fix the problem, and then re-run it and it >> complains again. >> >> 3) When there is a failure in while trying to fix a style error the >> original text should be saved! At a minimum, before starting to modify the >> file we should make a backup, but it would be even better if we restored >> the original file on an error. >> >> Andreas, do you have a quick fix for these issues? I could probably spend >> a few hours trying to track the problems down, but would rather not spend >> my time on it if you're able to do it quickly. >> >> Thanks, >> Jason >> > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
