Closed Bug 1762610 Opened 4 years ago Closed 4 years ago

Use #ifdef for mozilla::IsDevelopmentBuild?

Categories

(Core :: XPCOM, task)

Product:
Core
Component:
XPCOM
Type:
task

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: saschanaz, Unassigned)

Details

mozilla::IsDevelopmentBuild is enabled for all channel and checks the existence of Omnijar, but can we instead implement it with #ifdef instead to make it affect only dev builds? It is only for dev builds, so doing so makes sense. 🤔

Summary: Use #ifdef for mozilla::IsDevelopementBuild? → Use #ifdef for mozilla::IsDevelopmentBuild?

There's no macro for dev build but there is NIGHTLY_BUILD. Reusing it is probably the easiest way to reduce the number of affected installations significantly.

There is developer_options in moz.build, but it depends on the use case.

{Build|Do not build} with more conservative, release engineering-oriented options.

Totally makes sense, thanks! So that would be #ifdef DEVELOPER_OPTIONS.

It's not an ifdef, you'd need to set_define("MOZ_DEVELOPER_OPTIONS", developer_options) or so.

The name IsDevelopmentBuild doesn't not actually reflect what it's after. What it's after is actually IsPackagedBuild. And that's not something that should be ifdef'ed.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID

Does it mean this should be at least be renamed for the renaming? Hmm.

(In reply to Mike Hommey [:glandium] from comment #5)

The name IsDevelopmentBuild doesn't not actually reflect what it's after. What it's after is actually IsPackagedBuild. And that's not something that should be ifdef'ed.

While this is true (I also mentioned whether the function should be renamed in my review comment that triggered this), it is I believe always used as a proxy for a development build.
These might contain symlinks back to the developer repo and it is used to deal with the implications of that.

So, maybe those should be #ifdefed? ... but if everything that calls it is, then perhaps it should be as well?

Flags: needinfo?(mh+mozilla)

With a #ifdef on MOZ_DEVELOPER_OPTIONS, e.g. locally building beta, release or esr will yield a version that doesn't work (because it doesn't define MOZ_DEVELOPER_OPTIONS unless you also --disable-release).

Flags: needinfo?(mh+mozilla)

(In reply to Mike Hommey [:glandium] from comment #8)

With a #ifdef on MOZ_DEVELOPER_OPTIONS, e.g. locally building beta, release or esr will yield a version that doesn't work (because it doesn't define MOZ_DEVELOPER_OPTIONS unless you also --disable-release).

So perhaps MOZ_DEVELOPER_OPTIONS is the wrong thing to use, but is the principle that it should be #ifed correct?

The problem is that the build in dist/bin is always with symlinks, and the build in dist/firefox/dist/Firefox.app is always without. The former is what you get from ./mach build, the latter is what you get from ./mach package (and is derived from the former). So there really is nothing you can #if that could apply to both, since they use the same binaries.

You need to log in before you can comment on or make changes to this bug.