Build broken with --disable-bits-download: undefined symbol: _new_bits_service
Categories
(Toolkit :: Application Update, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox-esr68 | --- | fixed |
firefox68 | --- | wontfix |
firefox69 | --- | wontfix |
firefox70 | --- | fixed |
People
(Reporter: away, Assigned: agashlin)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-esr68+
|
Details | Review |
[task 2019-08-09T19:27:00.817Z] 19:27:00 INFO - lld-link: error: undefined symbol: _new_bits_service
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
I think this is because we always build the bitsdownload
component. I think that should be conditional on MOZ_BITS_DOWNLOAD
, which is what controls whether the Rust side gets built (that's what's missing here).
This only wasn't an issue on non-Windows because the C++ side is only built on Windows, and the component requires both MOZ_BITS_DOWNLOAD
and Windows.
For consistency all these should be doing the same check; I'm inclined to believe it should be MOZ_BITS_DOWNLOAD
since --enable-bits-download
only works on Windows.
At least one other wrinkle: Services.py always generates three files with Bits
and nsIBits
included.
I think that should be conditional as well, there's no reason to have the service, or interface, if there's no BITS support. I'm currently testing this.
- Build with
--disable-bits-download
: Fails as reported - Build with
--enable-bits-download
on linux64: Denied - Build with
--disable-bits-download
and proposed fixes: pending
Assignee | ||
Comment 2•5 years ago
|
||
References to nsIBits and Bits.jsm are all throughout UpdateService.jsm and UpdateTelemetry.jsm. It doesn't look like I'm going to be able to remove that without a lot of refactoring. The import would ideally be conditional on AppConstants.MOZ_BITS_DOWNLOAD, but I don't know how far the changes would have to spread for that to be safe.
I'm going to try just adding a MOZ_BITS_DOWNLOAD
check to the moz.build to avoid bringing in the C++ that links to the missing symbol, this should be equivalent to how it works now on non-Windows targets.
try:
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
BITS download can be disabled on Windows. This patch checks for the
config/define MOZ_BITS_DOWNLOAD when including the C++ parts of the
service, exposing the service in JS, setting the pref, and including
BITS-specific tests. For consistency and simplicity it also removes
the Windows checks; the configure system won't allow
MOZ_BITS_DOWNLOAD if not on Windows.
Comment 6•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
Comment on attachment 9088231 [details]
Bug 1572844 - Consistently check for MOZ_BITS_DOWNLOAD. r?bytesized
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: ESR is used (vs the faster rolling releases) as the basis of building a number of other projects, and they may want to build with --disable-bits-download (to avoid linking to QmgrPrxy.dll, or reduce build size or time, or for security purposes). In particular, Tor Browser needs this, see bug 1576621.
- User impact if declined: Anyone wanting to build for Windows with --disable-bits-download would have to cherry-pick this patch, or do their own work in patching out BITS support.
- Fix Landed on Version: 70
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This shouldn't affect our own builds at all, and the automated tests for the fallback update mechanism (incremental downloader) are still passing with --disable. I've tested grafting 7ce6426b7653 from mozilla-central and it seems ok: normal, --disable
- String or UUID changes made by this patch:
Comment 8•5 years ago
|
||
Comment on attachment 9088231 [details]
Bug 1572844 - Consistently check for MOZ_BITS_DOWNLOAD. r?bytesized
build fix for windows in a non-default config, approved for 68.2
Comment 9•5 years ago
|
||
bugherder uplift |
Updated•3 years ago
|
Description
•