822017 - Remove the global PB specific code from the testpilot extension

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[(no longer active)]

Is there any way for us to preprocess the files inside http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/extensions/[e-mail]/, for example http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/extensions/[e-mail]/modules/setup.js?  I can't quite figure out how this extension is built...

Comment 1

[(no longer active)]

Attachment: Patch (v1)

I replaced the install.rdf specific logic in the Makefile with a generic logic which can handle any file names *.in.

Comment 2

[Josh Matthews [:jdm]]

Comment on attachment 696575 [details] [diff] [review]
Patch (v1)

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

I don't think this is something I should review.

Comment 3

[(no longer active)]

Comment on attachment 696575 [details] [diff] [review]
Patch (v1)

Trying Dao...

Comment 4

[Josh Matthews [:jdm]]

Comment on attachment 696575 [details] [diff] [review]
Patch (v1)

I think I recall Gregg being granted ownership of Test Pilot when Jono left.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 696575 [details] [diff] [review]
Patch (v1)

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

::: browser/app/profile/extensions/Makefile.in
@@ +36,5 @@
>  endef # do not remove the blank line!
>  
>  libs::
>  	$(NSINSTALL) -D $(DISTROEXT)
>  	$(foreach dir,$(EXTENSIONS),$(_INSTALL_EXTENSION))

Man this whole thing is so horrible.

Comment 6

[Mike Hommey [:glandium]]

Attachment: Use less bad build rules in browser/app/profile/extensions

This replaces ehsan's build changes, while improving the existing build rules and making things less worse overall.

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 696613 [details] [diff] [review]
Use less bad build rules in browser/app/profile/extensions

Ah, need to find the .in recursively.

Comment 8

[Mike Hommey [:glandium]]

Attachment: Use less bad build rules in browser/app/profile/extensions

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 696616 [details] [diff] [review]
Use less bad build rules in browser/app/profile/extensions

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

::: browser/app/profile/extensions/Makefile.in
@@ +44,5 @@
> +$(foreach d,$(EXTENSIONS), \
> +  $(foreach in,$(shell cd $(srcdir) ; find $(d) -name '*.in'), \
> +    $(eval $(call pp_one,$(d),$(in))) \
> +  ) \
> +)

This sucks less, but this still kind of sucks. We should think about how we'd represent complicated logic like this in the new moz.build setup.

::: browser/app/profile/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/Makefile.in
@@ +15,1 @@
>  	$(NULL)

nit: fix the indentation while you're here.

Comment 10

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #9)
> Comment on attachment 696616 [details] [diff] [review]
> Use less bad build rules in browser/app/profile/extensions
> 
> Review of attachment 696616 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/app/profile/extensions/Makefile.in
> @@ +44,5 @@
> > +$(foreach d,$(EXTENSIONS), \
> > +  $(foreach in,$(shell cd $(srcdir) ; find $(d) -name '*.in'), \
> > +    $(eval $(call pp_one,$(d),$(in))) \
> > +  ) \
> > +)
> 
> This sucks less, but this still kind of sucks. We should think about how
> we'd represent complicated logic like this in the new moz.build setup.

We should just not have such complicated logic. In the present case, i'm planning to add a script to package extensions, based on the new packager code.

Comment 11

[(no longer active)]

Comment on attachment 696616 [details] [diff] [review]
Use less bad build rules in browser/app/profile/extensions

https://hg.mozilla.org/integration/mozilla-inbound/rev/eed2a6f24243

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 696575 [details] [diff] [review]
Patch (v1)

I assume the Makefile.in changes are obsoleted by attachment 696616 [details] [diff] [review].

Comment 13

[(no longer active)]

(In reply to comment #12)
> I assume the Makefile.in changes are obsoleted by attachment 696616 [details] [diff] [review].

They are, yes.

Comment 14

[Ed Morley [:emorley]]

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

Comment 15

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/041b62d88da9

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/041b62d88da9