[Patch] Bogus messages from ./configure

[Christian Franke]

Hi,

thanks for providing ddrutility. I maintain the ddrescue package for the Cygwin distro. A test build of ddrutility on Cygwin results in the following bogus messages from ./configure script:

$ ./configure
...
./configure: line 2272: LF_CONFIGURE_CC: command not found
./configure: line 2273: LF_CONFIGURE_CXX: command not found
./configure: line 2274: LF_HOST_TYPE: command not found
./configure: line 2275: LF_SET_WARNINGS: command not found
...

This is because configure.ac use the LF_* macros from autotoolset (https://sourceforge.net/projects/autotoolset/, 11 years old ?-). These macros were apparently missing during packaging and were therefore not expanded. Fortunately these are not needed. The following fix works for me to build on Cygwin:

--- ddrutility-2.5.orig/configure.ac    2014-06-08 19:03:10.000000000 +0200
+++ ddrutility-2.5/configure.ac 2014-07-28 13:45:40.744701400 +0200
@@ -16,17 +16,10 @@
AM_CONFIG_HEADER(config.h)
AM_INIT_AUTOMAKE([dist-bzip2])

-
-LF_CONFIGURE_CC
-LF_CONFIGURE_CXX
-LF_HOST_TYPE
-LF_SET_WARNINGS
-AC_PROG_RANLIB
-
-AC_PROG_CC
-
AC_PATH_PROGS(BASH, bash sh)

+AC_CHECK_LIB([iconv], [libiconv_open])
+
 AC_CONFIG_FILES([
    Makefile
    README

AC_CHECK_LIB is needed for Cygwin because -liconv is not linked by default.

Two unrelated suggestions:

The generated shebang in ddru_findbad contains 2 blanks which may be non-portable. Possible fix:

--- ddrutility-2.5.orig/src/Makefile.am 2014-06-08 19:03:10.000000000 +0200
+++ ddrutility-2.5/src/Makefile.am      2014-07-28 13:31:05.874280700 +0200
@@ -18,8 +18,6 @@
EXTRA_DIST = ddru_findbad.sh

ddru_findbad: $(srcdir)/ddru_findbad.sh
-       rm -f ddru_findbad
-       echo "#! " $(BASH) > ddru_findbad
-       cat $(srcdir)/ddru_findbad.sh | tail -n +2 >> ddru_findbad
+       sed '1s|/bin/bash|$(BASH)|' $(srcdir)/ddru_findbad.sh > ddru_findbad
        chmod ugo+x ddru_findbad

The m4/Makefile* could possibly be removed. The only effect is that "make install" creates a bogus empty directory /usr/{,local/}share/aclocal.

Thanks,
Christian

[maximus57]

This is because configure.ac use the LF_* macros from autotoolset 

I have made these changes you asked for, and did not see any problems building or compiling. I am attaching a test version with the changes if you would like to test with. I would caution actually using this for an actual build as I can not guarantee that I did not work on or change any of the source files since the last build (don't think I did, but I did not check before building this test).

The generated shebang in ddru_findbad contains 2 blanks which may be non-portable. 

Are you referring to extra spaces that should not be there? How about I just change the one line to:

echo "#!"$(BASH) > ddru_findbad

That gets rid of the spaces. I don't like using sed in cases like this as it is not always portable (different version of sed that act differently). I do admit that I could likely get rid of the "rm -f ddru_findbad" line as the next line should overwrite it, but maybe I put it there for a reason I don't remember.

The m4/Makefile* could possibly be removed.

Just tried to remove it, and it breaks the build process. So if it is not hurting anything, it stays.

[Christian Franke]

Are you referring to extra spaces that should not be there?

The original echo command produced two(!) spaces (one from the the quote, one from echo arg separator) in the shebang which might not be portable. One space is OK, none also.

I don't like using sed in cases like this as it is not always portable

The suggested sed command should work even on historic unices, which means: before linux (1991) and C++ (1986) were introduced :-). For other portable examples, see the 80+ sed commands in the generated ./configure script.

Rethinking this issue I would now suggest to remove the ddru_findbad Makefile.am target (and the related configure.ac check) and do the following instead:

• If the script is fully standard shell compatible, use a fixed /bin/sh in the shebang. A compatible shell must exist on each system as /bin/sh.

• If the script relies on bash extensions (aka "bashisms"), you must use /bin/bash in the shebang. Note that bash may disable some of its extensions if run as /bin/sh.

According to a quick smoke test, the latter is the case:

$ /bin/dash ddru_findbad
ddru_findbad: 7: ddru_findbad: Syntax error: "(" unexpected

Note that this /bin/dash is the default for /bin/sh on recent Debian systems (https://wiki.debian.org/Shell).

To resolve these issues, the checkbashisms script is your friend:

$ checkbashisms ddru_findbad
possible bashism in ddru_findbad line 7 ('function' is useless):
function version ()
...
if [ $PARTITIONONLY == "true" ]; then
possible bashism in ddru_findbad line 1068 (should be 'b = a'):

---

Just tried to remove it, and it breaks the build process.

Remove "m4/Makefile" from configure.ac and "m4" from Makefile.am.

[maximus57]

Remove "m4/Makefile" from configure.ac and "m4" from Makefile.am.

I thought I did, but maybe I missed something. If it is not hurting anything, it will be low on my priority list.

[Christian Franke]

Attached is a proposed patch to remove bashisms from ddru_findbad.sh. It would enable usage on systems where bash is not available by default (e.g. FreeBSD & friends). Editing the shebang by a Makefile rule should no longer be needed. Smoke tested with bash, dash, mksh, posh and zsh on Cygwin.

The patch lefts (at least) one issue open: echo -n "STRING" should be replaced by printf "STRING" if support of strictly POSIX compatible shells is desired. This is not needed for above shells.

See http://mywiki.wooledge.org/Bashism for useful hints.

[maximus57]

I will look at this when I get time. I have been busy with other things lately and have not done much work on ddrutility lately.