812667 - libsoundtouch fails to build due to missing cpuid.h , if you don't have xcode command line tools

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Daniel Holbert [:dholbert]]

After bug 809766 landed, a number of people reported build issues in libsoundtouch from
> fatal error: 'cpuid.h' file not found"

There's a workaround (bug 809766 comment 12), but ideally it'd be nice if libsoundtouch could handle not having cpuid.h.

In the other places where we #include cpuid.h, we guard it with #ifdef HAVE_CPUID_H, as shown here:
https://mxr.mozilla.org/mozilla-central/source/gfx/2d/Factory.cpp?mark=64-67#64
https://mxr.mozilla.org/mozilla-central/source/xpcom/glue/SSE.cpp?mark=17-20#17

but in libsoundtouch, we don't have that guard -- we just assume that we have it:
> 46     #if defined(__GNUC__)
> 47         // gcc and clang
> 48         #include "cpuid.h"
https://mxr.mozilla.org/mozilla-central/source/media/libsoundtouch/src/cpu_detect_x86.cpp#45

I think we want to edit our libsoundtouch patch for that chunk to have "&& defined(HAVE_CPUID_H)", and let people without cpuid.h fall into the " // If we still don't have the macros, add them" chunk below.

Comment 1

[Daniel Holbert [:dholbert]]

Paul, does my suggestion at the end of comment 0 make sense?

Comment 2

[Paul Adenot (:padenot)]

It does. I'll write a patch for that.

Comment 3

[Daniel Holbert [:dholbert]]

Thanks!

Comment 4

[Paul Adenot (:padenot)]

Attachment: Make soundtouch compile properly if cpuid.h is not present. r=

Comment 5

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a670db3bb816

Comment 6

[Ed Morley [:emorley]]

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

Comment 7

[Kent James (:rkent)]

This fix did not actually work for me. __get_cpuid is defined in cpuid.h, so when cpuid.h is missing this line in cpu_detect_x86.cpp fails to compile:

if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx)) return 0; // always disable extensions.

There are a variety of workarounds to this, but the real problem is expressed in the bug description: "you don't have xcode command line tools"  Installing those solves the problem, and so I'm not really sure this is a code bug but a failure on my part to properly setup my build environment.

But this is new in Moz20, previous versions worked OK with missing cpuid.h

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Kent James (:rkent) from comment #7)
> I'm not really sure this is a
> code bug but a failure on my part to properly setup my build environment.

I'd say it's a code bug (though thankfully there's an easy workaround). IIRC, when this bug was filed, this was the only thing keeping us from building on environments without xcode command line tools.  And I don't think we should impose that requirement just for this one cpu_detect_x86.cpp file. (especially when we can fix it)

I went ahead and spun off a new bug -- bug 836824 -- for comment 7, since it's technically a different issue (different line of code causing the compile error, though very much related ;)), and it's been months since this bug was closed.

Thanks for the heads-up about the issue!