Closed Bug 505832 Opened 11 years ago Closed 11 years ago

Ship an add-on blocklist by default

Categories

(SeaMonkey :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0b2

People

(Reporter: kairo, Assigned: kairo)

References

Details

Attachments

(1 file, 2 obsolete files)

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).
Nominating for b2 to get useful testing of the patch before final.
Flags: blocking-seamonkey2.0b2?
Flags: blocking-seamonkey2.0b2? → blocking-seamonkey2.0b2+
Attached patch add a blockist into our builds (obsolete) — Splinter Review
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.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #397466 - Flags: review?(neil)
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 ;-)
(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.
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.
Attachment #397466 - Attachment is obsolete: true
Attachment #397836 - Flags: review?(neil)
Attachment #397466 - Flags: review?(neil)
(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.
Oops, attached v1 again instead of the actual v2. Here's the right one.
Attachment #397836 - Attachment is obsolete: true
Attachment #397838 - Flags: review?(neil)
Attachment #397836 - Flags: review?(neil)
Attachment #397836 - Attachment description: add blocklist in builds, v2 → add blocklist in builds, v1 (wrongly attached again)
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 :-(
Attachment #397838 - Flags: review?(neil) → review+
(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.
Pushed as http://hg.mozilla.org/comm-central/rev/02903d938d67
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Blocks: 525465
You need to log in before you can comment on or make changes to this bug.