1061684 - Can't install 2.1 simulators on windows

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Simulator, defect)

Description

[Alexandre Poirot [:ochameau]]

We get the following exception:
Failed to install ...\fxos-simulator-2.1-win32.xpi from file:///.../fxos-simulator-2.1-win32.xpi: Win error 3 during operation open on file ...\AppData\Roaming\Mozilla\Firefox\Profiles\q26ze7sa.nightly\extensions\staged\[e-mail]\b2g\AccessibleMarshal.dll (Le chemin d’accès spécifié est introuvable.)

I would say bug 1056136 introduced some issue with xpi generation while using mozjar.JarWriter...

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: patch

Patch to use ZipFile again...

https://tbpl.mozilla.org/?tree=Try&rev=5113643e3ea1

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 8482744 [details] [diff] [review]
patch

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

JarWriter seems to produce broken .xpi file that Firefox can't install on Windows.
Whereas regular zip python module works just fine (verified with try build).
I'm not sure we care that much about generating idempotent xpi regarding mtimes,
but we care about supporting Windows.

Comment 3

[Gregory Szorc [:gps]]

The build system cares about generating idempotent XPIs. We should make an attempt to use JarWriter to produce XPIs. The proper solution to this bug is to fix JarWriter or this script.

Can someone please provide STR or a busted .xpi for analysis?

Comment 4

[Alexandre Poirot [:ochameau]]

You can get a broken xpi here (without the patch, using jarwriter):
http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/latest-mozilla-central/fxos-simulator-2.1-win32.xpi

and a working one with this patch here:
http://ftp.mozilla.org/pub/mozilla.org/b2g/try-builds/[e-mail]-5113643e3ea1/try-win32_gecko/fxos-simulator-2.1-win32.xpi

Comment 5

[Alexandre Poirot [:ochameau]]

Otherwise if you want to reproduce out of m-c, you need to set a gaia dir in your mozconfig:
export GAIADIR=~/gaia

And build the xpi like that:
./mach build package

Comment 6

[Gregory Szorc [:gps]]

$ unzip -v -l working.xpi | head -n 10
Archive:  working.xpi
 Length   Method    Size  Ratio   Date   Time   CRC-32    Name
--------  ------  ------- -----   ----   ----   ------    ----
       0  Stored        0   0%  08-29-14 13:33  00000000  lib/
    1304  Stored     1304   0%  08-29-14 13:33  913129d3  lib/main.js
    6054  Stored     6054   0%  08-29-14 13:33  b7bcd7b1  lib/simulator-process.js
    3005  Stored     3005   0%  09-02-14 08:42  0d95f186  install.rdf
    1759  Stored     1759   0%  08-29-14 13:33  d519668b  bootstrap.js
     382  Stored      382   0%  09-02-14 08:42  06fe3579  options.xul
    4762  Stored     4762   0%  08-27-14 03:38  0c54865f  icon.png

$ unzip -v -l busted.xpi | head -n 10
Archive:  busted.xpi
 Length   Method    Size  Ratio   Date   Time   CRC-32    Name
--------  ------  ------- -----   ----   ----   ------    ----
    1304  Defl:X      592  55%  01-01-10 00:00  913129d3  lib\main.js
    6054  Defl:X     2233  63%  01-01-10 00:00  b7bcd7b1  lib\simulator-process.js
    3005  Defl:X     1052  65%  01-01-10 00:00  0d95f186  install.rdf
    1759  Defl:X      726  59%  01-01-10 00:00  d519668b  bootstrap.js
     382  Defl:X      217  43%  01-01-10 00:00  06fe3579  options.xul
    4762  Stored     4762   0%  01-01-10 00:00  0c54865f  icon.png
    7858  Stored     7858   0%  01-01-10 00:00  78b5d346  icon64.png

Differences:

* JarWriter is actually compressing file content. Total file size is reduced from 93,268,526 to 60,752,646 bytes.
* We're using different path separators.
* JarWriter excludes directory entries

The issue causing the problem is the slashes. Fix this script to normalize path separators to / and it should "just work." That /might/ be something we should do automatically in JarWriter.

Comment 7

[Gregory Szorc [:gps]]

Attachment: Normalize path separators in JAR paths

The forward slash appears to be the standard path separator in zip/JAR
files. Accept back slashes when adding paths to a JAR.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8482744 [details] [diff] [review]
patch

This is a suboptimal solution.

Comment 9

[Alexandre Poirot [:ochameau]]

Thanks! The xpi works fine on windows with your patch.

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8483619 [details] [diff] [review]
Normalize path separators in JAR paths

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

::: python/mozbuild/mozpack/mozjar.py
@@ +579,5 @@
>          The given data may be a buffer, a file-like instance, a Deflater or a
>          JarFileReader instance. The latter two allow to avoid uncompressing
>          data to recompress it.
>          '''
> +        name = name.replace('\\', '/')

Use mozpath.normsep.

Comment 11

[Gregory Szorc [:gps]]

mozpath.normsep breaks the test because normsep only does stuff if os.pathsep != '/'

I'm not sure if this is a feature or a bug. I didn't want to make the test conditional on platform.

Comment 12

[Mike Hommey [:glandium]]

It's a feature, and you should change the unit test accordingly. There is no reason non-Windows platform should treat \\ as a directory separator.

Comment 13

[Gregory Szorc [:gps]]

Attachment: Normalize path separators in JAR paths

Now using mozpack.path.normsep per earlier review. Adjusted test
accordingly.

glandium is on PTO, so mshal gets review.

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8485781 [details] [diff] [review]
Normalize path separators in JAR paths

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

::: python/mozbuild/mozpack/test/test_mozjar.py
@@ +151,5 @@
>          self.assertEqual(files[2].filename, 'baz/qux')
>          self.assertFalse(files[2].compressed)
>          self.assertEqual(files[2].read(), 'aaaaaaaaaaaaanopqrstuvwxyz')
>  
> +        if sys.platform == 'win32':

if os.sep == '\\' ?

Comment 15

[Gregory Szorc [:gps]]

Attachment: Normalize path separators in JAR paths

The forward slash appears to be the standard path separator in zip/JAR
files. Accept back slashes when adding paths to a JAR.

Comment 16

[Michael Shal [:mshal]]

Comment on attachment 8485781 [details] [diff] [review]
Normalize path separators in JAR paths

I assume you meant to mark this one obsolete.

Comment 17

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cc3e560c8f06

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cc3e560c8f06

Comment 19

[Alexandre Poirot [:ochameau]]

Comment on attachment 8486071 [details] [diff] [review]
Normalize path separators in JAR paths

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
  No fxos simulators on windows for 2.1
[Describe test coverage new/current, TBPL]:
  Comes with specific unit test.
[Risks and why]: 
  This patch has been provided to fix broken xpi generation on windows that regressed on 2.1, but it landed later on 2.2 cycle...
[String/UUID change made/needed]:
  No

Comment 20

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8486071 [details] [diff] [review]
Normalize path separators in JAR paths

Build fix. Aurora+

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4c1c54b7671e