Closed Bug 849089 Opened 7 years ago Closed 7 years ago

Simple changes to make NSPR's configure.in work with the current version of autoconf

Categories

(NSPR :: NSPR, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wtc, Assigned: wtc)

Details

Attachments

(2 files, 2 obsolete files)

The attached patch fixes one problem in the NSPR configure script
generated by the current version of autoconf.

     AC_TRY_COMPILE([], [return(0);], 
         [ac_cv_prog_host_cc_works=1 AC_MSG_RESULT([yes])],
         AC_MSG_ERROR([installation or configuration problem: $host compiler $HOST_CC cannot create executables.]) )

Having
    ac_cv_prog_host_cc_works=1
and
    AC_MSG_RESULT([yes])
on the same line is problematic. This can be fixed by either adding
a semicolon or a new line after
    ac_cv_prog_host_cc_works=1.

Doing a Web search for ac_cv_prog_host_cc_works, I found other
people have created such patches:
https://github.com/GNOME/jhbuild/blob/master/patches/nspr.hostcompiler.patch
https://github.com/Barthalion/aports/blob/master/main/nspr/nspr-bb-shell.patch
http://hg.et.redhat.com/cgi-bin/hg-misc.cgi/fedora-mingw--devel/annotate/c376c13886ec/nspr/nspr-configure-remove-crack.patch

Ted: I have a question for you. These MXR queries seem to show
that ac_cv_prog_host_cc_works is set but not used:

http://mxr.mozilla.org/mozilla-central/search?string=ac_cv_prog_host_cc_works
http://mxr.mozilla.org/mozilla-central/search?string=ac_cv_prog_hostcc_works

Note that in mozilla and js's configure scripts, ac_cv_prog_host_cc_works
is spelled as ac_cv_prog_hostcc_works. Which one is right? Can they be
simply removed?
Attachment #722616 - Flags: review?(ted)
Comment on attachment 722616 [details] [diff] [review]
Add a semicolon after ac_cv_prog_host_cc_works=1

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

Given that nothing is using it, we could probably just remove the ac_cv_prog_host_cc_works variable altogether. I don't see any evidence that either spelling is more correct, FWIW.

Given that the other branch of this conditional is an AC_MSG_ERROR, I don't see what use this variable can possibly have. (You can always assume it's true if you get past this block.)
Attachment #722616 - Flags: review?(ted) → review+
Ted: thanks for the feedback. I agree that we should just
remove ac_cv_prog_host_cc_works=1. The variable name
ac_cv_prog_host_cc_works is apparently a variant of the
ac_cv_prog_cc_works variable used by autoconf.

In fact, the code generated autoconf 2.13 shows it is
not doing what's intended. The line
    [ac_cv_prog_host_cc_works=1 AC_MSG_RESULT([yes])],
becomes
    ac_cv_prog_host_cc_works=1 echo "$ac_t""yes" 1>&6

So ac_cv_prog_host_cc_works is actually set as an environment
variable for that invocation of 'echo' only :-)

In this patch I just remove ac_cv_prog_host_cc_works=1. I also
quote AC_MSG_ERROR as defensive programming. In autoconf 2.50+
it is sometimes more important to quote things:
http://www.gnu.org/software/autoconf/manual/autoconf.html#Changed-Quotation

I verified that the [] quote I added around the AC_MSG_ERROR
does not change the output of autoconf 2.13.
Attachment #722616 - Attachment is obsolete: true
Attachment #722888 - Flags: review?(ted)
Comment on attachment 722888 [details] [diff] [review]
Remove ac_cv_prog_host_cc_works=1

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

Looks good.

::: mozilla/nsprpub/configure.in
@@ +593,4 @@
>      LDFLAGS="$HOST_LDFLAGS"
>  
>      AC_MSG_CHECKING([whether the $host compiler ($HOST_CC $HOST_CFLAGS $HOST_LDFLAGS) works])
> +    AC_TRY_COMPILE([], [return 0;], 

nit: remove the trailing space while you're here?
Attachment #722888 - Flags: review?(ted) → review+
I remove a space at the end of the line that I spotted.

Ted: could you make the same change to mozilla/configure.in
and mozilla/js/src/configure.in? See

http://mxr.mozilla.org/mozilla-central/search?string=ac_cv_prog_hostcc_works
http://mxr.mozilla.org/mozilla-central/search?string=ac_cv_prog_hostcxx_works
Attachment #722888 - Attachment is obsolete: true
Attachment #722890 - Flags: review?(ted)
Comment on attachment 722890 [details] [diff] [review]
Remove ac_cv_prog_host_cc_works=1, v2

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

I can change those but not at the moment. Can you file a bug on that?
Attachment #722890 - Flags: review?(ted) → review+
This patch is only needed on Windows. (The autoconf that comes
with MozillaBuild is version 2.56. The latest version is 2.69.)
Attachment #723029 - Flags: review?(ted)
Comment on attachment 722890 [details] [diff] [review]
Remove ac_cv_prog_host_cc_works=1, v2

Patch committed and pushed to NSPR's new hg repository:
https://hg.mozilla.org/projects/nspr/rev/0e2e5e09fbac
Attachment #722890 - Flags: checked-in+
Attachment #723029 - Flags: review?(ted) → review+
Comment on attachment 723029 [details] [diff] [review]
Quote the AC_CHECK_HEADER call for dlfcn.h

Patch pushed to the NSPR hg repository (NSPR 4.10):
https://hg.mozilla.org/projects/nspr/rev/b116af2df022
Attachment #723029 - Flags: checked-in+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.10
You need to log in before you can comment on or make changes to this bug.