Closed Bug 875549 Opened 6 years ago Closed 6 years ago

move HOST_CSRCS to moz.build

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla24

People

(Reporter: joey, Assigned: joey)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

No description provided.
Assignee: nobody → joey
Comment on attachment 758666 [details] [diff] [review]
move HOST_CSRCS to moz.build (logic)

Add HOST_CSRCS as a passthrough variable.
Attachment #758666 - Flags: review?(ted)
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."
Attachment #758666 - Flags: review?(ted) → review+
Changed variable description per review, rebased, r=ted carried forward.
Attachment #758666 - Attachment is obsolete: true
Whiteboard: [leave open]
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 on attachment 759953 [details] [diff] [review]
move HOST_CSRCS to mozbuild (file batch #1).

First batch of HOST_CSRCS files converted.
Try results pending.
Attachment #759953 - Flags: review?(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']:
Attachment #759953 - Flags: review?(mshal) → review+
Attachment #759953 - Attachment is obsolete: true
Comment on attachment 760476 [details] [diff] [review]
move HOST_CSRCS to mozbuild (file batch #1).

Nit fix, r=mshal carried forward
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 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
Blocks: nomakefiles
(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.
Duplicate of this bug: 882326
Conversion patch has stuck for a week so should hold.
Remove DISABLED_ variables and add HOST_CSRCS to the restricted variable list.
Attachment #763709 - Flags: review?(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?
Attachment #763709 - Flags: review?(gps) → review+
(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.
See Also: → 870406
CSRCS= will be handled in the conversion bug 870406
Inbound push for HOST_CSRCS conversion cleanup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/045aa030111e
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/045aa030111e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.