Remove the global PB specific code from the testpilot extension

RESOLVED FIXED in Firefox 20

Status

()

Firefox
Private Browsing
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Away for a while, Assigned: Away for a while)

Tracking

unspecified
Firefox 20
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Updated

5 years ago
Blocks: 463027
(Assignee)

Comment 1

5 years ago
Created attachment 696575 [details] [diff] [review]
Patch (v1)

I replaced the install.rdf specific logic in the Makefile with a generic logic which can handle any file names *.in.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #696575 - Flags: review?(josh)

Comment 2

5 years ago
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.
Attachment #696575 - Flags: review?(josh)
(Assignee)

Comment 3

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

Trying Dao...
Attachment #696575 - Flags: review?(dao)

Comment 4

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

I think I recall Gregg being granted ownership of Test Pilot when Jono left.
Attachment #696575 - Flags: review?(glind)
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.
Created attachment 696613 [details] [diff] [review]
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.
Attachment #696613 - Flags: review?(ted)
Assignee: ehsan → mh+mozilla
Comment on attachment 696613 [details] [diff] [review]
Use less bad build rules in browser/app/profile/extensions

Ah, need to find the .in recursively.
Attachment #696613 - Flags: review?(ted)
Created attachment 696616 [details] [diff] [review]
Use less bad build rules in browser/app/profile/extensions
Attachment #696616 - Flags: review?(ted)
Attachment #696613 - Attachment is obsolete: true
Assignee: mh+mozilla → ehsan
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.
Attachment #696616 - Flags: review?(ted) → review+
(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.
(Assignee)

Comment 11

5 years ago
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
Attachment #696616 - Flags: checkin+
(Assignee)

Updated

5 years ago
Whiteboard: [leave open]
Comment on attachment 696575 [details] [diff] [review]
Patch (v1)

I assume the Makefile.in changes are obsoleted by attachment 696616 [details] [diff] [review].
Attachment #696575 - Flags: review?(dao) → review+
(Assignee)

Comment 13

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

They are, yes.
(Assignee)

Updated

5 years ago
Attachment #696575 - Flags: review?(glind)
https://hg.mozilla.org/mozilla-central/rev/041b62d88da9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
Depends on: 827851
You need to log in before you can comment on or make changes to this bug.