Jorg K (CEST = GMT+2) Reporter
	Description • 6 years ago

Sadly Provider for Google Calendar was dropped from Calendar as part of bug 1439469 and the test was disabled here:
https://hg.mozilla.org/comm-central/rev/8bbed059de0a#l13.14

We need to re-instate this asap since this is blocking TB 60 ESR.

	Philipp Kewisch [:Fallen] [:📆][:🧩]
	Comment 1 • 6 years ago

I'm fine building it with Thunderbird, but I do not want to ship it with Thunderbird at this time.

	Geoff Lankow (:darktrojan) Assignee
	Comment 2 • 6 years ago

We need to either figure out a way to build it and upload as a build artifact, or put it in the tests packages. I'm not really sure how to do either, but the latter would be easier to implement, I think.

	Geoff Lankow (:darktrojan) Assignee
	Comment 3 • 6 years ago

This is more of a mess than it appears. I've managed to (at least on my machine) fix things so the extension builds again (broken in several ways by the evolving build system) and the tests (almost) pass.

	Landry Breuil (:gaston)
	Comment 4 • 6 years ago

(In reply to Philipp Kewisch [:Fallen]  from comment #1)
> I'm fine building it with Thunderbird, but I do not want to ship it with
> Thunderbird at this time.

does this mean it will get built as a xpi somewhere in the objdir/dist and packagers get the choice to bundle its xpi in their packaging work ? I get it that tier1 platforms (ie mac & windows mostly) want it shipped via AMO/official channels, but for us building for source tarballs and doing our packaging that is not always possible.

	Geoff Lankow (:darktrojan) Assignee
	Comment 5 • 6 years ago

This will build gdata-provider-4.1.en-US.xpi from comm-beta, and with a small manipulation of the manifest files*, pass the tests. (Not on taskcluster. In fact the test_gdata_provider.js will fail on taskcluster if this lands.)

Is this enough to release for 60, given that the L10n repackaging is done manually anyway?


* In objdir/dist/bin/extensions/{a62ef8ec-5fdc-40c2-873c-223b8a6925cc}/chrome/gdata-provider.manifest, the resource line needs an extra dot, or moving to the parent directory. Ditto for Lightning, which also needs an "interfaces components/interfaces.xpt" line.

	Geoff Lankow (:darktrojan) Assignee
	Comment 6 • 6 years ago

Ignore that last commenting about modifying the built files, apparently if you find the magic words the build system will do it for you! (I knew it was there somewhere, just hadn't figured it out.)

	Carsten Schoenert
	Comment 7 • 6 years ago

I've taken the last provided patch from Geoff and it works smoothly with 60.0b9 to create the gdata-provider XPI archive again. Baked all together and uploaded the 60.0~b9-1 to Debian experimental just right now. Thanks for taking action on the missed gdata-provider built!

	Philipp Kewisch [:Fallen] [:📆][:🧩]
	Comment 8 • 6 years ago

Comment on attachment 8988645 [details] [diff] [review]
1471326-gdata-beta-2.diff

Thanks, this looks fine to me from a high level. I would love to get feedback from Tom because he was the one who removed this stuff. Would adding this back in break any part of the TC builds?

	Tom Prince [:tomprince]
	Comment 9 • 6 years ago

I've push to try with l10n jobs added
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8d05263a284f67a4e4baaea083e92de1adf8d0e2
via
> mach try fuzzy --full -p https://index.taskcluster.net/v1/task/comm.v2.comm-central.latest.taskgraph.decision/artifacts/public/parameters.yml -q "l10n '-1/ \!sign"

	Tom Prince [:tomprince]
	Comment 10 • 6 years ago

Comment on attachment 8988645 [details] [diff] [review]
1471326-gdata-beta-2.diff

Review of attachment 8988645 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see any obvious problems with the code here, although I'm not familiar enough with the code around packaging extensions.

The only concern I have is that automation continues to work with this, in particular the l10n repacks.

The code that broke when building extensions was
> https://searchfox.org/mozilla-central/rev/97d488a17a848ce3bebbfc83dc916cf20b88451c/testing/mozharness/scripts/desktop_l10n.py#504-535
As long as the l10n repack jobs are working, this should work fine in automation.

	Geoff Lankow (:darktrojan) Assignee
	Comment 11 • 6 years ago

Current version of the patch for comm-central. This does upload an XPI file as a build artifact, however it doesn't download it for the test, hence the test would fail if this landed. I need to make some change (not sure what, exactly) to the TaskCluster configuration before that will happen.

	Geoff Lankow (:darktrojan) Assignee
	Comment 12 • 6 years ago

This packs the XPI on the builder, uploads as an artifact, then downloads it onto the test machine and tests it. Also works on local builds.

By the way, the test is failing for icaljs.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e56535b15bf714f798fc7ac83ee1b4433a2a2cb2&group_state=expanded

	Philipp Kewisch [:Fallen] [:📆][:🧩]
	Comment 13 • 6 years ago

Comment on attachment 8996895 [details] [diff] [review]
1471326-gdata-2.diff

Review of attachment 8996895 [details] [diff] [review]:
-----------------------------------------------------------------

::: calendar/providers/gdata/manifest.json
@@ -33,5 @@
> -
> -    <em:name>Provider for Google Calendar</em:name>
> -    <em:description>Allows bidirectional access to Google Calendar</em:description>
> -    <em:creator>Philipp Kewisch</em:creator>
> -    <em:contributor>Mark James (http://www.famfamfam.com/lab/icons/silk/)</em:contributor>

I am using a famfamfam icon somewhere. Unless we get a new one of our own, this needs to go somewhere else in order to properly attribute it to the author.

	Geoff Lankow (:darktrojan) Assignee
	Comment 14 • 6 years ago

I've opened bug 1481116 for that. This might as well land.

	Pulsebot
	Comment 15 • 6 years ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/b3975c8d27c1
Build GData Provider with Thunderbird and re-enable its test test_gdata_provider.js. r=philipp

	Jorg K (CEST = GMT+2) Reporter
	Comment 16 • 6 years ago

Hmm, you built this in a way that disabling calendar now makes all Xpcshell tests fail :-(

See here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=1bc3123517e0d626bc01dcb77d78461a701d2ecc

	Jorg K (CEST = GMT+2) Reporter
	Comment 17 • 6 years ago

OK, closing this again. Tom told me on IRC that you can't have conditions in those TC files, so commenting out some lines from taskcluster/ci/test/tests.yml like here
https://hg.mozilla.org/comm-central/rev/b6fa8a1d975dfd1c2a4148a89102c2c026b96553#l1.15
is the best we can do :-(

	WaltS48 [:walts48]
	Comment 18 • 6 years ago

It's nice that gdata is being built as part of Thunderbird, but it would be nice to find and install an extension when testing release candidates before they are officially released.

Might it be time to start bundling it with Thunderbird?

	Jorg K (CEST = GMT+2) Reporter
	Comment 19 • 6 years ago

(In reply to WaltS48 [:walts48] from comment #18)
> Might it be time to start bundling it with Thunderbird?
I don't think so, but it's about time suitable version were stored at a known location instead of having to get a TaskCluster build artifact :-( - See my post on tb-drivers:

... storing the extensions in a place where savvy beta users might find it, that is on the Mozilla FTP server, and perhaps making this location known. Releases[4] end with 4.0 and candidates[5] end with 6.2b6. That directory also contains the last version of gData in a "semi"-guessable location[6].

[4] http://ftp.mozilla.org/pub/calendar/lightning/releases/
[5] http://ftp.mozilla.org/pub/calendar/lightning/candidates/
[6] http://ftp.mozilla.org/pub/calendar/lightning/candidates/6.2b6-candidates/build1/win32/gdata-provider-4.1b6.en-US.win32.xpi