Closed Bug 1234353 Opened 8 years ago Closed 8 years ago

[mozcrash] Notify the user if the stackwalk binary is not executable

Categories

(Testing :: Mozbase, defect)

defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: armenzg, Assigned: armenzg)

References

Details

Attachments

(1 file)

16:38:44     INFO -  mozcrash INFO | Downloading symbols from: https://queue.taskcluster.net/v1/task/e3NeMV5ZT3iUrNfEgbrbug/artifacts/public/build/target.crashreporter-symbols.zip
16:38:51     INFO -  Traceback (most recent call last):
16:38:51     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 738, in <module>
16:38:51     INFO -      main()
16:38:51     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 734, in main
16:38:51     INFO -      sys.exit(reftest.runTests(options.tests, options))
16:38:51     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 404, in runTests
16:38:51     INFO -      return self.runSerialTests(manifests, options, cmdlineArgs)
16:38:51     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 684, in runSerialTests
16:38:51     INFO -      debuggerInfo=debuggerInfo)
16:38:51     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 652, in runApp
16:38:51     INFO -      symbolsPath, test_name=self.lastTestSeen)
16:38:51     INFO -    File "/home/worker/workspace/build/venv/local/lib/python2.7/site-packages/mozcrash/mozcrash.py", line 93, in check_for_crashes
16:38:51     INFO -      for info in crash_info:
16:38:51     INFO -    File "/home/worker/workspace/build/venv/local/lib/python2.7/site-packages/mozcrash/mozcrash.py", line 202, in __iter__
16:38:51     INFO -      rv = self._process_dump_file(path, extra)
16:38:51     INFO -    File "/home/worker/workspace/build/venv/local/lib/python2.7/site-packages/mozcrash/mozcrash.py", line 237, in _process_dump_file
16:38:51     INFO -      stderr=subprocess.PIPE)
16:38:51     INFO -    File "/usr/lib/python2.7/subprocess.py", line 679, in __init__
16:38:52     INFO -      errread, errwrite)
16:38:52     INFO -    File "/usr/lib/python2.7/subprocess.py", line 1249, in _execute_child
16:38:52     INFO -      raise child_exception
16:38:52     INFO -  OSError: [Errno 13] Permission denied
Blocks: 1233716
Comment on attachment 8700768 [details]
MozReview Request: Bug 1234353 - Notify the user if the stackwalk binary is not executable. r=chmanchester

https://reviewboard.mozilla.org/r/28787/#review25601

::: testing/mozbase/mozcrash/mozcrash/mozcrash.py:234
(Diff revision 1)
> +            os.access(self.stackwalk_binary, os.X_OK)):
> +            command = [

This makes me a little nervous, can we instead try: except OSError:, then do some checks and try to output a diagnostic when something goes wrong?

::: testing/mozbase/mozcrash/mozcrash/mozcrash.py:240
(Diff revision 1)
> +            print 'Copy/paste: ' + ' '.join(command)

We've converted this file from using print to mozlog, I think.

::: testing/mozbase/mozcrash/mozcrash/mozcrash.py:274
(Diff revision 1)
> +            elif os.access(self.stackwalk_binary, os.X_OK):

This condition looks inverted (we want to warn when the path isn't executable).
Attachment #8700768 - Flags: review?(cmanchester)
Comment on attachment 8700768 [details]
MozReview Request: Bug 1234353 - Notify the user if the stackwalk binary is not executable. r=chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28787/diff/1-2/
Attachment #8700768 - Flags: review?(cmanchester)
Attachment #8700768 - Flags: review?(cmanchester) → review+
Comment on attachment 8700768 [details]
MozReview Request: Bug 1234353 - Notify the user if the stackwalk binary is not executable. r=chmanchester

https://reviewboard.mozilla.org/r/28787/#review25661

::: testing/mozbase/mozcrash/mozcrash/mozcrash.py:233
(Diff revision 2)
> -            os.path.exists(self.stackwalk_binary)):
> +            os.path.exists(self.stackwalk_binary) and

My issue here wasn't addressed, but looking at it again it's probably fine if this has had some testing.
https://reviewboard.mozilla.org/r/28787/#review25601

> This makes me a little nervous, can we instead try: except OSError:, then do some checks and try to output a diagnostic when something goes wrong?

What kind of diagnostic are you thinking of?
Comment on attachment 8700768 [details]
MozReview Request: Bug 1234353 - Notify the user if the stackwalk binary is not executable. r=chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28787/diff/2-3/
Is this better aligned with what you have in mind?
https://reviewboard.mozilla.org/r/28787/#review25661

> My issue here wasn't addressed, but looking at it again it's probably fine if this has had some testing.

Did this get addressed on my latest patch?
https://reviewboard.mozilla.org/r/28787/#review25661

> Did this get addressed on my latest patch?

Mostly (although I'd make the area under the try: smaller), but let's just take version 2. My concern was that os.access would be unreliable on some platforms, but I see it's used all over the tree.
Comment on attachment 8700768 [details]
MozReview Request: Bug 1234353 - Notify the user if the stackwalk binary is not executable. r=chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28787/diff/3-4/
https://reviewboard.mozilla.org/r/28787/#review25661

> Mostly (although I'd make the area under the try: smaller), but let's just take version 2. My concern was that os.access would be unreliable on some platforms, but I see it's used all over the tree.

Dropped since we're going with version 2.
I've gone back to version 2.

I want to see a try run before landing it.
https://hg.mozilla.org/integration/mozilla-inbound/rev/63873e6945e52ead88b2564f032347cdf2bfa04b
Bug 1234353 - Notify the user if the stackwalk binary is not executable. r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/63873e6945e5
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: