Closed
Bug 1573570
Opened 5 years ago
Closed 5 years ago
Include prebuilt OTR libraries in Thunderbird CI builds
Categories
(Thunderbird :: Build Config, enhancement)
Thunderbird
Build Config
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: rjl, Assigned: rjl)
References
Details
Attachments
(1 file, 4 obsolete files)
|
(deleted),
patch
|
darktrojan
:
review+
|
Details | Diff | Splinter Review |
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-*.
|
Assignee | |
Comment 1•5 years ago
|
||
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.
Attachment #9085303 -
Flags: review?(geoff)
|
|
Assignee | |
Updated•5 years ago
|
Assignee: nobody → rob
Status: NEW → ASSIGNED
|
|
Assignee | |
Comment 2•5 years ago
|
||
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.
|
|
Assignee | |
Updated•5 years ago
|
Attachment #9085303 -
Flags: review?(geoff)
|
||
Comment 3•5 years ago
|
||
Comment on attachment 9085304 [details] [diff] [review]
Patch 2 - otr_fetches.patch
Nit in commit message: This just sets up the Taskcluster pieec.
|
|
Assignee | |
Comment 4•5 years ago
|
||
Latest update of patch. This is working now.
|
|
Assignee | |
Updated•5 years ago
|
Attachment #9085304 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 5•5 years ago
|
||
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.
Attachment #9085303 -
Flags: review?(geoff)
|
|
Assignee | |
Comment 6•5 years ago
|
||
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.
Attachment #9089913 -
Flags: review?(geoff)
|
||
Comment 7•5 years ago
|
||
I confirm the file hashes are correct.
|
|
Assignee | |
Comment 8•5 years ago
|
||
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.
Attachment #9090112 -
Flags: review?(geoff)
|
|
Assignee | |
Updated•5 years ago
|
Summary: Create new Taskcluster loader to look for definitions in comm- and mozilla- in a single kind → Include prebuilt OTR libraries in Thunderbird CI builds
|
|
Assignee | |
Updated•5 years ago
|
Attachment #9090112 -
Attachment is obsolete: true
Attachment #9090112 -
Flags: review?(geoff)
|
||
Updated•5 years ago
|
Attachment #9085303 -
Flags: review?(geoff) → review+
|
|
||
Updated•5 years ago
|
Attachment #9089913 -
Flags: review?(geoff) → review+
|
|
||
Comment 10•5 years ago
|
||
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.
Attachment #9090119 -
Flags: review?(geoff) → review+
|
|
||
Comment 11•5 years ago
|
||
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?
|
|
Assignee | |
Updated•5 years ago
|
Attachment #9085303 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•5 years ago
|
Attachment #9089913 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 12•5 years ago
|
||
Bug 1518166 is ready pending a couple of reviews/approvals so there's no need to use the prebuilt libraries.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•