Closed
Bug 469654
Opened 12 years ago
Closed 11 years ago
Windows x64 build support
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: m_kato, Assigned: m_kato)
References
()
Details
Attachments
(2 files, 5 obsolete files)
11.76 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
11.54 KB,
patch
|
Details | Diff | Splinter Review |
Current configure.in works x86 only. I will merge Windows x64 config.
Assignee | ||
Updated•12 years ago
|
Blocks: tracking_win64
Assignee | ||
Comment 1•12 years ago
|
||
MOZ_PATH_PROGS cannot handle a path which has space character such as "Program Files (x86)". So after resolving it, I will send review.
Comment 2•12 years ago
|
||
The MOZ_PATH_PROGS bit should be fixed by bug 485264.
Assignee | ||
Comment 3•12 years ago
|
||
Thanks, Ted. unfortunately, this still occurs even if applying bug 485264 (see bug 485264 comment #3).
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #370579 -
Attachment is obsolete: true
Attachment #374204 -
Flags: review?(ted.mielczarek)
Comment 5•12 years ago
|
||
I like most of this patch, but I'm not sure how I feel about the js6450 bits, as rational as that change is. I know the 32 is purely historical at this point, and our Mac/Linux builds are just libmozjs. Brendan: is this the right way to go for Windows x64, naming the library js6450.dll?
Comment 6•12 years ago
|
||
Argh, what is there to say? js3250.dll is named based on Netscape 5! Unless there is an ABI issue I don't know about, we should rename to whatever makes sense. /be
Updated•12 years ago
|
Attachment #374204 -
Flags: review?(ted.mielczarek) → review-
Comment 8•12 years ago
|
||
Comment on attachment 374204 [details] [diff] [review] patch v1 + if test "$AS_BIN"; then + AS="$(basename "$AS_BIN")" + fi What's the purpose of this change? + AC_MSG_ERROR([If you build $target, you have to set 32-bit toolchain to your PATH, instead of 64-bit version.]) I think this needs to read like: "You are targeting i386 but using the 64-bit compiler." Most of this patch looks good, but per brendan above and bsmedberg (on irc), I'd like you to just rename js3250.dll to mozjs.dll. This should be as simple as removing the LIBRARY_NAME bit from js/src/Makefile.in and fixing up MOZ_JS_LIBS in configure.
Assignee | ||
Comment 9•12 years ago
|
||
(In reply to comment #8) > (From update of attachment 374204 [details] [diff] [review]) > + if test "$AS_BIN"; then > + AS="$(basename "$AS_BIN")" > + fi > > What's the purpose of this change? Because the result of "MOZ_PATH_PROGS(AS, $AS as, $CC)" becomes that AS is "$(CC)". Even if x86 msvc, this is same result (AS = $(CC)). (See your autoconf.mk of Win32 target.) Also, this hack is used on WINCE target, too. Win32 target of VC++ doesn't use command line assembler (ml.exe), so build error doesn't occur. > > + AC_MSG_ERROR([If you build $target, you have to set 32-bit > toolchain to your PATH, instead of 64-bit version.]) > > I think this needs to read like: > "You are targeting i386 but using the 64-bit compiler." OK. I will change this. > Most of this patch looks good, but per brendan above and bsmedberg (on irc), > I'd like you to just rename js3250.dll to mozjs.dll. This should be as simple > as removing the LIBRARY_NAME bit from js/src/Makefile.in and fixing up > MOZ_JS_LIBS in configure. Should I change it for WINCE, too?
Comment 10•12 years ago
|
||
Ok, sounds reasonable. (In reply to comment #9) > Should I change it for WINCE, too? Yes. The binary should be mozjs on all platforms.
Assignee | ||
Comment 11•12 years ago
|
||
Attachment #374204 -
Attachment is obsolete: true
Comment 12•12 years ago
|
||
Your removed-files.in changes are missing a line to remove the old js3250.dll, and also I suspect you also want to try to remove both jsd3250.dll and jsd6450.dll, so that a 64 bit libxul installer can pave over both a 64 bit and a 32 bit non-libxul install.
Comment 13•12 years ago
|
||
And for that matter, for non-libxul (and thus Tb/SM) it needs to remove the other jsd, making me think it might be a nice time for jsd to drop the MOZ_BITS bit and just have a single name.
Comment 14•12 years ago
|
||
MOZ_BITS die die die! No mo 16 vs. 32 bit memories. Calgon, take me awayyy! ;-) /be
Assignee | ||
Comment 15•12 years ago
|
||
(In reply to comment #14) > MOZ_BITS die die die! No mo 16 vs. 32 bit memories. Calgon, take me awayyy! ;-) > > /be OS_BITS is using filename of self exec installer "firefox-<version>.<lang>.win<OS_BITS>.installer.exe". If we decide that official filename of installer for x64, (ex. firefox-<version>.<lang>.win-x64.exe), I can change this and may remove OS_BITS.
Assignee | ||
Updated•12 years ago
|
Hardware: x86 → x86_64
Comment 17•12 years ago
|
||
(In reply to comment #16) > Created an attachment (id=390385) [details] > patch v2.1 Note that this will fail make check due to changing config/rules.mk but not js/src/config/rules.mk. Otherwise it seems to build fine for me. Or maybe it's something else up my patch queue :)
Assignee | ||
Comment 18•12 years ago
|
||
(In reply to comment #17) > Note that this will fail make check due to changing config/rules.mk but not > js/src/config/rules.mk. Otherwise it seems to build fine for me. Or maybe > it's something else up my patch queue :) Mook, what compiler and version do you use?
Comment 19•12 years ago
|
||
(In reply to comment #18) > (In reply to comment #17) > > > Note that this will fail make check due to changing config/rules.mk but not > > js/src/config/rules.mk. Otherwise it seems to build fine for me. Or maybe > > it's something else up my patch queue :) > > Mook, what compiler and version do you use? mingw-w64 (gcc) off svn trunk, with a few local patches, cross compiled from 32bit msys (but with the binaries copied to look like a native compiler). I can also try building with msvc8 and msvc9 if desired, but that's not my main working environment :)
Assignee | ||
Comment 20•12 years ago
|
||
update by Mook comment.
Attachment #390385 -
Attachment is obsolete: true
Comment 21•12 years ago
|
||
Makoto, please add review tag and ted as reviewer
Assignee | ||
Updated•12 years ago
|
Attachment #398102 -
Flags: review?(ted.mielczarek)
Comment 22•12 years ago
|
||
Ted, any chances you could review this soon?
Comment 23•12 years ago
|
||
Yes, I'll clean out my review queue this week. Sorry for the delay.
Comment 24•12 years ago
|
||
Comment on attachment 398102 [details] [diff] [review] patch v2.2 +++ b/browser/installer/windows/packages-static [xpcom] -bin\js3250.dll +bin\mozjs.dll I bitrotted this in bug 511642. On the plus side, all you have to do now is remove the ifdef here: http://mxr.mozilla.org/mozilla-central/source/browser/installer/package-manifest.in#43 + case "${host_cpu}" in + x86_64) + HOST_CFLAGS="$HOST_CFLAGS -D_AMD64_" + ;; + esac Is this really necessary? Where are we relying on this define? (And what compilers produce it?) + if test "$HAVE_64BIT_OS"; then + AC_DEFINE(_WIN64) + fi I guess this is just for consistency (since we define _WIN32)? --- a/embedding/config/basebrowser-win Nice of you to patch it, although I'm guessing it's painfully out of date at this point anyway. Looks good otherwise, sorry for the delay!
Attachment #398102 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 25•12 years ago
|
||
This patch depends on comm-central, so after I get approve of all patches (I will post new patch to other bugs soon), I will land this. (In reply to comment #24) > (From update of attachment 398102 [details] [diff] [review]) > +++ b/browser/installer/windows/packages-static > [xpcom] > -bin\js3250.dll > +bin\mozjs.dll > > I bitrotted this in bug 511642. On the plus side, all you have to do now is > remove the ifdef here: > http://mxr.mozilla.org/mozilla-central/source/browser/installer/package-manifest.in#43 I will sync this. > + case "${host_cpu}" in > + x86_64) > + HOST_CFLAGS="$HOST_CFLAGS -D_AMD64_" > + ;; > + esac > > Is this really necessary? Where are we relying on this define? (And what > compilers produce it?) This define is for NSPR. NSPR header needs _AMD64_. > + if test "$HAVE_64BIT_OS"; then > + AC_DEFINE(_WIN64) > + fi > > I guess this is just for consistency (since we define _WIN32)? If anyone creates IA64 build, this will be needed. > > --- a/embedding/config/basebrowser-win > > Nice of you to patch it, although I'm guessing it's painfully out of date at > this point anyway. I think so. But, until we remove it, we should consider this.
Assignee | ||
Comment 26•11 years ago
|
||
Comment 27•11 years ago
|
||
You may need put "checkin-needed" into keywords section to make your patch get checked in.
Assignee | ||
Comment 28•11 years ago
|
||
landed http://hg.mozilla.org/mozilla-central/rev/9c2ef289c411
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Flags: in-testsuite-
Target Milestone: --- → mozilla1.9.3a1
Version: unspecified → Trunk
Updated•3 years ago
|
Product: Core → Firefox Build System
Comment 29•3 years ago
|
||
This only replaces the tunnel using a new nr_socket implementation. The actual proxy detection code has not been touched. The proxy url is retrieved but address resolution is done by necko code in the parent process. The new socket wraps a WebrtcProxyChannel which uses necko to handle proxy negotiation. This has a happy side effect of enabling all authentication modes that necko already supports for http proxies.
Updated•3 years ago
|
Attachment #8984333 -
Attachment description: part 2. replace proxy tunnel with new ipc-based tunnel → accidental upload
Attachment #8984333 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•