Can't install 2.1 simulators on windows

RESOLVED FIXED in Firefox 34, Firefox OS v2.1

Status

Firefox OS
Simulator
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ochameau, Assigned: gps)

Tracking

unspecified
2.1 S4 (12sep)
All
Windows 7

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

4 years ago
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...
(Reporter)

Updated

4 years ago
Blocks: 1053730
(Reporter)

Comment 2

4 years ago
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)
(Assignee)

Comment 3

4 years ago
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?
(Reporter)

Comment 5

4 years ago
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
(Assignee)

Comment 6

4 years ago
$ 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.
(Assignee)

Comment 7

4 years ago
Created attachment 8483619 [details] [diff] [review]
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.
Attachment #8483619 - Flags: review?(mh+mozilla)
(Assignee)

Updated

4 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
(Assignee)

Comment 8

4 years ago
Comment on attachment 8482744 [details] [diff] [review]
patch

This is a suboptimal solution.
Attachment #8482744 - Flags: review?(gps) → review-
(Reporter)

Comment 9

4 years ago
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+
(Assignee)

Comment 11

4 years ago
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.
(Assignee)

Updated

4 years ago
Attachment #8482744 - Attachment is obsolete: true
(Assignee)

Comment 13

4 years ago
Created attachment 8485781 [details] [diff] [review]
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.
Attachment #8485781 - Flags: review?(mshal)
(Assignee)

Updated

4 years ago
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 == '\\' ?
(Assignee)

Comment 15

4 years ago
Created attachment 8486071 [details] [diff] [review]
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.
Attachment #8486071 - Flags: review?(mshal)

Updated

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 19

4 years ago
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+
Attachment #8485781 - Attachment is obsolete: true
https://hg.mozilla.org/releases/mozilla-aurora/rev/4c1c54b7671e
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
status-firefox33: --- → wontfix
status-firefox34: --- → fixed
status-firefox35: --- → fixed
Target Milestone: --- → 2.1 S4 (12sep)
You need to log in before you can comment on or make changes to this bug.