Closed Bug 1061684 Opened 6 years ago Closed 5 years ago

Can't install 2.1 simulators on windows

Categories

(Firefox OS Graveyard :: Simulator, defect)

All
Windows 7
defect
Not set

Tracking

(firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S4 (12sep)
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: ochameau, Assigned: gps)

References

Details

Attachments

(1 file, 3 obsolete files)

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\fxos_2_1_simulator@mozilla.org\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...
Blocks: 1053730
Attached patch patch (obsolete) — Splinter Review
Patch to use ZipFile again...

https://tbpl.mozilla.org/?tree=Try&rev=5113643e3ea1
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.
Attachment #8482744 - Flags: review?(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?
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
$ 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.
The forward slash appears to be the standard path separator in zip/JAR
files. Accept back slashes when adding paths to a JAR.
Attachment #8483619 - Flags: review?(mh+mozilla)
Assignee: nobody → gps
Status: NEW → ASSIGNED
Comment on attachment 8482744 [details] [diff] [review]
patch

This is a suboptimal solution.
Attachment #8482744 - Flags: review?(gps) → review-
Thanks! The xpi works fine on windows with your patch.
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.
Attachment #8483619 - Flags: review?(mh+mozilla) → feedback+
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.
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.
Attachment #8482744 - Attachment is obsolete: true
Now using mozpack.path.normsep per earlier review. Adjusted test
accordingly.

glandium is on PTO, so mshal gets review.
Attachment #8485781 - Flags: review?(mshal)
Attachment #8483619 - Attachment is obsolete: true
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 == '\\' ?
The forward slash appears to be the standard path separator in zip/JAR
files. Accept back slashes when adding paths to a JAR.
Attachment #8486071 - Flags: review?(mshal)
Attachment #8486071 - Flags: review?(mshal) → review+
Comment on attachment 8485781 [details] [diff] [review]
Normalize path separators in JAR paths

I assume you meant to mark this one obsolete.
Attachment #8485781 - Flags: review?(mshal) → review-
https://hg.mozilla.org/mozilla-central/rev/cc3e560c8f06
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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
Attachment #8486071 - Flags: approval-mozilla-aurora?
Comment on attachment 8486071 [details] [diff] [review]
Normalize path separators in JAR paths

Build fix. Aurora+
Attachment #8486071 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.