Closed Bug 1261473 Opened 3 years ago Closed 3 years ago

Remove INSTALL_TARGETS from addon-sdk/Makefile.in

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: mshal, Assigned: mshal)

Details

Attachments

(1 file)

We have to explicitly list the addons in moz.build, since I don't believe we can do the equivalent of a $(wildcard) with a $(patsubst) to replace text.
Comment on attachment 8737344 [details]
MozReview Request: Bug 1261473 - Remove INSTALL_TARGETS from addon-sdk/Makefile.in; r?chmanchester

https://reviewboard.mozilla.org/r/43905/#review40817

I have a question below, but this is probably good to land despite that.

::: addon-sdk/moz.build:62
(Diff revision 1)
> +    'translators',
> +    'unsafe-content-script',
> +]
> +
> +addons = ['source/test/addons/%s.xpi' % f for f in addons]
> +GENERATED_FILES += addons

So, by having these in GENERATED_FILES with no script, we assign these targets to a tier, and leave the target implementations in the makefile.

This seems a little tricky, any particular reason we're not turning the targets into a script at this point?

::: addon-sdk/moz.build:64
(Diff revision 1)
> +TEST_HARNESS_FILES.testing.mochitest['jetpack-addon']['addon-sdk'].source.test.addons += [
> +    '!%s' % f for f in addons
> +]

Do we disallow wildcards in objdir paths? (This looks fine, just curious.)
Attachment #8737344 - Flags: review?(cmanchester) → review+
Comment on attachment 8737344 [details]
MozReview Request: Bug 1261473 - Remove INSTALL_TARGETS from addon-sdk/Makefile.in; r?chmanchester

https://reviewboard.mozilla.org/r/43905/#review40831

::: addon-sdk/moz.build:62
(Diff revision 1)
> +    'translators',
> +    'unsafe-content-script',
> +]
> +
> +addons = ['source/test/addons/%s.xpi' % f for f in addons]
> +GENERATED_FILES += addons

I also notice these move from misc to export as a result of this change, is that intentional?
Attachment #8737344 - Flags: review+
(In reply to Chris Manchester (:chmanchester) from comment #2)
> ::: addon-sdk/moz.build:62
> (Diff revision 1)
> > +    'translators',
> > +    'unsafe-content-script',
> > +]
> > +
> > +addons = ['source/test/addons/%s.xpi' % f for f in addons]
> > +GENERATED_FILES += addons
> 
> So, by having these in GENERATED_FILES with no script, we assign these
> targets to a tier, and leave the target implementations in the makefile.
> 
> This seems a little tricky, any particular reason we're not turning the
> targets into a script at this point?

I'd like to get all of these kinds of rules turned into GENERATED_FILES.scripts this quarter, and addons in particular are covered by bug 988938. There's no particular reason we can't move these now, other than we have to break it down into somewhat manageable chunks :)

> ::: addon-sdk/moz.build:64
> (Diff revision 1)
> > +TEST_HARNESS_FILES.testing.mochitest['jetpack-addon']['addon-sdk'].source.test.addons += [
> > +    '!%s' % f for f in addons
> > +]
> 
> Do we disallow wildcards in objdir paths? (This looks fine, just curious.)

Good question - I gave it a quick test, and it seems we might be able to add support for that by making sure the recursive make backend adds $(wildcard) when necessary in the _FILES variables. However, that doesn't buy us much until the addons are in moz.build since we'll still need the full list in GENERATED_FILES I believe.
(In reply to Chris Manchester (:chmanchester) from comment #3)
> ::: addon-sdk/moz.build:62
> (Diff revision 1)
> > +    'translators',
> > +    'unsafe-content-script',
> > +]
> > +
> > +addons = ['source/test/addons/%s.xpi' % f for f in addons]
> > +GENERATED_FILES += addons
> 
> I also notice these move from misc to export as a result of this change, is
> that intentional?

Yeah, we don't have much control over which tier GENERATED_FILES are actually generated, since we try to hide tier information in moz.build files (it's an implementation detail of the recursive make backend). The current logic for it is to generate the files in the export tier, unless the script depends on other generated files. In this case, all of the addon files are in the source directory, so while moving it to the export tier isn't really intended, it isn't harmful to do so.
https://hg.mozilla.org/mozilla-central/rev/42ba1f78bde5
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.