Change BZIP2 variable name for configure script

[eborisch (Eric A. Borisch)]

As the configure script now calls bzip2 directly, having BZIP2 set during configure (as required to explicitly set the BZIP2 path for MP to use) causes bzip2 (when run by configure) to fail, as bzip2 uses $BZIP2 as an argument: ​https://www.opensource.apple.com/source/bzip2/bzip2-16.5/bzip2/bzip2.txt )

I set BZIP2 to call my own installed pbzip2 (because it makes anything locally compressed and then extracted -- for example during port activations -- go faster.) This actually works because pbzip2 doesn't use BZIP2. But setting the BZIP2 path via ./configure BZIP2=/usr/bin/bzip2 or BZIP2=/usr/bin/bzip2 ./configure will fail on a fresh copy (or distclean-ed) of base.

[ryandesign (Ryan Carsten Schmidt)]

It would be more customary if the path to the bzip2 program could be specified by a configure argument (e.g. --with-bzip2=...) instead of an environment variable.

[ryandesign (Ryan Carsten Schmidt)]

Cc Me!

[neverpanic (Clemens Lang)]

If we did that for all our incovations of AC_ARG_VAR that would add another 17 possible arguments. I don't think this is the way to go.

The list would be:

• BSDMAKE
• BZIP2
• CVS
• GNUMAKE
• GNUTAR
• LZMA
• MAKE
• MTREE
• OPEN
• OPENSSL
• RSYNC
• SED
• SVN
• SWIG
• TAR
• XAR
• XZ

[eborisch (Eric A. Borisch)]

Most applications (wild speculation; haven't run into others, I'm sure there are some out there) don't use an environment variable sharing their name as an argument.

Perhaps this tweak aclocal.m4 is lower impact:

aclocal.m4

@@ -138 +138 @@
                                 AC_MSG_ERROR([Don't know how to extract tarball $mp_tarball])
                                 ;;
                 esac
+    # bzip2 uses the BZIP2 environment variable as an argument. Clear it out during execution.
+    BZIP2_SAVE=$BZIP2
+    BZIP2=
                 (cd "$mp_tarball_vendordir"; "$mp_tarball_extract_cmd" -d < "$ac_abs_confdir/$mp_tarball" | tar xf - || AC_MSG_ERROR([failed to extract $mp_tarball]))
+    BZIP2=$BZIP2_SAVE
         fi
         if ! test -d "$ac_dir"; then
                 AC_MSG_ERROR([tarball $mp_tarball did not extract to $ac_dir])

The only side effect here is an extra BZIP2_SAVE variable after it has been evauated.

[eborisch (Eric A. Borisch)]

Also related: there is some discussion going on in the users list -- I didn't start it ;) -- about using something other than vanilla bzip2. ​https://lists.macosforge.org/pipermail/macports-users/2014-April/035115.html

[neverpanic (Clemens Lang)]

I'd rather go with:

aclocal.m4

@@ -133 +133 @@
                                         AC_MSG_ERROR([bzip2 not found])
                                 fi
                                 mp_tarball_extract_cmd="$BZIP2"
+                                export -n BZIP2
                                 ;;
                         *)
                                 AC_MSG_ERROR([Don't know how to extract tarball $mp_tarball])

which should get the job done aswell.

[eborisch (Eric A. Borisch)]

Works for me. I can commit this up, if you like.

[neverpanic (Clemens Lang)]

This has already been fixed in r118736.

[eborisch (Eric A. Borisch)]

That doesn't actually fix it -- it still fails on the first (taken from BZIP2 env) argument and then decompresses the second. It still fails when used with lbzip2 (which emulates bzip2's usage of the BZIP2 variable) which bails when asked to decompress a non-bzip2 file.

I would suggest revert r118736 (or don't -- either way works) and use the export -n BZIP2 change above. (Potentially followed by a re-export, if you really want to limit the impact of the change.)

make distclean
BZIP2=/usr/bin/bzip2 ./configure
[...]
configure: === extracting vendor/tclx8.4.1.tar.bz2
bzip2: /usr/bin/bzip2 is not a bzip2 file.

[neverpanic (Clemens Lang)]

Yeah, I thought the "fix" wasn't particularly robust, so feel free to add the export -n BZIP2 line as well, probably as (export -n BZIP2) >/dev/null 2>&1 && export -n BZIP2 to avoid errors in shells that don't support export -n.

Don't forget to run ./regen.sh after changing aclocal.m4.

[cooljeanius (Eric Gallager)]

Cc Me!

[eborisch (Eric A. Borisch)]

Going through it, I think I prefer the configure arguments for paths. I'm not convinced that 17 extra arguments is any more or less complex than 17 environment variables. Given that the ENVs get exported to anything running underneath, the pure arguments are in a sense less complex.

Here's a patch that will (after running through ./regen.sh) update configure to have --with-tool=path arguments. This fixes the exported BZIP2 variable tripping up bzip2 issue, as well.

This patch also backs-out r118736; this could be done separately for tracking, of course. (Or not at all, either works, I was just trying to minimize total changes vs -r old for this ticket.)

Note that anyone performing TOOL=PATH ./configure will still get (essentially) the previous behavior -- so this shouldn't break anyone's custom build that was relying on setting the tool paths in environment variables. This occurs because $with_tool will be unset, and AC_PATH_PROG(TOOL, ...) will be overridden if the variable TOOL is set. (They may not now be marked as precious, but that's another level of autoconf grokking that I'm not at...)

aclocal.m4

@@ -96 +96 @@
         $1=$(AS_ECHO([$2]) | sed -E 's/^--?([[^=]]+)=.*$/\1/')dnl
 ])
 
+dnl Similar to AC_ARG_VAR, but implemented with --with-pkg=PATH for PKG.
+dnl
+dnl Parameters: Similar to AC_ARG_VAR ($2 gets some boiler plate around it)
+AC_DEFUN([MP_TOOL_PATH], [dnl
+        AC_ARG_WITH([m4_tolower($1)], [AS_HELP_STRING([--with-m4_tolower($1)=PATH],
+                        [Path to alternate $2 command])],dnl
+                [$1=$withval],dnl
+                [])dnl
+])
+
 dnl Configure a project contained in a .tar.gz, .tgz or .tar.bz2 tarball,
 dnl extracting it previously, if necessary. Different from AC_CONFIG_SUBDIRS
 dnl (on which this macro is based), you can pass parameters to the
@@ -138 +148 @@
                                 AC_MSG_ERROR([Don't know how to extract tarball $mp_tarball])
                                 ;;
                 esac
-                (cd "$mp_tarball_vendordir"; "$mp_tarball_extract_cmd" -dc "$ac_abs_confdir/$mp_tarball" | tar xf - || AC_MSG_ERROR([failed to extract $mp_tarball]))
+                (cd "$mp_tarball_vendordir"; "$mp_tarball_extract_cmd" -d < "$ac_abs_confdir/$mp_tarball" | tar xf - || AC_MSG_ERROR([failed to extract $mp_tarball]))
         fi
         if ! test -d "$ac_dir"; then
                 AC_MSG_ERROR([tarball $mp_tarball did not extract to $ac_dir])

configure.ac

@@ -101 +101 @@
 AC_PROG_MAKE_SET
 AC_PROG_OBJC([clang cc gcc])
 
+# Check for user-supplied paths before searching
+MP_TOOL_PATH(BSDMAKE, [bsdmake/pmake])
+MP_TOOL_PATH(BZIP2, [bzip2])
+MP_TOOL_PATH(CVS, [cvs])
+MP_TOOL_PATH(GNUMAKE, [gnumake])
+MP_TOOL_PATH(GNUTAR, [gnutar])
+MP_TOOL_PATH(LZMA, [lzma])
+MP_TOOL_PATH(MAKE, [make])
+MP_TOOL_PATH(MTREE, [mtree])
+MP_TOOL_PATH(OPEN, [open])
+MP_TOOL_PATH(OPENSSL, [openssl])
+MP_TOOL_PATH(RSYNC, [rsync])
+MP_TOOL_PATH(SED, [sed])
+MP_TOOL_PATH(SVN, [svn])
+MP_TOOL_PATH(SWIG, [swig])
+MP_TOOL_PATH(TAR, [tar])
+MP_TOOL_PATH(XAR, [xar])
+MP_TOOL_PATH(XZ, [xz])
+
+# Search for tool paths. Any set above (via --with-pkg=PATH) are retained
 AC_PATH_PROGS(BSDMAKE, [bsdmake pmake], [])
 AC_PATH_PROG(BZIP2, [bzip2], [])
 AC_PATH_PROG(BZR, [bzr], [])
@@ -149 +169 @@
 
 AC_CHECK_PROG(HAVE_LAUNCHD, [launchd], [yes], [], [/sbin])
 
-# Define some precious variables allowing user to override PATH for some programs
-AC_ARG_VAR(BSDMAKE, [path to bsdmake/pmake command])
-AC_ARG_VAR(BZIP2, [path to bzip2 command])
-AC_ARG_VAR(CVS, [path to cvs command])
-AC_ARG_VAR(GNUMAKE, [path to gnumake command])
-AC_ARG_VAR(GNUTAR, [path to gnutar command])
-AC_ARG_VAR(LZMA, [path to lzma command])
-AC_ARG_VAR(MAKE, [path to make command])
-AC_ARG_VAR(MTREE, [path to mtree command])
-AC_ARG_VAR(OPEN, [path to open command])
-AC_ARG_VAR(OPENSSL, [path to openssl command])
-AC_ARG_VAR(RSYNC, [path to rsync command])
-AC_ARG_VAR(SED, [path to sed command])
-AC_ARG_VAR(SVN, [path to svn command])
-AC_ARG_VAR(SWIG, [path to swig command])
-AC_ARG_VAR(TAR, [path to tar command])
-AC_ARG_VAR(XAR, [path to xar command])
-AC_ARG_VAR(XZ, [path to xz command])
 
 if test "x$MTREE" = "x"; then
     AC_MSG_ERROR([mtree not found])

Snippet of ./configure --help output now:

Optional Packages:
  --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
  --without-PACKAGE       do not use PACKAGE (same as --with-PACKAGE=no)
  --with-bsdmake=PATH     Path to alternate bsdmake/pmake command
  --with-bzip2=PATH       Path to alternate bzip2 command
  --with-cvs=PATH         Path to alternate cvs command

[eborisch (Eric A. Borisch)]

Committed in r119421:119423.

[jeremyhu (Jeremy Huddleston Sequoia)]

Note that anyone performing TOOL=PATH ./configure will still get (essentially) the previous behavior -- so this shouldn't break anyone's custom build that was relying on setting the tool paths in environment variables. This occurs because $with_tool will be unset, and AC_PATH_PROG(TOOL, ...) will be overridden if the variable TOOL is set. (They may not now be marked as precious, but that's another level of autoconf grokking that I'm not at...)

Oh, so THIS is why my build script hasn't been working right for months...

This actually doesn't do the same thing for people setting environment variables with AC_ARG_VAR. AC_ARG_VAR results in the value being cached whereas this does not.

For example, I check ac_cv_env_BZIP2_value and others to see what base was built with when determining if I need to rebuild base. This new approach never saves those in the autoconf cache, so I end up rebuilding base every time I do an update.