505832 - Ship an add-on blocklist by default

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Robert Kaiser]

Once AMO sends us a useful blocklist (see bug 505806), we should incorporate the current copy from there as a default blocklist in every SM installation, copying what Firefox does in http://mxr.mozilla.org/comm-central/source/mozilla/browser/app/Makefile.in#362 which installs blocklist.xml (and of course the package lists need to package that file).

Comment 1

[Robert Kaiser]

Nominating for b2 to get useful testing of the patch before final.

Comment 2

[Robert Kaiser]

Attachment: add a blockist into our builds

This is the patch to add the current AMO blocklist to our builds, once bug 505806 gives us a better list on AMO, we should update the XML file in the tree, but the build integration is there with this patch.

Comment 3

[[e-mail]]

Comment on attachment 397466 [details] [diff] [review]
add a blockist into our builds

>+libs:: $(srcdir)/blocklist.xml
>+	$(INSTALL) $(IFLAGS1) $^ $(DIST)/bin
This looks wrong, if compared to, say, the rules for the seamonkey script.

>+    <emItem id="[e-mail]">
>+      <versionRange minVersion="2.0" maxVersion="2.0"/>
>+    </emItem>
>+    <emItem id="[e-mail]">
>+      <versionRange minVersion=" " maxVersion="1.0.0.5"/>
>+    </emItem>
These items bear no relevance to SeaMonkey, so I don't see the point of including them...

> bin/README
> bin/license.txt
>+bin/blocklist.xml
> bin/@MOZ_APP_NAME@-bin
> bin/@MOZ_APP_NAME@
> bin/run-mozilla.sh
> bin/application.ini
> bin/platform.ini
Hmm, our ordering here seems to be creation order, so I'd simply add blocklist.xml to the end of the list ;-)

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> (From update of attachment 397466 [details] [diff] [review])
> >+libs:: $(srcdir)/blocklist.xml
> >+	$(INSTALL) $(IFLAGS1) $^ $(DIST)/bin
> This looks wrong, if compared to, say, the rules for the seamonkey script.

It's possible that other calls are missing the $(IFLAGS1) if the should have them. This is what e.g. Firefox uses and it makes sense, the flags set it to have 644 access flags on POSIX file systems.

> >+    <emItem id="[e-mail]">
> >+      <versionRange minVersion="2.0" maxVersion="2.0"/>
> >+    </emItem>
> >+    <emItem id="[e-mail]">
> >+      <versionRange minVersion=" " maxVersion="1.0.0.5"/>
> >+    </emItem>
> These items bear no relevance to SeaMonkey, so I don't see the point of
> including them...

It's what AMO currently serves for us, I don't think we should ship anything else than what they give us, but it surely might make sense to remove those two one day, and we possibly should file a bug for that. It doesn't harm to block things that don't apply in any case, so I'd rather go with what they give us than diverge from it.

> > bin/README
> > bin/license.txt
> >+bin/blocklist.xml
> > bin/@MOZ_APP_NAME@-bin
> > bin/@MOZ_APP_NAME@
> > bin/run-mozilla.sh
> > bin/application.ini
> > bin/platform.ini
> Hmm, our ordering here seems to be creation order, so I'd simply add
> blocklist.xml to the end of the list ;-)

The order is somewhat up to taste here, yes, I just wanted to keep the executables, execute script, and relevant INIs together, I don't care much about everything else.

Comment 5

[Robert Kaiser]

Attachment: add blocklist in builds, v1 (wrongly attached again)

Here's the patch with the .xml moved in the packages files and the list itself updated to the new state on AMO after bug 505806, now with the actually useful plugin blocks. I'm leaving the other items in to ship exactly what AMO has right now.
I'll file another bug on updating the other $(INSTALL) lines where needed, we should port bug 461322 over.

Comment 6

[Robert Kaiser]

(In reply to comment #5)
> I'll file another bug on updating the other $(INSTALL) lines where needed, we
> should port bug 461322 over.

Bug 513900 now, btw.

Comment 7

[Robert Kaiser]

Attachment: add blocklist in builds, v2

Oops, attached v1 again instead of the actual v2. Here's the right one.

Comment 8

[[e-mail]]

Comment on attachment 397838 [details] [diff] [review]
add blocklist in builds, v2

>+libs:: $(srcdir)/blocklist.xml
>+	$(INSTALL) $(IFLAGS1) $^ $(DIST)/bin
I checked and there's no useful difference between $^ here and $< which we use in the rest of app/Makefile.in, but you can drop the $(srcdir)/ since that's the VPATH and will get picked up automatically; compare branding/Makefile.in:

>libs:: icons/gtk/seamonkey.png
>        $(INSTALL) $^ $(DIST)/bin/chrome/icons/default
> 
>install:: icons/gtk/seamonkey.png
>        $(SYSINSTALL) $(IFLAGS1) $^ $(DESTDIR)$(mozappdir)/chrome/icons/default
[Interesting that we do use $(IFLAGS1) this time!]

>\ No newline at end of file
Save as "XML document, complete" bit you :-(

Comment 9

[Robert Kaiser]

(In reply to comment #8)
> >install:: icons/gtk/seamonkey.png
> >        $(SYSINSTALL) $(IFLAGS1) $^ $(DESTDIR)$(mozappdir)/chrome/icons/default
> [Interesting that we do use $(IFLAGS1) this time!]

Yes, the patch we need to port is about making things happening with $(INSTALL) and $(SYSINSTALL) equal.

Comment 10

[Robert Kaiser]

Pushed as http://hg.mozilla.org/comm-central/rev/02903d938d67