920637 - Nightly OS X xulrunner builds broken (OSError: [Errno 2] No such file or directory: '../../dist/universal/xulrunner/XUL.framework/Versions/Current')

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P1)

Description

[Gregory Szorc [:gps]]

https://tbpl.mozilla.org/php/getParsedLog.php?id=28354687&tree=Mozilla-Central

Traceback (most recent call last):
  File "/builds/slave/m-cen-osx64-xr-ntly-0000000000/build/toolkit/mozapps/installer/packager.py", line 375, in <module>
    main()
  File "/builds/slave/m-cen-osx64-xr-ntly-0000000000/build/toolkit/mozapps/installer/packager.py", line 369, in main
    copier.copy(args.destination)
  File "/builds/slave/m-cen-osx64-xr-ntly-0000000000/build/python/mozbuild/mozpack/copier.py", line 212, in copy
    os.mkdir(d)
OSError: [Errno 2] No such file or directory: '../../dist/universal/xulrunner/XUL.framework/Versions/Current'
make[3]: *** [stage-package] Error 1
make[2]: *** [postflight_all] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2

This is fallout from bug 918392 changing how directories are created. The code now assumes the parent directory exists.

Comment 1

[Phil Ringnalda (:philor)]

I've been assuming that the Android single-locale repacks like https://tbpl.mozilla.org/php/getParsedLog.php?id=28561264&tree=Mozilla-Central are busted from the same thing, am I actually right?

Comment 2

[Mike Hommey [:glandium]]

(In reply to Phil Ringnalda (:philor) from comment #1)
> I've been assuming that the Android single-locale repacks like
> https://tbpl.mozilla.org/php/getParsedLog.php?id=28561264&tree=Mozilla-
> Central are busted from the same thing, am I actually right?

It's likely not the same thing.

Comment 3

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #2)
> It's likely not the same thing.

Actually, it probably is. Too early to think straight.

Comment 4

[Gregory Szorc [:gps]]

Attachment: Ensure destination directory exists in packager

FileCopier now requires the caller to create the destination directory.

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 813270 [details] [diff] [review]
Ensure destination directory exists in packager

Looks good, though I wonder if we will need to move the directory creation to before the '# shlibsign libraries' block since args.destination is referenced there:

                copier.add(libbase + '.chk',
                           LibSignFile(os.path.join(args.destination,
                                                    libname)))

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 813270 [details] [diff] [review]
Ensure destination directory exists in packager

Review of attachment 813270 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/installer/packager.py
@@ +370,5 @@
> +    try:
> +        os.makedirs(args.destination)
> +    except OSError as e:
> +        if e.errno != errno.EEXIST:
> +            raise

Why not do it in the copier?

Comment 7

[Gregory Szorc [:gps]]

Attachment: FileCopier should create destination directory if it doesn't exist

Yeah, doing it in FileCopier is better. I thought there was some API
change that said the caller was responsible. I must be thinking of an
earlier revision or something else.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 813314 [details] [diff] [review]
FileCopier should create destination directory if it doesn't exist

Review of attachment 813314 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozpack/copier.py
@@ +169,5 @@
>          have_symlinks = hasattr(os, 'symlink')
>          destination = os.path.normpath(destination)
>  
> +        try:
> +            os.makedirs(destination)

I'd rather you do that in the except for the mkdir further below.

Comment 9

[Gregory Szorc [:gps]]

Attachment: FileCopier should create destination directory if it doesn't exist

The mkdir() loop below is actually quite scary because it removes
symlinks and adjusts file permissions. Performing these actions parents
of destination is scary and would complicate that loop's logic. I think
this implementation is cleanest. I've added a comment to this effect.

Comment 10

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/74c262c73ebb

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/74c262c73ebb

Comment 12

[Phil Ringnalda (:philor)]

Yeah, turns out that didn't fix the Android l10n repacks, bug 923950.