js-ctypes cannot build on Windows x64

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

unspecified
x86_64
Windows NT
Points:
---

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch patch v1 (obsolete) — Splinter Review
FFI_STDCALL isn't defined on Windows x64.  So js-ctypes cannot build on Windows x64.
Attachment #402735 - Flags: review?(dwitte)
Comment on attachment 402735 [details] [diff] [review]
patch v1

>+#if defined(XP_WIN32) && !defined(_WIN64)

Ugh, XP_WIN32 is defined for win64?

Anyway, let's just do #if defined(_WIN32), since that's what we really want. r=me with that.

Does ctypes build and pass tests for you? I haven't built on win64 so I'm curious.

Thanks for the patch!
Attachment #402735 - Flags: review?(dwitte) → review+
Landed on trunk as http://hg.mozilla.org/mozilla-central/rev/6c221a030a78, with the _WIN32 change. (I'll land this fix on branch, too, when I land ctypes there.)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #1)
> (From update of attachment 402735 [details] [diff] [review])
> >+#if defined(XP_WIN32) && !defined(_WIN64)
> 
> Ugh, XP_WIN32 is defined for win64?

Yes.  buildconfig patch is bug 469654. (I have to get approve comm-central's patch to land this buuilgconfig patch for m-c, so I don't land it yet)

If I should add XP_WIN64 macro, I will discuss with Ted.  We have to replace most XP_WIN32 with XP_WIN.

> Does ctypes build and pass tests for you? I haven't built on win64 so I'm
> curious.

Build test is done.  I will create test code for Windows x64.
You can do |make xpcshell-test| in $OBJDIR/js/ctypes/tests; let me know if that works or not.
Landed on 1.9.2 per bug 513783.
(In reply to comment #4)
> You can do |make xpcshell-test|

Er, |make xpcshell-tests|, that is.
I reopen this bug.  Because your check in causes build error on Win x64.

Dan, why do you change to _WIN32 instead of my patch??  _WIN32 means both Win32 and Win64 target.

In reference of http://msdn.microsoft.com/ja-jp/library/b0084kay.aspx,

_WIN32  Defined for applications for Win32 and Win64. Always defined.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Posted patch patch v2Splinter Review
_WIN32 means both Win32 and Win64.  So we need additional condition for Win64.
Attachment #402735 - Attachment is obsolete: true
Attachment #403712 - Flags: review?
Attachment #403712 - Flags: review? → review+
Comment on attachment 403712 [details] [diff] [review]
patch v2

Thanks. I assumed they were mutually exclusive, which obviously they're not!

Does jsctypes pass unit tests (comment 4) with this patch? Let's not land it til we know everything's working.
(In reply to comment #9)
> (From update of attachment 403712 [details] [diff] [review])
> Thanks. I assumed they were mutually exclusive, which obviously they're not!
> 
> Does jsctypes pass unit tests (comment 4) with this patch? Let's not land it
> til we know everything's working.

test is passed.

$ make -C js/ctypes/tests/ xpcshell-tests
make: Entering directory `/c/Workspace/mozilla-hg/objdir-win64/js/ctypes/tests'
c:/mozilla-build/python25/python.exe -u /c/Workspace/mozilla-hg/mozilla-win64/config/pythonpath.py \
          -I/c/Workspace/mozilla-hg/mozilla-win64/build \
          /c/Workspace/mozilla-hg/mozilla-win64/testing/xpcshell/runxpcshelltests.py \
          --symbols-path=../../../dist/crashreporter-symbols \
          ../../../dist/bin/xpcshell \
          ../../../_tests/xpcshell/jsctypes-test/unit
TEST-PASS | c:\Workspace\mozilla-hg\objdir-win64\_tests\xpcshell\jsctypes-test\unit\test_jsctypes.js | test pa
ssed
INFO | Result summary:
INFO | Passed: 1
INFO | Failed: 0
make: Leaving directory `/c/Workspace/mozilla-hg/objdir-win64/js/ctypes/tests'
Attachment #403712 - Flags: approval1.9.2?
Product: Other Applications → Core
Version: Trunk → unspecified
http://hg.mozilla.org/mozilla-central/rev/4fae2bb3a717

Thanks for the (second!) fix.
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Attachment #403712 - Flags: approval1.9.2? → approval1.9.2+
You need to log in before you can comment on or make changes to this bug.