SpiderMonkey configure with --enable-ctypes and --enable-posix-nspr-emulation used together should error out

ASSIGNED
Assigned to

Status

()

Core
JavaScript Engine
P3
normal
ASSIGNED
10 months ago
9 months ago

People

(Reporter: ptomato, Assigned: arai)

Tracking

52 Branch
Points:
---

Firefox Tracking Flags

(firefox57 fix-optional)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 months ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce:

Run js/src/configure with --enable-ctypes and --enable-posix-nspr-emulation


Actual results:

In file included from /<<PKGBUILDDIR>>/js/src/ctypes/Library.cpp:9:0:
/<<PKGBUILDDIR>>/js/src/dist/system_wrappers/prerror.h:3:15: fatal error: prerror.h: No such file or directory
 #include_next <prerror.h>
               ^~~~~~~~~~~
In file included from /<<PKGBUILDDIR>>/js/src/ctypes/CTypes.h:15:0,
                 from /<<PKGBUILDDIR>>/js/src/ctypes/CTypes.cpp:7:
/<<PKGBUILDDIR>>/js/src/dist/system_wrappers/prlink.h:3:15: fatal error: prlink.h: No such file or directory
 #include_next <prlink.h>
               ^~~~~~~~~~


Expected results:

It looks like ctypes has a hard dependency on NSPR, so configure should not allow those two switches to be used together.
(Assignee)

Comment 1

10 months ago
Created attachment 8901614 [details] [diff] [review]
Throw error when --enable-ctypes and --enable-posix-nspr-emulation are used together.

Added option declaration for --enable-posix-nspr-emulation in js/moz.configure,
and now it throws if --enable-ctypes and --enable-posix-nspr-emulation are used together.

--enable-posix-nspr-emulation is moved from old.configure, since old_configure_options is executed after js/moz.configure, and we cannot use @depends for 
--enable-posix-nspr-emulation in js/moz.configure unless moving it.

the actual configure script for --enable-posix-nspr-emulation is not touched.


here's output:

$ ../js/src/configure --enable-ctypes --enable-posix-nspr-emulation
Creating Python environment
...
checking whether the C++ compiler supports -Wno-noexcept-type... no
Traceback (most recent call last):
  File "../js/src/../../configure.py", line 124, in <module>
...
  File ".../mozilla-unified/build/moz.configure/util.configure", line 19, in configure_error
    raise ConfigureError(message)
mozbuild.configure.ConfigureError: --enable-ctypes and --enable-posix-nspr-emulation cannot be used together
Assignee: nobody → arai.unmht
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8901614 - Flags: review?(sphink)
Comment on attachment 8901614 [details] [diff] [review]
Throw error when --enable-ctypes and --enable-posix-nspr-emulation are used together.

Review of attachment 8901614 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, but you'll need a build peer for this review.
Attachment #8901614 - Flags: review?(sphink) → review?(mh+mozilla)

Comment 3

10 months ago
Comment on attachment 8901614 [details] [diff] [review]
Throw error when --enable-ctypes and --enable-posix-nspr-emulation are used together.

Review of attachment 8901614 [details] [diff] [review]:
-----------------------------------------------------------------

You're not actually moving the option off old-configure, which means it doesn't actually work.

In fact, if you run configure with the patch applied, this is what happens:

js/src> ERROR: Missing option in `@old_configure_options` in /home/glandium/gecko/build/moz.configure/old.configure: --enable-posix-nspr-emulation
Attachment #8901614 - Flags: review?(mh+mozilla) → review-
(Assignee)

Updated

9 months ago
Flags: needinfo?(arai.unmht)
status-firefox57: --- → fix-optional
Priority: -- → P3
(Assignee)

Comment 4

9 months ago
thank you for reviewing, and sorry for being too late to reply.

do you think we should move whole --enable-posix-nspr-emulation related things from build/autoconf/nspr-build.m4 to python before doing this?
(looks like JS_POSIX_NSPR there depends on so many things in the file)
or can we add exception for --enable-posix-nspr-emulation into old_configure function?
Flags: needinfo?(arai.unmht) → needinfo?(mh+mozilla)
Ideally, you should move the whole thing, but the shortcut would be to set JS_POSIX_NSPR with add_old_configure_assignment (and avoid nspr-build.m4 overriding that with JS_POSIX_NSPR=unset)
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.