Closed Bug 875342 Opened 7 years ago Closed 7 years ago

TypedArray move and parallel js are enabled on non-official builds that are not intended to have them enabled

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox21 affected, firefox22 affected, firefox23 affected, firefox24 fixed)

RESOLVED FIXED
mozilla24
Tracking Status
firefox21 --- affected
firefox22 --- affected
firefox23 --- affected
firefox24 --- fixed

People

(Reporter: glandium, Assigned: Gavin)

References

Details

Attachments

(1 file, 1 obsolete file)

The Makefile.in snippet is:

  ifeq (,$(filter beta release esr,$(MOZ_UPDATE_CHANNEL)))
  DEFINES += -DENABLE_TYPEDARRAY_MOVE
  endif
  ifeq (,$(filter aurora beta release esr,$(MOZ_UPDATE_CHANNEL)))
  DEFINES += -DENABLE_PARALLEL_JS
  endif

MOZ_UPDATE_CHANNEL is usually default for non-official builds of any branch, which means both are enabled on such builds.

This affects release, where the snippet is
  ifeq (,$(filter beta release esr,$(MOZ_UPDATE_CHANNEL)))
  DEFINES += -DENABLE_TYPEDARRAY_MOVE
  endif

and beta, where it is
  ifeq (,$(filter beta release esr,$(MOZ_UPDATE_CHANNEL)))
  DEFINES += -DENABLE_TYPEDARRAY_MOVE
  DEFINES += -DENABLE_PARALLEL_JS
  endif

On other branches, it's supposed to be enabled, so it's not really a problem.
Isn't it more serious than that, since on Aurora we set the channel on nightly builds and don't on on-push builds, so that sets us up for nightly-only failures, and on beta and release we set the channel on release builds, but not on on-push builds and there are no nightly builds, so that sets us up for totally unseen release failures. 

I'd think that means that the last time we actually ran tests on the disabled state that we ship was when those branches last built a nightly on Aurora.
Depends on: 853071
Note there are many other places where MOZ_UPDATE_CHANNEL is used this way. I guess we have to change most of them, and forbid the use of MOZ_UPDATE_CHANNEL, it's just too much of a footgun.

https://mxr.mozilla.org/mozilla-central/search?string=MOZ_UPDATE_CHANNEL
We should certainly use RELEASE_BUILD/NIGHTLY_BUILD where appropriate, instead of MOZ_UPDATE_CHANNEL.
Attached patch patch (obsolete) — Splinter Review
See https://wiki.mozilla.org/Platform/Channel-specific_build_defines
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #753885 - Flags: review?(sstangl)
Comment on attachment 753885 [details] [diff] [review]
patch

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

Looks right to me.
Attachment #753885 - Flags: review?(sstangl) → review+
Comment on attachment 753885 [details] [diff] [review]
patch

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

::: js/src/builtin/TestingFunctions.cpp
@@ -168,5 @@
>  #endif
>      if (!JS_SetProperty(cx, info, "oom-backtraces", &value))
>          return false;
>  
> -#ifdef ENABLE_PARALLEL_JS

There's value in keeping ENABLE_PARALLEL_JS: you're not going to forget one when you want it to start riding the train.
Attached patch patchSplinter Review
Good point, here's a patch that addresses that.
Attachment #753885 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c7aae5ead16f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
No longer blocks: 878291
Depends on: 878291
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.