712450 - mozinstall copying files from .dmg incorrectly

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Andrew Halberstadt [:ahal]]

Aki ran into this issue on bug 712100

Basically the contents of Nightly.app are being copied instead of Nightly.app itself. This means when --destination is specified, mozinstall will be looking for the binary in the wrong place.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 - copy files correctly

Makes sure the files are getting copied into the directory that mozinstall expects them to be.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 583304 [details] [diff] [review]
Patch 1.0 - copy files correctly

>-        subprocess.call("cp -r " + os.path.join(appDir, appName) + " " + dest,
>+
>+        dest = os.path.join(dest, appName)
>+        assert not os.path.isfile(dest)
>+        if not os.path.isdir(dest):
>+            os.makedirs(dest)
>+        subprocess.call("cp -r " +
>+                        os.path.join(appDir,appName, "*") + " " + dest,

Why are you using subprocess calls instead of using native Python code? shutil.copytree is doing exactly the same thing.

Comment 3

[Andrew Halberstadt [:ahal]]

Heh, I knew that was going to be brought up. Basically because:
a) We already shell out for 'hdiutil' (any system that doesn't have 'cp' won't have 'hdiutil')
b) Because that's how the code I copied from already was and it was easier

I can change it if it's a big deal, though it would mean hijacking a firebug vm for testing.

Comment 4

[Mark Côté [:mcote]]

Comment on attachment 583304 [details] [diff] [review]
Patch 1.0 - copy files correctly

The code is fine, although I am confused as to why this works and the original doesn't.  The commands just changed from

cp -r /srcpath/FirefoxNightly.app /destpath

to

mkdir /destpath/FirefoxNightly.app
cp -r /srcpath/FirefoxNightly.app/* /destpath/FirefoxNightly.app

Those two things should be equivalent (and appear to be, on my Macbook Pro running Lion).  It might be worth investigating this further, but then again there are bigger bugs to deal with... worth keeping this in mind though.

Comment 5

[Andrew Halberstadt [:ahal]]

FWIW I agree with Mark.. it's just that I know that mozinstall without the above patch doesn't work on one of the test slaves and does work with it. I don't know why, but trying to figure it out isn't very high on my list of priorities atm.

I also agree with Henrik, but I'm somewhat afraid to change anything for fear of breaking it on the test slave again.

Comment 6

[Andrew Halberstadt [:ahal]]

I guess I should say what was happening on the test slave before..

If you ran 'mozinstall -s path/to/src -d path/to/dest' the binary would be at /path/to/dest/Contents/MacOS/firefox-bin instead of /path/to/dest/Nightly.app/Contents/MacOS/firefox-bin. When I wrote this patch I just assumed it was a weird .app Mac-ism.

One theory could be that .app files were treated differently on older versions of OSX but then this was fixed in Lion.

Anyway, master: https://github.com/mozilla/mozbase/commit/802906b62108e0d1e1bff4b4a6468a9d4f1de023

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Andrew Halberstadt [:ahal] from comment #6)
> One theory could be that .app files were treated differently on older
> versions of OSX but then this was fixed in Lion.

We haven't had any problems so far with our code (which also needs an overhaul):
http://hg.mozilla.org/qa/mozmill-automation/file/05e4f5f2cae8/libs/install.py#l97