396496 - install gdata-provider into dist/bin/extensions/ when built with Thunderbird

Status: RESOLVED FIXED

(Calendar :: Provider: GData, defect)

Description

[Steffen Wilberg]

Attachment: patch

Gdata-provider should be installed automatically when build with Thunderbird.

I've just added a patch to bug 349870 which installs Lightning into Thunderbird when Thunderbird is build with --enable-extensions=default,lightning. The same should happen with gdata-provider if it is built.

So the result is a Thunderbird with (Lightning and) gdata-provider already installed in dist/bin/extensions.

The key is the INSTALL_EXTENSION_ID. It extracts the contents of
dist/xpi-stage/lightning.xpi to dist/bin/extensions:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/config/rules.mk&rev=3.570&mark=1764,1769-1774#1764

Tested with Thunderbird on the 1.8 branch.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Steffen, thanks for the patch. We have not yet decided if lightning should contain the gdata provider by default. I'll definitely keep this patch in mind though.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 281253 [details] [diff] [review]
patch

This is just a suggestion, I am eager to hear your opinion.

I think it might be a valid approach to change the patch to install the provider by default only if a mozconfig variable is set. Something like:

mk_add_options GDATA_INSTALL_GLOBAL=1

Then in the makefile something like:

ifdef GDATA_INSTALL_GLOBAL
    INSTALL_EXTENSION_ID = {a62ef8ec-5fdc-40c2-873c-223b8a6925cc}
    NO_JAR_AUTO_REG = 1
endif

In general, don't align spaces. Just one space between the = operator. Also, the use of export was intended, so that submakefiles also inherit USE_EXTENSION_MANIFEST and XPI_NAME.


Just for my personal laziness, whats NO_JAR_AUTO_REG for?

Comment 3

[Steffen Wilberg]

I'm fine with a mozconfig variable. But it should rather be LIGHTNING_INSTALL_GLOBAL (over in bug 349870): If Lightning is installed globally, and if you hack the makefile to enable gdata (I want a variable for that...), gdata should be installed globally as well.

NO_JAR_AUTO_REG:
> Finally, in order to be backwards-compatible with the old contents.rdf chrome
> registration scheme, whenever the jar.mn processor encounters a contents.rdf
> file, it will automatically add the path of that contents.rdf to the
> chrome/installed-chrome.txt file. If this behavior is undesirable (for
> instance, when packaging an extension XPI), set NO_JAR_AUTO_REG = 1 in the
> Makefile to suppress this default behavior.
I've added that for good measure and because everyone else seems to do it, but it's not strictly necessary since there's no contents.rdf around.
http://developer.mozilla.org/en/docs/JAR_Manifests#Register_Chrome

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I'm not sure I understand you correctly, but I think that automatically installing lightning with thunderbird should be a separate variable to automatically installing the gdata provider with thunderbird/sunbird.

Since I have no contents.rdf, I'd rather keep the NO_JAR_AUTO_REG out until a problem shows up that requires it.

Comment 5

[Steffen Wilberg]

Attachment: just INSTALL_EXTENSION_ID

I tried you suggestion with the mozconfig variable, but it didn't work. 
So here's another patch with just the INSTALL_EXTENSION_ID line. I think it's not too bad that the gdata provider is installed automatically as a global extension when you build it. You can even have another version of the extension in your profile; the one in the profile will override AFAIK.
A lot of extensions use INSTALL_EXTENSION_ID unconditionally: http://mxr.mozilla.org/seamonkey/search?string=INSTALL_EXTENSION_ID

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 283614 [details] [diff] [review]
just INSTALL_EXTENSION_ID

I agree, it seems not to be possible, without explicitly setting environment variables. I think this might be worth filing a but against the build system. Adding custom options in the  mozconfig seems like a sensible thing to do, but its not possible.

Regarding the tinderboxen, we have not decided to build the gdata provider by default, so there is no use in integrating the gdata provider into the application by default. If you want to do this for your own build, its not to much of a deal to add it.

Also, I don't think the gdata provider is in a state, where its a good idea to integrate it into lightning/sunbird. 

If you have good reasons to integrate, I may rethink, but I think its too early right now.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: GDATA_INSTALL_EXTENSION via mozconfig

Steffen, I found a possibility to use the mozconfig variable, by importing the processed mozconfig from $(topsrcdir)/.mozconfig.mk. This works quite well and doesn't do any extra processing when the flag is not set. 

Is this solution acceptable for your, Steffen?

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 283861 [details] [diff] [review]
GDATA_INSTALL_EXTENSION via mozconfig

this might seem to work, but you are abusing the whole build system. Please don't do this.

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Since the build system doesn't really offer an alternative, I don't see why not. .mozconfig.mk seems to be prominent enough to deserve a cvsignore entry and it contains the mozconfig entries, which is just what I want to read out.

Since I don't really have strong feelings about this bug, I'd be fine WONTFIXing this for now though. Other opinions?

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

mk_add_options ais for adding options to client.mk. That's what does checkout etc. For changes to the configuration (which is what we want here), you use ac_add_options, and change configure.in to accept that flag (plus some other file, to make it propagate). The build system does offer that alternative. I don't see why you should not use that.

But as an aside, why would we want this change, if we don't have the same for lightning itself?

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

This requires a lot of non-calendar changes just to add a configure variable, which I don't think is feasible for something small like this. In that case I'd vote for WONTFIX.

Comment 12

[Daniel Boelzle [:dbo]]

Comment on attachment 283861 [details] [diff] [review]
GDATA_INSTALL_EXTENSION via mozconfig

We are in progress of rethinking what providers should ship by default, and gdata is a good candidate. Unless we've decided on that, it IMO makes little sense to add such build options. Despsite that, the patch looks rather hacky to me; removing review.

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

This issue was fixed with the Provider for Google Calendar 1.0.2, I am therefore closing this bug.