1162845 - Move ASFLAGS to moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Move ASFLAGS to moz.build

Note the removal of -I$(DIST)/include in xpcom/reflect/... which shouldn't matter because that's already in LOCAL_INCLUDES, which is passed to the assembler invocations for .S files, which are those that need preprocessing. Although, two of the .s files for SunOS do have a #include, but it's for a file that's in the current directory, not $(DIST)/include.

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8603172 [details] [diff] [review]
Move ASFLAGS to moz.build

Err, subtle problem, some Makefiles are not doing +=, but =... and the build breaks for the latter ones.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Move ASFLAGS to moz.build

Comment 4

[Michael Shal [:mshal]]

Comment on attachment 8603213 [details] [diff] [review]
Move ASFLAGS to moz.build

>diff --git a/media/libtheora/moz.build b/media/libtheora/moz.build
>index 30ac65f..031a5ee 100644
>--- a/media/libtheora/moz.build
>+++ b/media/libtheora/moz.build
>+        # These flags are a lie; they're just used to enable the requisite
>+        # opcodes; actual arch detection is done at runtime.
>+        ASFLAGS += [
>+            '-march=armv7-a',
>+            '-mfpu=neon',
>+        ]

I think these ASFLAGS should be close to the GENERATED_SOURCES in this moz.build file, since those are the asm files. We could probably just move GENERATED_SOURCES down here where you added ASFLAGS, since it looks like those conditions are redundant. Eg:

from:
if CONFIG['CPU_ARCH'] == 'arm' and CONFIG['GNU_AS']:
    GENERATED_FILES ...

if CONFIG['GNU_AS']:
    if 'arm' in CONFIG['OS_TEST']:
        ASFLAGS ...

to:
if CONFIG['GNU_AS']:
    if 'arm' in CONFIG['OS_TEST']:
        GENERATED_FILES ...
        ASFLAGS ...

>diff --git a/media/libvpx/moz.build b/media/libvpx/moz.build
>+ASFLAGS += [
>+    '-I.',
>+    '-I%s/media/libvpx/' % TOPSRCDIR,
>+    '-I%s/media/libvpx/vpx_ports/' % TOPSRCDIR,
>+]
>+    ASFLAGS += [
>+        '-D__ANDROID__'
>+    ]

Can we just use LOCAL_INCLUDES / DEFINES instead of -I / -D? Or would that break the .c files?

Everything else looks fine as far as I can tell. I'm glad this moves the responsibility of whether or not to pass in -c out of the Makefile/moz.build...

Comment 5

[Mike Hommey [:glandium]]

(In reply to Michael Shal [:mshal] from comment #4)
> Can we just use LOCAL_INCLUDES / DEFINES instead of -I / -D? Or would that
> break the .c files?

Unfortunately, we don't use LOCAL_INCLUDES / DEFINES in the assembler command lines. We can certainly improve that, but I'd rather avoid the rabbit hole for now.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5583ce8972c5

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5583ce8972c5