Closed Bug 1218554 Opened 4 years ago Closed 2 years ago

[mozinstall] Failure during install on OS X: "local variable 'appDir' referenced before assignment"

Categories

(Testing :: Mozbase, defect)

All
macOS
defect
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: whimboo, Assigned: vedantc98, Mentored)

Details

(Whiteboard: [lang=py])

Attachments

(1 file)

Sometimes I can see the following exception in our firefox-ui-tests on OS X:

07:27:43 Traceback (most recent call last):
07:27:43   File "runtests.py", line 236, in <module>
07:27:43     main()
07:27:43   File "runtests.py", line 231, in main
07:27:43     runner.run_tests(options, args)
07:27:43   File "runtests.py", line 67, in run_tests
07:27:43     install_path = mozinstall.install(installer_path, 'firefox')
07:27:43   File "/Users/mozauto/jenkins/workspace/mozilla-central_update/venv/lib/python2.7/site-packages/mozinstall/mozinstall.py", line 117, in install
07:27:43     install_dir = _install_dmg(src, dest)
07:27:43   File "/Users/mozauto/jenkins/workspace/mozilla-central_update/venv/lib/python2.7/site-packages/mozinstall/mozinstall.py", line 261, in _install_dmg
07:27:43     subprocess.call('hdiutil detach %s -quiet' % appDir,
07:27:43 mozinstall.mozinstall.InstallError: Failed to install "/Users/mozauto/jenkins/workspace/mozilla-central_update/firefox-44.0a1.en-US.mac.dmg (local variable 'appDir' referenced before assignment)"

It might be that we have a problem in attaching the DMG and are failing in line 249:
http://mxr.mozilla.org/comm-central/source/mozilla/testing/mozbase/mozinstall/mozinstall/mozinstall.py#249

Then the code in finally is directly executed and fails because appDir has not been defined yet.

I'm up for mentoring this bug if someone wants to take it.
I'm new to the open source community. I would love to take this as my first bug. How should I start? So far I have cloned the repository from GitHub and set up the environment.
Hi Kent. Good to hear that and that you already cloned the repository. Do you use cinnebar (https://github.com/glandium/git-cinnabar)? If not you will have to use Mercurial and clone http://hg.mozilla.org/mozilla-central/ which is the canonical repository at the moment.

Basically you should have a look at https://github.com/mozilla/ateam-bootcamp first.

I have just seen that the path I gave above is wrong. As pointed out above it's mozilla-central which needs the fix: http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozinstall/mozinstall/mozinstall.py#248

When you have done the changes you should check that nothing is broken by running the mozbase unit tests via: http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/test.py

Let me know if you need further information. For a direct conversation you can also join Mozilla IRC (irc.mozilla.org). You can find me in the #ateam and #automation channel. Also I will assign this bug to you once you have made some progress. I hope that is ok.
OS: Unspecified → Mac OS X
Hardware: Unspecified → All
Thanks a lot. It's very helpful. I was actually having trouble finding the test.py yesterday. So far I have read the webdev bootcamp and the ateam bootcamp. I'm going to dive into it and give it a try now. I will keep you posted.
Hey, I want to check back if you are still working on this bug. If you have problems with something please let me know. If it's too hard we may be able to find something else for you. Thanks.
Flags: needinfo?(kentliang)
Hi Henrik! I would like to work on this.
Hi Konstantin, feel free to get started on the bug. It's all yours. :)

I checked the above links and meanwhile MXR is no longer available. So here the updated URLs:

https://dxr.mozilla.org/mozilla-central/rev/f9362554866b327700c7f9b18050d7b7eb3d2b23/testing/mozbase/mozinstall/mozinstall/mozinstall.py#248

To run the mozbase tests please check:
https://wiki.mozilla.org/Auto-tools/Projects/Mozbase#Running_the_tests

Let me know if you have further questions. Thanks.
Flags: needinfo?(kentliang)
Hi! I install prerequisites and environment for build and run test(bug 1218554). im stuck with xcode compiler version.
trace
 ./mach build 

0:09.15 DEBUG: ENDIANNESS = little
0:09.15 ERROR: Only clang/llvm 3.6 or newer is supported.
0:09.18 *** Fix above errors and then restart with               "/Appli
cations/Xcode.app/Contents/Developer/usr/bin/make -f client.mk build"

on my mac xcode 6.2 and os x mavericks

LLVM 3.6 in version xcode 6.3 and later. maybe possible resolve this trouble with reconfigure build environment?
Did you run "./mach bootstrap" at all? If you haven't done this yet, please do so.
Hi Henrik, if nobody is working on this, can I take it up?

Basically here if the DMG is failing to attach, what do we want to do about it? Throw an exception and display it? Since it doesn't make sense to execute the finally block(it detaches the dmg, which might not have been attached).
Flags: needinfo?(hskupin)
Totally! Feel free to work on it if you are on MacOS.

In case of a failure we have to throw an exception, yes. But we also have to make sure that whenever a failure occurs inside the try/except that an already mounted image gets cleaned-up. Please see that there is also an InstallError raised, in case when the destination folder already exists.
Flags: needinfo?(hskupin)
Vedant, I would like to ask if you are still interested on this bug. We actually haven't heard from each other for a while.
Flags: needinfo?(vedantc98)
Hey Henrik, I'm sorry I've been a bit busy with college.
I would still like to work on this bug, and could probably get to work on it in a day or two, but in case somebody else wants to work on it, that would be okay.
Flags: needinfo?(vedantc98)
Comment on attachment 8921835 [details]
Bug 1218554 - Handled the error while attaching the DMG(macOS).

https://reviewboard.mozilla.org/r/192866/#review198296

::: testing/mozbase/mozinstall/mozinstall/mozinstall.py
(Diff revision 1)
> -        if os.path.exists(dest):
> +    if os.path.exists(dest):
> -            raise InstallError('App bundle "%s" already exists.' % dest)
> +        raise InstallError('App bundle "%s" already exists.' % dest)
>  
> -        shutil.copytree(mounted_path, dest, False)
> +    shutil.copytree(mounted_path, dest, False)
>  
> -    finally:

You cannot remove `finally` here because if something raises an exception between the lines 288 to 308, the already attached DMG image will not be unmounted.

It means you have to definetly keep the try/finally block as is.
Attachment #8921835 - Flags: review?(hskupin) → review-
Please note that you might wanna check that the detach operation doesn't fail in case of the failure happened during mounting the DMG. If the detach command doesn't result in an error, we should be able to leave the finally block as is.
Assignee: nobody → vedantc98
Status: NEW → ASSIGNED
In case the mounting fails, will we still have to detach in the first place?
Or does the partially mounted image still exist at that directory location?
As I said, as long as the detach command does not fail in such a situation, we should just try it.
Maybe we could use the subprocess Popen object that is returned instead of the try-finally block?
There's a member variable called returncode which can be used to check if the process returned a successful execution code or not.
I wouldn't make it that complicated. Lets find a good solution to get the issue as filed fixed.

So I had a look at the code and in case of an issue we would pass `None` to hdiutil, which indeed will not work. So here my proposal:

1) Move the declaration of appdir out of the try block
2) Only run `hdiutil detach` if appdir is not None
Comment on attachment 8921835 [details]
Bug 1218554 - Handled the error while attaching the DMG(macOS).

https://reviewboard.mozilla.org/r/192866/#review200466
Attachment #8921835 - Flags: review?(hskupin) → review+
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eba0d9962ef4
Handled the error while attaching the DMG(macOS). r=whimboo
https://hg.mozilla.org/mozilla-central/rev/eba0d9962ef4
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.