The default bug view has changed. See this FAQ.

Fix bsdiff linking for builds compiled with CROSS_COMPILE

RESOLVED FIXED

Status

()

Core
Build Config
P2
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: coop, Assigned: coop)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(status1.9.2 .2-fixed, status1.9.1 .9-fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

7 years ago
As part of bug 511967, I need to get the bsdiff tool building on the slaves when --enable-update-packaging is specified. This is working out-of-the-box on Linux/Mac/Windows, but is currently failing on WinMo (and WinCE, same symptoms). 

Here's the patch I ran through try-server:

http://pastebin.mozilla.org/691229

And here's the brief tinderbox log for the WinMo failure:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1260995123.1260996702.30857.gz#err4

WinCE failure is near-identical, so I'm hoping there is a single solution:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1260995122.1260996857.32737.gz#err4

This won't prevent us from getting updates running for WinMo (Bug 534025) but it will prevent us from generating those updates on the slaves.
(Assignee)

Comment 1

7 years ago
After talking with blassey on IRC today, he determined that how we build bsdiff is pretty broken. bsdiff *should* be getting built for the host (indeed, it gets put in dist/host/bin), but it gets linked against the version of nspr built for (in this case) WinMo.

We never really planned to use bsdiff in this way, so this doesn't surprise me terribly. blassey suggested that I put the bsdiff section in toolkit-tiers.mk in an "ifndef CROSS_COMPILE." This worked on the try-server and effectively unblocks me on bug 511967.

Still, if we ever want to do updates-on-slaves for WinMo via the same code path, we'll need to fix how bsdiff gets linked.
No longer blocks: 511967
Component: Windows Mobile → Build Config
Product: Fennec → Core
QA Contact: mobile-windows → build-config
Summary: Building bsdiff on WinMo fails with 2 unresolved externals → Fix bsdiff linking for builds compiled with CROSS_COMPILE
I think you mean the target version of libbz2, not NSPR. It should be fairly trivial to build a host version of libbz2: just set HOST_LIBRARY_NAME and HOST_CSRCS in modules/libbz2/src/Makefile.in
I do something similar in the Breakpad code:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/google-breakpad/src/common/linux/Makefile.in

Since I need to build some of that code for both the host (for dump_syms) and the target (for the crashreporter itself).
(Assignee)

Updated

7 years ago
Assignee: nobody → ccooper
Status: NEW → ASSIGNED
Priority: -- → P2
(Assignee)

Comment 4

7 years ago
I'm taking this on because I *think* I can muddle my way through the Makefile changes given enough pointers.

I have bsdiff attempting to build now by default when --enable-update-packaging is specified. However, I'm hitting the following problem when I try to compile bsdiff for WinCE (on a standard Win2K3 build slave in staging):

http://pastebin.mozilla.org/700308

I am assuming this is because of how HOST_LIBS is defined for bsdiff:

http://mxr.mozilla.org/mozilla-central/source/other-licenses/bsdiff/Makefile.in#50

What's the right way to change how the BZ2_LIBS are included to make sure I pick up host_bz2.lib when required (and not break the system vs. moz bz2 distinction also)?

Here's my current WIP patch:

http://pastebin.mozilla.org/700309
(Assignee)

Updated

7 years ago
Blocks: 511967
(Assignee)

Comment 5

7 years ago
(In reply to comment #4)
> What's the right way to change how the BZ2_LIBS are included to make sure I
> pick up host_bz2.lib when required (and not break the system vs. moz bz2
> distinction also)?

Ted: I suppose the above question is meant for you.

I've successfully made host_bz2.lib with the above steps, but bsdifff linking still fails because it's trying to pull in ../../modules/libbz2/src/bz2.lib and "unresolved external symbol __imp__htonl@4" is not getting pulled in from Ws2_32 when building under WinCE.

I can get mbsdiff.exe to link for WinCE if I execute the following:

$ cd obj-firefox/other-licenses/bsdiff
$ link -NOLOGO -OUT:mbsdiff.exe host_bsdiff.obj ../../dist/host/lib/hostbz2.lib Ws2_32.lib

I guess that narrows it down to two questions:

1) How to set HOST_LIBS properly so that host_bz2.lib gets picked up? Should I be ifdef-ing on some var, and if so, which one? 

2) How to set HOST_EXTRA_LIBS in the CROSS_COMPILE case (WinCE is this case)? Do I need something like "ifeq (,$(filter WINCE WINNT,$(OS_ARCH)))" ?
(Assignee)

Comment 6

7 years ago
Created attachment 424625 [details] [diff] [review]
[checked in] Compile bsdiff for host OS rather than target

This patch seems to work on WinCE/Win2k3, so I'm running it through the tryserver to make sure it doesn't break anything else before asking for review.
(Assignee)

Updated

7 years ago
Attachment #424625 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 7

7 years ago
Comment on attachment 424625 [details] [diff] [review]
[checked in] Compile bsdiff for host OS rather than target

bsdiff created successfully on all platforms on try, now Ted can tell me how I'm doing it all wrong. ;)
(Assignee)

Comment 8

7 years ago
Comment on attachment 424625 [details] [diff] [review]
[checked in] Compile bsdiff for host OS rather than target

Ted: gentle ping about the review here. 

This will have cascade unblocking effects with other nightly updates work if I can get it landed. Mostly I just want to know if I'm on the right track since I don't often muck around in Makefiles.
Yeah, sorry, my review queue got a bit backed up. I started unclogging it today, I'll definitely get this by tomorrow.
(Assignee)

Updated

7 years ago
Depends on: 542222
Comment on attachment 424625 [details] [diff] [review]
[checked in] Compile bsdiff for host OS rather than target

This looks reasonable. I'm not sure why we haven't hoisted that HOST_NSPR_MDCPUCFG bit into config.mk or somewhere like that. I'm also not sure if you want to parrot the .NOTPARALLEL bit from a similar makefile:
http://mxr.mozilla.org/mozilla-central/source/modules/libmar/src/Makefile.in#75

I know we're not actually building with -save-temps anywhere anymore, so maybe it doesn't matter.
Attachment #424625 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 11

7 years ago
There was a little bit of churn in this area of code with the changes for bug 542222, so I'm going to run this through the tryserver again with .NOTPARALLEL set (in progress) before landing.
(Assignee)

Comment 12

7 years ago
Comment on attachment 424625 [details] [diff] [review]
[checked in] Compile bsdiff for host OS rather than target

http://hg.mozilla.org/mozilla-central/rev/e3ae391d249f
Attachment #424625 - Attachment description: Compile bsdiff for host OS rather than target → [checked in] Compile bsdiff for host OS rather than target
(Assignee)

Comment 13

7 years ago
Created attachment 427840 [details] [diff] [review]
[checked in][1.9.2] Compile bsdiff for host OS rather than target

Carrying forward review from Ted, this just needs to land on the 1.9.2 branch as well.
Attachment #427840 - Flags: review+
Attachment #427840 - Flags: approval1.9.2.2?
(Assignee)

Comment 14

7 years ago
Created attachment 427841 [details] [diff] [review]
[checked in][1.9.1] Compile bsdiff for host OS rather than target

Carrying forward review from Ted, this just needs to land on the 1.9.1 branch as well.
Attachment #427841 - Flags: review+
Attachment #427841 - Flags: approval1.9.1.9?
Comment on attachment 424625 [details] [diff] [review]
[checked in] Compile bsdiff for host OS rather than target

>+ifneq (,$(filter WINCE WINNT,$(OS_ARCH)))
$(HOST_OS_ARCH)

Guess who cross-compiles around here ;-)
Attachment #427841 - Flags: approval1.9.1.9? → approval1.9.1.9+
Comment on attachment 427841 [details] [diff] [review]
[checked in][1.9.1] Compile bsdiff for host OS rather than target

Approved for 1.9.1.9, a=dveditz for release-drivers
Comment on attachment 427840 [details] [diff] [review]
[checked in][1.9.2] Compile bsdiff for host OS rather than target

Approved for 1.9.2.2, a=dveditz for release-drivers
Attachment #427840 - Flags: approval1.9.2.2? → approval1.9.2.2+
Marking bug FIXED on mozilla-centreal per comment 12. Branch status we track with the status1.9.1/status1.9.2 fields
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Resolution: --- → FIXED
(Assignee)

Comment 19

7 years ago
Comment on attachment 427840 [details] [diff] [review]
[checked in][1.9.2] Compile bsdiff for host OS rather than target

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8f07bc479476
Attachment #427840 - Attachment description: [1.9.2] Compile bsdiff for host OS rather than target → [checked in][1.9.2] Compile bsdiff for host OS rather than target
(Assignee)

Comment 20

7 years ago
Comment on attachment 427841 [details] [diff] [review]
[checked in][1.9.1] Compile bsdiff for host OS rather than target

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/90fbd13348e0
Attachment #427841 - Attachment description: [1.9.1] Compile bsdiff for host OS rather than target → [checked in][1.9.1] Compile bsdiff for host OS rather than target
(Assignee)

Comment 21

7 years ago
(In reply to comment #15)
> (From update of attachment 424625 [details] [diff] [review])
> >+ifneq (,$(filter WINCE WINNT,$(OS_ARCH)))
> $(HOST_OS_ARCH)

Fix included on branch landings, will land on m-c shortly.
status1.9.1: wanted → .9-fixed
status1.9.2: wanted → .2-fixed
(Assignee)

Comment 22

7 years ago
(In reply to comment #21)
> Fix included on branch landings, will land on m-c shortly.

...and done:

http://hg.mozilla.org/mozilla-central/rev/ebdeb3b29736
Depends on: 547955
You need to log in before you can comment on or make changes to this bug.