875549 - move HOST_CSRCS to moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Joey Armstrong [:joey]]

Attachment: move HOST_CSRCS to moz.build (logic)

Comment 2

[Joey Armstrong [:joey]]

Comment on attachment 758666 [details] [diff] [review]
move HOST_CSRCS to moz.build (logic)

Add HOST_CSRCS as a passthrough variable.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 758666 [details] [diff] [review]
move HOST_CSRCS to moz.build (logic)

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

::: python/mozbuild/mozbuild/frontend/sandbox_symbols.py
@@ +92,5 @@
>          delimiters.
>          """),
>  
> +    'HOST_CSRCS': (StrictOrderingOnAppendList, list, [],
> +        """C code source files to include when cross-compiling.

I would probably say "C source files to compile with the host compiler."

Comment 4

[Joey Armstrong [:joey]]

Attachment: move HOST_CSRCS to moz.build (logic)

Changed variable description per review, rebased, r=ted carried forward.

Comment 5

[Joey Armstrong [:joey]]

Comment on attachment 759373 [details] [diff] [review]
move HOST_CSRCS to moz.build (logic)

Inbound push: http://hg.mozilla.org/integration/mozilla-inbound/rev/ebae7298e381

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Joey Armstrong [:joey]]

Attachment: move HOST_CSRCS to mozbuild (file batch #1).

Comment 8

[Joey Armstrong [:joey]]

Comment on attachment 759953 [details] [diff] [review]
move HOST_CSRCS to mozbuild (file batch #1).

First batch of HOST_CSRCS files converted.
Try results pending.

Comment 9

[Michael Shal [:mshal]]

Comment on attachment 759953 [details] [diff] [review]
move HOST_CSRCS to mozbuild (file batch #1).

>diff --git a/toolkit/crashreporter/google-breakpad/src/common/moz.build b/toolkit/crashreporter/google-breakpad/src/common/moz.build
>--- a/toolkit/crashreporter/google-breakpad/src/common/moz.build
>+++ b/toolkit/crashreporter/google-breakpad/src/common/moz.build
>@@ -42,8 +42,11 @@ if CONFIG['OS_ARCH'] == 'Linux':
> if CONFIG['OS_TARGET'] == 'Android':
>     pass
> else:
>     if CONFIG['OS_TARGET'] != 'WINNT':
>         CPP_SOURCES += [
>             'stabs_to_module.cc',
>             'stabs_reader.cc',
>         ]
>+
>+if 'WINNT' != CONFIG['OS_TARGET'] and CONFIG['MOZ_CRASHREPORTER']:
>+    HOST_CSRCS += CSRCS

nit: For consistency with other moz.build files, please put the CONFIG variable before the string it is being compared to:

if CONFIG['OS_TARGET'] != 'WINNT' and CONFIG['MOZ_CRASHREPORTER']:

Comment 10

[Joey Armstrong [:joey]]

Attachment: move HOST_CSRCS to mozbuild (file batch #1).

Comment 11

[Joey Armstrong [:joey]]

Comment on attachment 760476 [details] [diff] [review]
move HOST_CSRCS to mozbuild (file batch #1).

Nit fix, r=mshal carried forward

Comment 12

[Joey Armstrong [:joey]]

Comment on attachment 760476 [details] [diff] [review]
move HOST_CSRCS to mozbuild (file batch #1).

try job: https://tbpl.mozilla.org/?tree=Try&rev=3cf081964557

Comment 13

[Joey Armstrong [:joey]]

Comment on attachment 760476 [details] [diff] [review]
move HOST_CSRCS to mozbuild (file batch #1).

push to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/3e73c00a7db1

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3e73c00a7db1

Comment 15

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/3e73c00a7db1#l16.11 looks silly.

Comment 16

[Joey Armstrong [:joey]]

(In reply to :Ms2ger from comment #15)
> https://hg.mozilla.org/mozilla-central/rev/3e73c00a7db1#l16.11 looks silly.

Feel free to grab and work on more of the conversions then.

Comment 18

[jarmstrong]

Attachment: Cleanup for mozbuild HOST_CSRCS conversion.

Conversion patch has stuck for a week so should hold.
Remove DISABLED_ variables and add HOST_CSRCS to the restricted variable list.

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 763709 [details] [diff] [review]
Cleanup for mozbuild HOST_CSRCS conversion.

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

Good except for the one CSRCS issue (which isn't fatal, but might as well clean it up now).

::: modules/libmar/src/Makefile.in
@@ +23,5 @@
> +CSRCS = \
> +    mar_create.c \
> +    mar_extract.c \
> +    mar_read.c \
> +    $(NULL)

What's up with this file? Hasn't CSRCS moved to moz.build files? Should this list be in a moz.build file?

Comment 20

[Joey Armstrong [:joey]]

(In reply to Gregory Szorc [:gps] from comment #19)
> Comment on attachment 763709 [details] [diff] [review]
> Cleanup for mozbuild HOST_CSRCS conversion.
> 
> Review of attachment 763709 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Good except for the one CSRCS issue (which isn't fatal, but might as well
> clean it up now).
> 
> ::: modules/libmar/src/Makefile.in
> @@ +23,5 @@
> > +CSRCS = \
> > +    mar_create.c \
> > +    mar_extract.c \
> > +    mar_read.c \
> > +    $(NULL)
> 
> What's up with this file? Hasn't CSRCS moved to moz.build files? Should this
> list be in a moz.build file?

There are a boatload of variables/values that need to move and there has only been traction for landing passthrough variable/conversion patches recently.  The nomakefiles bug https://bugzilla.mozilla.org/show_bug.cgi?id=847009 bug and wiki page still itemize all the gory details for conversion status.

Most of the CSRCS assignments have already moved to mozbuild.  Any lingering (like libmar/) were included in a try patch with failures and need triage/diagnosis.

Temporarily expanding CSRCS=$(HOST_CSRCS) in this case helps to prevent patches from becoming inter-dependent.  If CSRCS= is still failing that can block this patch which is ready to land on researching a try failure.

Comment 21

[Joey Armstrong [:joey]]

CSRCS= will be handled in the conversion bug 870406

Comment 22

[jarmstrong]

Inbound push for HOST_CSRCS conversion cleanup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/045aa030111e

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/045aa030111e