Closed Bug 843387 Opened 11 years ago Closed 11 years ago

Remove unused MOZ_CHROME_FILE_FORMAT_JAR configure variable

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla22

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
MOZ_CHROME_FILE_FORMAT_JAR appears to be unused.  This patch removes the remaining references from the build system.
Attachment #716277 - Flags: review?(ted)
Comment on attachment 716277 [details] [diff] [review]
patch

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

FWIW, that will break that particular test for people building with MOZ_CHROME_FILE_FORMAT=jar (I doubt people do that for firefox, but I wouldn't be surprised if some third-party apps built on top of gecko do). It should however be possible to derive the resource urls from a resolved chrome url, which would give information whether the particular build is using jars or not.

Also note that these #ifdefs are the only reason that file is preprocessed, so you can rename the file and remove the preprocessing rule too.
Comment on attachment 716277 [details] [diff] [review]
patch

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

::: caps/tests/mochitest/test_bug292789.html.in
@@ -40,5 @@
>      var resjs = document.getElementById("resjs");
>      resjs.onload = scriptOnload;
> -#ifdef MOZ_CHROME_FILE_FORMAT_JAR
> -    resjs.src = "jar:resource://gre/chrome/toolkit.jar!/content/mozapps/xpinstall/xpinstallConfirm.js";
> -#else

These appear to be the only #ifdefs in this file, presumably this could stop being preprocessed at this point.
Attachment #716277 - Flags: review?(ted) → review+
(In reply to Mike Hommey [:glandium] from comment #1)
> FWIW, that will break that particular test for people building with
> MOZ_CHROME_FILE_FORMAT=jar (I doubt people do that for firefox, but I
> wouldn't be surprised if some third-party apps built on top of gecko do).

If I understand right, --enable-chrome-format=jar is the default value, so MOZ_CHROME_FILE_FORMAT_JAR is currently defined by default for desktop Firefox builds.  Does this mean that default Firefox builds will break?

However, configure always overwrites MOZ_CHROME_FILE_FORMAT with "flat" or "symlink" after using the original value to set MOZ_PACKAGER_FORMAT, MOZ_CHROME_FILE_FORMAT_JAR, and MOZ_OMNIJAR.  So the "jar" value only affects packaged builds, right?  Does this mean the test is currently broken by default in unpackaged builds?

(I'm still working on understanding this code; thanks for your patience.)
The values in browser/confvars.sh make the default omni, which then becomes flat or symlink. But Firefox is not the only thing around and third party programs may just use the default, which is jar. For them the test would break.
Thanks, now I get it.  I guess if test failures become an issue for third-party apps, we could use MOZ_PACKAGER_FORMAT instead, to either restore the ifdefs or just disable the test.

Removed the preprocessing of the test file, and pushed to Try:
https://tbpl.mozilla.org/?tree=Try&rev=a37da0d8be65
https://hg.mozilla.org/mozilla-central/rev/62f53c3e2f27
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.