1399877 - Globally define DLL_PREFIX/DLL_SUFFIX

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[(not currently active) Ted Mielczarek]

We use these defines in a handful of source files in the tree, but each of them sets the `DEFINES` locally in moz.build:
https://dxr.mozilla.org/mozilla-central/search?q=DLL_PREFIX+path%3Amoz.build&redirect=true

We also have a globally-defined `MOZ_DLL_SUFFIX`:
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/moz.configure#223

which is used in all of one source file:
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/xpcom/build/nsXPCOMPrivate.h#84

The locally-set DEFINEs are a problem in the WSL build because the quoting gets lost going from bash to cl.exe. Setting them globally with `set_define` works fine, so we might as well just do that.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: bug 1399877 - Globally define DLL_PREFIX/DLL_SUFFIX.

Several source files use DLL_PREFIX/DLL_SUFFIX defines, and they all
set them in moz.build using `DEFINES`. This is problematic for the WSL build
because the quoting gets lost somewhere between bash and cl.exe. This patch
makes them set globally in moz.configure with `set_define`. There was an
existing global `MOZ_DLL_SUFFIX` define that was only used in one place, so
that has been removed in favor of simply `DLL_SUFFIX`.

Review commit: https://reviewboard.mozilla.org/r/179824/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179824/

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8908141 [details]
bug 1399877 - Globally define DLL_PREFIX/DLL_SUFFIX.

https://reviewboard.mozilla.org/r/179824/#review185568

I wish we didn't have to put this in DEFINES. But that would be follow-up fodder.

Nice cleanup.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9bc6c96c492
Globally define DLL_PREFIX/DLL_SUFFIX. r=gps

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/c28d790ecc91a33afe7b036523813137c9f8a0fb for build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=131383203&repo=autoland

Comment 5

[Bob Clary [:bc] (inactive)]

This also busted autophone's t and tpn tests:
<https://treeherder.mozilla.org/#/jobs?filter-searchStr=autophone&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3&repo=autoland&fromchange=d9bc6c96c492eb23bb08716607f276d1e5ea2f20&tochange=c28d790ecc91a33afe7b036523813137c9f8a0fb>

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Also some of these: https://treeherder.mozilla.org/logviewer.html#?job_id=131384898&repo=autoland

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Actually, looks like all or most of the android tests are timing out.

Comment 8

[Intermittent Failures Robot]

18 failures in 75 pushes (0.24 failures/push) were associated with this bug yesterday.    

Repository breakdown:
* autoland: 18

Platform breakdown:
* android-7-1-armv8-api16: 6
* android-4-4-armv7-api16: 6
* android-4-2-armv7-api16: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1399877&startday=2017-09-16&endday=2017-09-16&tree=all

Comment 9

[Intermittent Failures Robot]

28 failures in 1032 pushes (0.027 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* autoland: 28

Platform breakdown:
* android-7-1-armv8-api16: 9
* android-6-0-armv8-api16: 7
* android-4-4-armv7-api16: 6
* android-4-2-armv7-api16: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1399877&startday=2017-09-11&endday=2017-09-17&tree=all

Comment 10

[(not currently active) Ted Mielczarek]

I think I will rework this patch to use MOZ_DLL_{PREFIX,SUFFIX} everywhere, which should avoid the conflict with the defines used by package-manifest.in.

Comment 11

[Nathan Froyd [:froydnj]]

Attachment: globally define MOZ_DLL_PREFIX/MOZ_DLL_SUFFIX

This version reworks things--I believe--according to the plan Ted mentioned in
comment 10.  I'm going to carry over gps's review from the previous patch, and
assuming try comes back green, land it.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e186d8665b00
globally define MOZ_DLL_PREFIX/MOZ_DLL_SUFFIX; r=gps

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

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