Closed Bug 1228230 Opened 8 years ago Closed 8 years ago

media/libav/libavutil/mem.c:34: /usr/include/malloc.h:3:2: error: "<malloc.h> has been replaced by <stdlib.h>"

Categories

(Firefox Build System :: General, defect)

Unspecified
FreeBSD
defect
Not set
normal

Tracking

(firefox42 affected, firefox43 fixed, firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

VERIFIED FIXED
mozilla45
Tracking Status
firefox42 --- affected
firefox43 --- fixed
firefox44 --- fixed
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: jbeich, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

malloc.h is non-POSIX and may either be forbidden (FreeBSD) or missing (DragonFly).

  In file included from /objdir/media/libav/Unified_c_media_libav0.c:119:
  In file included from /mozilla-central/media/libav/libavutil/mem.c:34:
  In file included from ../../dist/system_wrappers/malloc.h:3:
  /usr/include/malloc.h:3:2: error: "<malloc.h> has been replaced by <stdlib.h>"
  #error "<malloc.h> has been replaced by <stdlib.h>"
   ^
  1 error generated.
Attached patch Limit to Linux or GNU/kFreeBSD (obsolete) — Splinter Review
This is enough to unbreak build. Some stuff in config_unix.h still doesn't make sense on BSDs.
OS: Unspecified → FreeBSD
Comment on attachment 8692333 [details] [diff] [review]
Limit to Linux or GNU/kFreeBSD

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

LGTM, and feel free to file other bugs for BSD compat issues if you find them, am happy to review.
Attachment #8692333 - Flags: review?(kyle) → review+
Keywords: checkin-needed
Comment on attachment 8692333 [details] [diff] [review]
Limit to Linux or GNU/kFreeBSD

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

::: media/libav/config_unix.h
@@ +143,5 @@
>  #define HAVE_IO_H 0
>  #define HAVE_MACH_MACH_TIME_H 0
>  #define HAVE_MACHINE_IOCTL_BT848_H 0
>  #define HAVE_MACHINE_IOCTL_METEOR_H 0
> +#if defined(XP_LINUX) || defined(__GLIBC__)

It's actually unnecessary due to HAVE_POSIX_MEMALIGN below. I'll post v2 and also propagate value to config.asm for consistency.
Attachment #8692333 - Flags: approval-mozilla-beta?
Attachment #8692333 - Flags: approval-mozilla-aurora?
Keywords: checkin-needed
|#define HAVE_MALLOC_H 0| breaks Linux due to namespace pollution. Let's deduplicate then.

  media/libav/config_unix.h:147:0: error: "HAVE_MALLOC_H" redefined [-Werror]
  In file included from <command-line>:0:0:
  ./../../../../mozilla-config.h:66:0: note: this is the location of the previous definition
  cc1: all warnings being treated as errors

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7712d76874d2
Attachment #8692886 - Flags: review?(kyle)
Attachment #8692333 - Attachment is obsolete: true
Comment on attachment 8692886 [details] [diff] [review]
Rely more on top-level configure auto-detection

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

LGTM. I'll run it through try just to make sure things work everywhere.
Attachment #8692886 - Flags: review?(kyle) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/63946a132b19
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8692886 [details] [diff] [review]
Rely more on top-level configure auto-detection

Approval Request Comment
[Feature/regressing bug #]: bug 1157768 regression
[User impact if declined]: Broken build on FreeBSD and DragonFly
[Describe test coverage new/current, TreeHerder]: comment 8, m-i, m-c
[Risks and why]: Low. Can only break build.
[String/UUID change made/needed]: None

mozilla-beta Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c59a7bd5e070
Attachment #8692886 - Flags: approval-mozilla-beta?
Attachment #8692886 - Flags: approval-mozilla-aurora?
Jan, could you please confirm that with this fix you can build on FreeBSD and DragonFly using latest Nightly? Thanks!
Flags: needinfo?(jbeich)
I believe we stop taking changes to the build configuration (at least in aurora) earlier this week because of how release engineering creates and stages the beta 1 builds.   nthomas can you help out here and explain? Is that the case or not?  I am not sure if we would then take the change for beta 43 (and uplift it to 44 later on).
Flags: needinfo?(nthomas)
I'm not sure exactly what you're referring to here. Maybe that we used to do staging releases as we got towards the end of the aurora cycle for the upcoming b1 ? That's lapsed for a while so out of play. Completely up to you if you want to take this for beta or not.

Drive-by comment - why do the files for mac and windows need to be be changed to facilitate FreeBSD support ?
Flags: needinfo?(nthomas)
(In reply to Nick Thomas [:nthomas] from comment #15)
> Drive-by comment - why do the files for mac and windows need to be be
> changed to facilitate FreeBSD support ?

Consistency and to reduce overlap -> less hardcoded stuff to maintain manually. media/libav/config*.h and mozilla-config.h cannot have different opinion on their values, see comment 6.

(In reply to Ritu Kothari (:ritu) from comment #13)
> Jan, could you please confirm that with this fix you can build on FreeBSD
> and DragonFly using latest Nightly? Thanks!

Only for FreeBSD: my build went fine after |hg rebase| eliminated the patch. I cannot test DragonFly but it uses the same downstream (via overlay) and has "checking for malloc.h... no" in the log, so should be fine.

https://svnweb.freebsd.org/ports/head/www/firefox/files/patch-media_libav_config__unix.h?revision=401981&view=markup
http://muscles.dragonflybsd.org/bulk/latest-per-pkg/firefox/42.0%2C1/bleeding-edge-potential.log

Note, bug 1228227, bug 1228208 also break build on FreeBSD (and DragonFly). The first train to derail is probably firefox42 circa the time BSD buildbot (provided by :gaston) went on hiatus.
Flags: needinfo?(jbeich)
Thanks Nick, I can take it off the release management calendar then.
Comment on attachment 8692886 [details] [diff] [review]
Rely more on top-level configure auto-detection

OK, let's take this in aurora and beta.
Attachment #8692886 - Flags: approval-mozilla-beta?
Attachment #8692886 - Flags: approval-mozilla-beta+
Attachment #8692886 - Flags: approval-mozilla-aurora?
Attachment #8692886 - Flags: approval-mozilla-aurora+
(In reply to Jan Beich from comment #16)
> (In reply to Ritu Kothari (:ritu) from comment #13)
> > Jan, could you please confirm that with this fix you can build on FreeBSD
> > and DragonFly using latest Nightly? Thanks!
> 
> Only for FreeBSD: my build went fine after |hg rebase| eliminated the patch.
> I cannot test DragonFly but it uses the same downstream (via overlay) and
> has "checking for malloc.h... no" in the log, so should be fine.
> 

Thanks! Updating status to Verified.
Status: RESOLVED → VERIFIED
Blocks: 1239550
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.