Closed Bug 1471326 Opened Last year Closed Last year

Build (not necessarily bundle) gdata as part of thunderbird and re-enable its test test_gdata_provider.js

Categories

(Calendar :: Provider: GData, enhancement, critical)

enhancement
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jorgk, Assigned: darktrojan)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
Flags: needinfo?(philipp)
Flags: needinfo?(geoff)
I'm fine building it with Thunderbird, but I do not want to ship it with Thunderbird at this time.
Flags: needinfo?(philipp)
Summary: Bundle gdata as part of thunderbird and re-enable its test test_gdata_provider.js → Build (not necessarily bundle) gdata as part of thunderbird and re-enable its test test_gdata_provider.js
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.
Flags: needinfo?(geoff)
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.
(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.
Attached patch 1471326-gdata-beta-1.diff (obsolete) β€” β€” Splinter Review
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.
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Flags: needinfo?(philipp)
Attached patch 1471326-gdata-beta-2.diff (obsolete) β€” β€” Splinter Review
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.)
Attachment #8988381 - Attachment is obsolete: true
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!
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?
Flags: needinfo?(philipp)
Attachment #8988645 - Flags: feedback?(mozilla)
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.
Attachment #8988645 - Flags: feedback?(mozilla)
Attached patch 1471326-gdata-1.diff (obsolete) β€” β€” Splinter Review
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.
Attachment #8988645 - Attachment is obsolete: true
Attached patch 1471326-gdata-2.diff β€” β€” Splinter Review
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
Attachment #8994773 - Attachment is obsolete: true
Attachment #8996895 - Flags: review?(philipp)
Depends on: 1480274
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.
Attachment #8996895 - Flags: review?(philipp) → review+
Depends on: 1481116
I've opened bug 1481116 for that. This might as well land.
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
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → 6.5
Depends on: 1481180
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: needinfo?(geoff)
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 :-(
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Flags: needinfo?(geoff)
Resolution: --- → FIXED
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?
(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
You need to log in before you can comment on or make changes to this bug.