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

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Jan Beich]

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.

Comment 1

[Jan Beich]

Attachment: Limit to Linux or GNU/kFreeBSD

This is enough to unbreak build. Some stuff in config_unix.h still doesn't make sense on BSDs.

Comment 2

[Jan Beich]

Comment on attachment 8692333 [details] [diff] [review]
Limit to Linux or GNU/kFreeBSD

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ce1141a5be5

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

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.

Comment 4

[Jan Beich]

Comment on attachment 8692333 [details] [diff] [review]
Limit to Linux or GNU/kFreeBSD

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 2, m-i soon
[Risks and why]: Can only break build or have no effect
[String/UUID change made/needed]: None

Comment 5

[Jan Beich]

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.

Comment 6

[Jan Beich]

Attachment: Rely more on top-level configure auto-detection

|#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

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

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.

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbb74bfaa24a

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63946a132b19

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/63946a132b19

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/63946a132b19

Comment 12

[Jan Beich]

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

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Jan, could you please confirm that with this fix you can build on FreeBSD and DragonFly using latest Nightly? Thanks!

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

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).

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

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 ?

Comment 16

[Jan Beich]

(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.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Thanks Nick, I can take it off the release management calendar then.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8692886 [details] [diff] [review]
Rely more on top-level configure auto-detection

OK, let's take this in aurora and beta.

Comment 19

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/4509e30ac56b

Comment 20

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

https://hg.mozilla.org/releases/mozilla-beta/rev/9aaa59338c8e

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(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.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/4509e30ac56b