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]<mailto:[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<http://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<http://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<http://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<http://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<http://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<http://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]<mailto:[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
<http://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
<http://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
<http://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

Reply via email to