Closed Bug 929905 Opened 7 years ago Closed 7 years ago

Consolidate sources in moz.build

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla27

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(2 files, 3 obsolete files)

We currently have CPP_SOURCES, CSRCS, ASFILES, etc. As discussed in I-don't-remember-what-bug, we should just use a single variable and discriminate by extension.

[Since i wanted to add another category of sources in bug 928244, I figured I should do this first]
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Comment on attachment 820867 [details] [diff] [review]
Consolidate sources in moz.build, GTEST only.

This is my implementation proposal, limited to gtests as a conversation opener. Kind of hackish, but i prefer keeping the filtering out of the backend.
Attachment #820867 - Flags: feedback?(gps)
Depends on: 913268
Comment on attachment 820867 [details] [diff] [review]
Consolidate sources in moz.build, GTEST only.

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

Makes sense to me.

Not sure if you are planning to expand scope to CPP_SOURCES and friends. But I was thinking those would always end up using our next gen binary defining syntax from bug 879837 (whatever that may be). Not sure if it is worth consolidating lists today only to munge it again shortly.

::: python/mozbuild/mozbuild/frontend/emitter.py
@@ +181,5 @@
> +            GTEST_CSRCS=('GTEST_SOURCES', ['.c']),
> +        )
> +        for mak, (moz, ext) in varmap.items():
> +            if sandbox[moz]:
> +                filtered = [f for f in sandbox[moz] if any(f.endswith(e) for e in ext)]

endswith() can accept a tuple of items to match against.
Attachment #820867 - Flags: feedback?(gps) → feedback+
With adjustments to the endswith thing, and handling all source variables.

https://tbpl.mozilla.org/?tree=Try&rev=cdeccb117004
Attachment #821372 - Flags: review?(gps)
Attachment #820867 - Attachment is obsolete: true
Comment on attachment 821372 [details] [diff] [review]
Consolidate sources in moz.build.

There are a couple issues to solve, still.
Attachment #821372 - Flags: review?(gps)
Comment on attachment 821372 [details] [diff] [review]
Consolidate sources in moz.build.

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

I only casually looked at everything except the .py files.
Attachment #821372 - Attachment is obsolete: true
Attachment #821512 - Attachment mime type: text/x-python → text/plain
(In reply to Mike Hommey [:glandium] from comment #7)
> Created attachment 821510 [details] [diff] [review]
> Consolidate sources in moz.build.
> 
> https://tbpl.mozilla.org/?tree=Try&rev=0c22327d9cd8

And for Windows, since i fucked up bug 930380:
https://tbpl.mozilla.org/?tree=Try&rev=bd2c5cea84c6
Depends on: 930380
Err, I was effectively reverting bug 930380 with the emitter changes.
Attachment #821532 - Flags: review?(gps)
Attachment #821510 - Attachment is obsolete: true
Attachment #821510 - Flags: review?(gps)
Another windows try with hopefully a working bug 930380:
https://tbpl.mozilla.org/?tree=Try&rev=b85dc94eb0aa
Comment on attachment 821532 [details] [diff] [review]
Consolidate sources in moz.build.

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

Again, I only very casually looked at the automated moves.

::: python/mozbuild/mozbuild/frontend/emitter.py
@@ +184,5 @@
> +        for mak, (moz, ext) in varmap.items():
> +            if sandbox[moz]:
> +                filtered = [f for f in sandbox[moz] if f.endswith(ext)]
> +                if filtered:
> +                    passthru.variables[mak] = filtered

Do you think its worth checking for files that aren't passed through because of an unrecognized extension? I'd hate to see things silently fail for this reason. Followup?
Attachment #821532 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/5dd08c88e328
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: 932133
Duplicate of this bug: 877917
Blocks: 932170
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.