Bug 38186 – REGRESSION(r58261): webkit-patch upload does not work in an SVN checkout.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Maciej Stachowiak]

webkit-patch upload does not work in an SVN checkout. This is a regression from r58261. It gives this error:

$ webkit-patch upload
Parsing ChangeLog: /Users/mjs/Work/src/Safari/OpenSource/WebCore/ChangeLog
Traceback (most recent call last):
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkit-patch", line 56, in <module>
    main()
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkit-patch", line 51, in main
    WebKitPatch(__file__).main()
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py", line 302, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py", line 113, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 43, in execute
    self._sequence.run_and_handle_errors(tool, options, self._prepare_state(options, args, tool))
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py", line 66, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py", line 60, in _run
    step(tool, options).run(state)
  File "/Users/mjs/Work/src/Safari/OpenSource/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py", line 56, in run
    if self._tool.scm().should_squash(self._options.squash):
AttributeError: SVN instance has no attribute 'should_squash'

[Adam Barth]

Created attachment 54400 [details]
Patch

[WebKit Review Bot]

Attachment 54400 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/Scripts/webkitpy/common/checkout/scm.py:215:  deprecated form of raising exception  [pep8/W602] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Maciej Stachowiak]

Comment on attachment 54400 [details]
Patch

r=me

You may want to fix the style complaints before committing.

[Adam Barth]

Committed r58297: <http://trac.webkit.org/changeset/58297>

[Adam Barth]

We use this style for "NotImplemented" exceptions because Eric said it integrates with some python tool.  I'm not sure if that's something we still care about now that we've adopted PEP8.

[Eric Seidel (no email)]

The tool is pylint.

But we don't have to follow it to the letter. WE just have to throw NotImplemented.  But we could throw the more supported raise NotImplemented("foo") i think.

[Eric Seidel (no email)]

raise NotImplementedError("foo") rather.

[Eric Seidel (no email)]

We could test this using something like:
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py#L159
which just called SVN.should_squash to validate it returned false.