autoconf: Port NSPR's build/autoconf/acwinpaths.m4 to autoconf 2.56+

RESOLVED FIXED in 4.10.5

Status

P2
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: wtc, Assigned: wtc)

Tracking

other
4.10.5

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 722612 [details] [diff] [review]
Remove NSPR's build/autoconf/acwinpaths.m4 from NSPR upstream

Ben, Ted:

In NSPR bug 524785, you added mozilla/nsprpub/build/autoconf/acwinpaths.m4
to allow running NSPR's configure script with a full path name containing
a Windows drive letter, for example,
    d:/nspr-src/mozilla/nsprpub/configure

Last night I tried to make NSPR's configure.in work with the current
version of autoconf. This is the first problem I ran into.

wtc@fips:~/nspr-tip.3/mozilla/nsprpub$ autoconf2.50 
build/autoconf/acwinpaths.m4:10: error: defn: undefined macro: AC_OUTPUT_FILES
build/autoconf/acwinpaths.m4:10: the top level
autom4te: /usr/bin/m4 failed with exit status: 1

I found that build/autoconf/acwinpaths.m4 was custom-written to
patch the internal m4 macros of autoconf 2.13. I tried to find the
equivalent internal macros of the current version of autoconf, but
could not make it work. I gave up after two hours.

I would appreciate it if you could port build/autoconf/acwinpaths.m4
to the current version of autoconf.  Until it's ported, I'd like
to back out your patch from NSPR upstream, and maintain it as a
private patch in the copy of NSPR in mozilla-central.
(Assignee)

Comment 1

6 years ago
Comment on attachment 722612 [details] [diff] [review]
Remove NSPR's build/autoconf/acwinpaths.m4 from NSPR upstream

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

::: mozilla/nsprpub/aclocal.m4
@@ -3,4 @@
>  dnl The contents of this file are under the Public Domain.
>  dnl 
>  
> -builtin(include, build/autoconf/acwinpaths.m4)

Ben, Ted: is it possible to only execute this 'builtin' directive
if the autoconf version is 2.13? Does m4 allow us to do that?

That would be a better interim solution than maintaining this as
a private patch in mozilla-central. Thanks.
(Assignee)

Comment 2

5 years ago
Created attachment 8384153 [details] [diff] [review]
Patch for nspr/configure generated by autoconf 2.56

I don't know how to port nspr/build/autoconf/acwinpaths.m4
to autoconf 2.56, but I figured out how to manually fix up
the nspr/configure file generated by autoconf 2.56. I verified
that the makefiles work under both GNU make and pymake.

I am planning to maintain this patch in the NSPR upstream,
so that I can update nspr/configure.in to autoconf 2.56.

In the next comment I will explain why I think this patch
is sufficient.
Attachment #8384153 - Flags: review?(ted)
Attachment #8384153 - Flags: feedback?(benjamin)
(Assignee)

Comment 3

5 years ago
Comment on attachment 8384153 [details] [diff] [review]
Patch for nspr/configure generated by autoconf 2.56

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

::: configure.orig
@@ +11430,5 @@
>         ac_top_srcdir=`echo $ac_top_builddir | sed 's,/$,,'`
>      fi ;;
>    [\\/]* | ?:[\\/]* )  # Absolute path.
>      ac_srcdir=$srcdir$ac_dir_suffix;
>      ac_top_srcdir=$srcdir ;;

The nspr/build/autoconf/acwinpaths.m4 changeset results in
two changes to nspr/configure:
https://hg.mozilla.org/projects/nspr/rev/599ac415c10d

The first change is to change the case for an absolute path from
  /*)
to
  /* | ?:/*)

I believe the equivalent code in the autoconf-2.56 generated
configure file is here, and it already handles a DOS drive letter
in an absolute path.

@@ +11468,1 @@
>      for f in $ac_file_in; do

The second change is:

-  ac_file_inputs=`echo $ac_file_in|sed -e "s%^%$ac_given_srcdir/%" -e "s%:% $ac_given_srcdir/%g"`
+  ac_file_inputs=`echo $ac_file_in|sed -e "s%^%$ac_given_srcdir/%" `

I believe the equivalent code is here, and the new code uses IFS=:
to accomplish the sed -e "s%:% $ac_given_srcdir/%g" command. So my
patch removes IFS=: from the new code.

It seems that pymake doesn't support an MSYS-style absolute path
like /d/foo/bar (for D:\foo\bar), so we can't avoid hacking autoconf
like this.
(Assignee)

Comment 4

5 years ago
Created attachment 8384169 [details] [diff] [review]
Use new forms of the AC_INIT and AC_OUTPUT macros

The old forms of AC_INIT and AC_OUTPUT still work but are deprecated.
I replaced them with the new forms.

I also found out why autoconf needs to set IFS to ':' -- a colon can
be used to specify alternative input file names in AC_CONFIG_FILES:
https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Configuration-Files.html#Configuration-Files
Attachment #8384169 - Flags: review?(ted)
(Assignee)

Comment 5

5 years ago
Created attachment 8384181 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56

I figured out how to do this.

The AC_OUTPUT_HEADER and AC_OUTPUT_FILES functions
in autoconf 2.13 became _AC_OUTPUT_HEADERS and
_AC_OUTPUT_FILES functions in autoconf 2.56. NSPR
only uses _AC_OUTPUT_FILES, so I only patched that
function.

Here is the diffs between the configure files
generated before and after this patch:

diff -u configure.save configure
--- configure.save      2014-03-01 12:10:43 -0800
+++ configure   2014-03-01 12:10:59 -0800
@@ -11478,7 +11478,7 @@

   # First look for the input files in the build tree, otherwise in the
   # src tree.
-  ac_file_inputs=`IFS=:
+  ac_file_inputs=`#IFS=:
     for f in $ac_file_in; do
       case $f in
       -) echo $tmp/stdin ;;
Attachment #722612 - Attachment is obsolete: true
Attachment #8384181 - Flags: superreview?(benjamin)
Attachment #8384181 - Flags: review?(ted)
(Assignee)

Comment 6

5 years ago
Created attachment 8384196 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56, v2

I found that the _AC_OUTPUT_FILES macro that we're
trying to patch was removed and the bug was fixed in
autoconf 2.60. It doesn't seem possible to get the
autoconf version before 2.62, so I make the workaround
conditional on the _AC_OUTPUT_FILES macro being defined.

When MozillaBuild includes autoconf 2.60+, we can do
AC_PREREQ(2.60) and remove this workaround.

I also added datarootdir to nspr/config/autoconf.mk.in
because the 'datadir' variable is now dependent on
datarootdir, and autoconf warns if the makefile uses
datadir but not datarootdir. See
http://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Changed-Directory-Variables.html
Attachment #8384181 - Attachment is obsolete: true
Attachment #8384181 - Flags: superreview?(benjamin)
Attachment #8384181 - Flags: review?(ted)
Attachment #8384196 - Flags: superreview?(benjamin)
Attachment #8384196 - Flags: review?(ted)

Updated

5 years ago
Attachment #8384196 - Flags: superreview?(benjamin) → superreview+

Updated

5 years ago
Attachment #8384153 - Flags: feedback?(benjamin)
(Assignee)

Comment 7

5 years ago
Comment on attachment 8384196 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56, v2

Patch checked in: https://hg.mozilla.org/projects/nspr/rev/73bd99bf6158

I also included the AC_PREREQ(2.56) change in this changeset.
Attachment #8384196 - Flags: checked-in+
(Assignee)

Comment 8

5 years ago
Comment on attachment 8384196 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56, v2

I reverted this patch because I forgot that I haven't fixed
bug 849093: https://hg.mozilla.org/projects/nspr/rev/b7ba947e65a3
Attachment #8384196 - Flags: checked-in+
(Assignee)

Updated

5 years ago
Summary: Make NSPR's build/autoconf/acwinpaths.m4 a private patch in mozilla-central → autoconf: Port NSPR's build/autoconf/acwinpaths.m4 to autoconf 2.56+
(Assignee)

Updated

5 years ago
Attachment #8384153 - Attachment is obsolete: true
Attachment #8384153 - Flags: review?(ted)
(Assignee)

Comment 9

5 years ago
Comment on attachment 8384196 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56, v2

I just fixed bug 849093, so I checked in this patch again:
https://hg.mozilla.org/projects/nspr/rev/4bc1fa513c32
Attachment #8384196 - Flags: checked-in+
(Assignee)

Comment 10

5 years ago
Comment on attachment 8384169 [details] [diff] [review]
Use new forms of the AC_INIT and AC_OUTPUT macros

Ted: ping!

Kai: could you also review this patch?

Please see the AC_INIT and AC_OUTPUT entries in:

http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Obsolete-Macros.html#Obsolete-Macros

for the rewrites in the latest versions of autoconf.
Attachment #8384169 - Flags: review?(kaie)
(Assignee)

Comment 11

4 years ago
Comment on attachment 8384196 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56, v2

This patch (including an NSPR configure script generated with
autoconf 2.68) has been pushed to mozilla-central:
https://hg.mozilla.org/mozilla-central/rev/1bb7e442dfbb
(Assignee)

Comment 12

4 years ago
Created attachment 8398889 [details] [diff] [review]
Replace obsolete autoconf macros as suggested by autoupdate 2.61

Obsolete macros and their replacements are documented in
http://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Obsolete-Macros.html

The only obsolete macro I didn't replace is AC_TRY_COMPILE.
It is used in a lot of the Android code that NSPR shares
with Mozilla's top-level configure.in.
Attachment #8384169 - Attachment is obsolete: true
Attachment #8384169 - Flags: review?(ted)
Attachment #8384169 - Flags: review?(kaie)
Attachment #8398889 - Flags: superreview?(mh+mozilla)
Attachment #8398889 - Flags: review?(cls)
Comment on attachment 8384196 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56, v2

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

::: build/autoconf/acwinpaths.m4
@@ +11,3 @@
>  ])
> +m4_ifdef([_AC_OUTPUT_FILES],
> +         [GENERATE_FILES_NOSPLIT(defn([_AC_OUTPUT_FILES]))])

If you AC_PREREQ(2.61), why do you need this?
Comment on attachment 8398889 [details] [diff] [review]
Replace obsolete autoconf macros as suggested by autoupdate 2.61

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

::: configure.in
@@ +2596,2 @@
>  dnl AC_HEADER_TIME
>  dnl AC_STRUCT_TM

Those are all commented, you might as well remove them all.
Attachment #8398889 - Flags: superreview?(mh+mozilla)
Attachment #8398889 - Flags: review?(cls)
Attachment #8398889 - Flags: review+
(Assignee)

Comment 15

4 years ago
Created attachment 8400154 [details] [diff] [review]
Replace obsolete autoconf macros as suggested by autoupdate 2.61, v2

Mike: I believe those commented-out AC_ macro invocations were
suggested by autoscan when we first created the configure.in
script for NSPR. On your suggestion, I deleted them all.

Patch checked in: https://hg.mozilla.org/projects/nspr/rev/02d42a8e9372
Attachment #8398889 - Attachment is obsolete: true
Attachment #8400154 - Flags: checked-in+
(Assignee)

Comment 16

4 years ago
Comment on attachment 8400154 [details] [diff] [review]
Replace obsolete autoconf macros as suggested by autoupdate 2.61, v2

Forgot to request a post-commit review.
Attachment #8400154 - Flags: review?(mh+mozilla)
(Assignee)

Comment 17

4 years ago
Comment on attachment 8384196 [details] [diff] [review]
Port nspr/build/autoconf/acwinpaths.m4 to autoconf 2.56, v2

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

::: build/autoconf/acwinpaths.m4
@@ +11,3 @@
>  ])
> +m4_ifdef([_AC_OUTPUT_FILES],
> +         [GENERATE_FILES_NOSPLIT(defn([_AC_OUTPUT_FILES]))])

I needed this patch when I only required autoconf 2.56. You're right
that this can be removed now.
(Assignee)

Comment 18

4 years ago
Created attachment 8400159 [details] [diff] [review]
Remove the workaround for a problem in autoconf 2.59 and earlier with splitting a DOS-style absolute path by the colon

This patch removes nspr/aclocal.m4 and nspr/build/autoconf/acwinpaths.m4,
which were added in https://hg.mozilla.org/projects/nspr/rev/599ac415c10d

Patch checked in: https://hg.mozilla.org/projects/nspr/rev/02d42a8e9372
Attachment #8400159 - Flags: review?(mh+mozilla)
Attachment #8400159 - Flags: checked-in+
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.10.5
(Assignee)

Comment 19

4 years ago
Pushed the last two patches to mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9abb2a6237c1
Attachment #8384196 - Flags: review?(ted) → review+
Attachment #8400154 - Flags: review?(mh+mozilla)
Attachment #8400159 - Flags: review?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.