Last Comment Bug 505832 - Ship an add-on blocklist by default
: Ship an add-on blocklist by default
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.0b2
Assigned To: Robert Kaiser
:
:
Mentors:
Depends on: 505806
Blocks: 525465
  Show dependency treegraph
 
Reported: 2009-07-22 11:45 PDT by Robert Kaiser
Modified: 2009-10-30 08:50 PDT (History)
0 users
neil: blocking‑seamonkey2.0b2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
add a blockist into our builds (2.37 KB, patch)
2009-08-29 12:09 PDT, Robert Kaiser
no flags Details | Diff | Splinter Review
add blocklist in builds, v1 (wrongly attached again) (2.37 KB, patch)
2009-09-01 04:13 PDT, Robert Kaiser
no flags Details | Diff | Splinter Review
add blocklist in builds, v2 (3.47 KB, patch)
2009-09-01 04:27 PDT, Robert Kaiser
neil: review+
Details | Diff | Splinter Review

Description Robert Kaiser 2009-07-22 11:45:03 PDT
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 2009-07-28 09:57:15 PDT
Nominating for b2 to get useful testing of the patch before final.
Comment 2 Robert Kaiser 2009-08-29 12:09:15 PDT
Created attachment 397466 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2009-08-31 15:12:55 PDT
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="langpack-vi-VN@firefox.mozilla.org">
>+      <versionRange minVersion="2.0" maxVersion="2.0"/>
>+    </emItem>
>+    <emItem id="support@daemon-tools.cc">
>+      <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 2009-08-31 17:54:47 PDT
(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="langpack-vi-VN@firefox.mozilla.org">
> >+      <versionRange minVersion="2.0" maxVersion="2.0"/>
> >+    </emItem>
> >+    <emItem id="support@daemon-tools.cc">
> >+      <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 2009-09-01 04:13:35 PDT
Created attachment 397836 [details] [diff] [review]
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 2009-09-01 04:17:25 PDT
(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 2009-09-01 04:27:39 PDT
Created attachment 397838 [details] [diff] [review]
add blocklist in builds, v2

Oops, attached v1 again instead of the actual v2. Here's the right one.
Comment 8 neil@parkwaycc.co.uk 2009-09-01 04:45:50 PDT
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 2009-09-01 11:09:25 PDT
(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 2009-09-01 11:14:53 PDT
Pushed as http://hg.mozilla.org/comm-central/rev/02903d938d67

Note You need to log in before you can comment on or make changes to this bug.