1573570 - Include prebuilt OTR libraries in Thunderbird CI builds

Status: RESOLVED WONTFIX

(Thunderbird :: Build Config, enhancement)

Description

[Rob Lemley [:rjl]]

First round of getting OTR messaging into Thunderbird is going to involve downloading and caching prebuilt versions of libotr itself and its dependencies.

Taskcluster can fetch, verify, and cache a URL without too much work. Fetch tasks are all defined in the Mozilla checkout using the "reference_loader". A new loader is needed that is able to combine the generated jobs data for the past few days. This new "merge_loader" will make use of the current "reference_loader" implementation as well as "transform_loader".

The basic idea is to create a new loader that will make use of the existing reference_loader to look for kind definitions in M-* first, then use the "jobs-from" functionality to bring in some jobs defined in C-*.

Comment 1

[Rob Lemley [:rjl]]

Attachment: Patch 1 - merge_loader.patch

This is the Taskcluster merge_loader.

Using the merge loader, tasks within a single kind can come from
both M-C and C-C trees. The implementation calls the reference_loader
function followed by the transform_loader function, merging the
two loaders into one. 

As noted in the commit message, there are issues when the kind
has dependencies. I believe it is only a problem when a task
within a kind depends on another task of the same kind. This
happens quite a bit with toolchains for instance.

The initial use case for this is for some fetch tasks, and there
are no dependency issues there.

Comment 2

[Rob Lemley [:rjl]]

Attachment: Patch 2 - otr_fetches.patch

Task configuration that grabs Kai's prebuilt OTR libraries from his
server so they can be included in Taskcluster builds.

Uses merge_loader.

Currently broken.

Comment 3

[Jorg K (CEST = GMT+2)]

Comment on attachment 9085304 [details] [diff] [review]
Patch 2 - otr_fetches.patch

Nit in commit message: This just sets up the Taskcluster pieec.

Comment 4

[Rob Lemley [:rjl]]

Attachment: Patch 2 - otr_fetches.patch

Latest update of patch. This is working now.

Comment 5

[Rob Lemley [:rjl]]

Comment on attachment 9085303 [details] [diff] [review]
Patch 1 - merge_loader.patch

No changes have been made to this patch since initial submission. It's been working well.

Comment 6

[Rob Lemley [:rjl]]

Comment on attachment 9089913 [details] [diff] [review]
Patch 2 - otr_fetches.patch

This are the fetch tasks that will retrieve Kai's prebuilt libraries and modifications to the build configurations so they depend on those files.
Only submitting for review, waiting for a green light to actually land these.

Comment 7

[Kai Engert (:KaiE:)]

I confirm the file hashes are correct.

Comment 8

[Rob Lemley [:rjl]]

Attachment: Patch 3 - otr_packaging.patch

If you have suggestions to improve package-manifest.in, let me know.
Hopefully some creative Makefile flags can improve the situation in the next
iteration.

Comment 9

[Rob Lemley [:rjl]]

Attachment: Patch 3 - otr_packaging.patch

Typo fix.

Comment 10

[Geoff Lankow (:darktrojan)]

Comment on attachment 9090119 [details] [diff] [review]
Patch 3 - otr_packaging.patch

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

This seems fine.

Just for the record, this is only a review of the code. I'm not making a comment on whether or not it should be done this way (or at all) – those decisions are for other people.

::: third_party/Makefile.in
@@ +15,5 @@
> +	libgpg-error-0.dll \
> +	libotr-5.dll \
> +	libssp-0.dll
> +else
> +ifeq (Darwin,$(OS_ARCH))

I'm no expert on Makefiles, so ignore me if I'm wrong, but I think if this was "else ifeq …" you'd only need one endif.

@@ +24,5 @@
> +else
> +OTR_LIBS = \
> +	libgcrypt.so.20 \
> +    libgpg-error.so.0 \
> +    libotr.so.5

Indentation.

Comment 11

[Kai Engert (:KaiE:)]

Rob, as soon as this bug is fixed, will all published builds contain the binaries?

Can you also port this to the 68.x branch?

Comment 12

[Rob Lemley [:rjl]]

Bug 1518166 is ready pending a couple of reviews/approvals so there's no need to use the prebuilt libraries.