CCExtractor / ccextractor

CCExtractor - Official version maintained by the core team
https://www.ccextractor.org
GNU General Public License v2.0
721 stars 426 forks source link

[BUG] CCextractor uses bashism in configure script #1549

Closed utkarsh2102 closed 1 year ago

utkarsh2102 commented 1 year ago

Originally reported as: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=998781

Hi,

Your package uses configure script with bash features not present in POSIX without explicitly declaring the need to bash shell; this currently works as configure scripts select bash, but when dash enables LINENO support, your configure script will start failing:

 checking pkg-config m4 macros... ./configure: 4747: test: yes: unexpected operator
 no
 configure: error: 
 pkg-config is required.
  cd linux && tail -v -n \+0 config.log

To test this, you can install dash from experimental and re-run the configure script.

Please replace non-POSIX features with their equivalents to make sure the script runs with dash. Most common ones are usage of == instead of = and for with arrays (not lists).

utkarsh2102 commented 1 year ago

Hi @prateekmedia,

I believe this patch works:

--- a/linux/configure.ac
+++ b/linux/configure.ac
@@ -15,7 +15,7 @@ AC_PROG_MAKE_SET

 #Checks for "pkg-config" utility
 AC_MSG_CHECKING([pkg-config m4 macros])
-if test m4_ifdef([PKG_CHECK_MODULES], [yes], [no]) == yes; then
+if test m4_ifdef([PKG_CHECK_MODULES], [yes], [no]) = yes; then
        AC_MSG_RESULT([yes]);
 else
        AC_MSG_RESULT([no]);
diff --git a/mac/configure.ac b/mac/configure.ac
index 7cf89f2..81781c7 100644
--- a/mac/configure.ac
+++ b/mac/configure.ac
@@ -15,7 +15,7 @@ AC_PROG_MAKE_SET

 #Checks for "pkg-config" utility
 AC_MSG_CHECKING([pkg-config m4 macros])
-if test m4_ifdef([PKG_CHECK_MODULES], [yes], [no]) == yes; then
+if test m4_ifdef([PKG_CHECK_MODULES], [yes], [no]) = yes; then
        AC_MSG_RESULT([yes]);
 else
        AC_MSG_RESULT([no]);

What do you think? If you think that works for you as well, I can make a PR and we can land this one.

utkarsh2102 commented 1 year ago

Also, when doing the standard linux compilation, I get:

./configure: line 7381: test: syntax error: `-larchive' unexpected

on running ./configure. Hm, is that expected?

utkarsh2102 commented 1 year ago

Also, when doing the standard linux compilation, I get:

./configure: line 7381: test: syntax error: `-larchive' unexpected

on running ./configure. Hm, is that expected?

Oh no that's a bug:

--- a/linux/configure.ac
+++ b/linux/configure.ac
@@ -149,7 +149,7 @@ AS_IF([ (test x$ocr = xtrue || test x$hardsubx = xtrue) && test ! $HAS_LEPT -gt
 AM_CONDITIONAL(HARDSUBX_IS_ENABLED, [ test x$hardsubx = xtrue ])
 AM_CONDITIONAL(OCR_IS_ENABLED, [ test x$ocr = xtrue || test x$hardsubx = xtrue ])
 AM_CONDITIONAL(FFMPEG_IS_ENABLED, [ test x$ffmpeg = xtrue ])
-AM_CONDITIONAL(TESSERACT_PRESENT, [ test ! -z  `pkg-config --libs-only-l --silence-errors tesseract` ])
+AM_CONDITIONAL(TESSERACT_PRESENT, [ test ! -z  "`pkg-config --libs-only-l --silence-errors tesseract`" ])
 AM_CONDITIONAL(TESSERACT_PRESENT_RPI, [ test -d "/usr/include/tesseract" && test `ls -A /usr/include/tesseract | wc -l` -gt 0 ])
 AM_CONDITIONAL(SYS_IS_LINUX, [ test `uname -s` = "Linux"])
 AM_CONDITIONAL(SYS_IS_MAC, [ test `uname -s` = "Darwin"])

This patch fixes this. However I think we should be using $(...) as backticks are deprecated now. @prateekmedia, what do you think?

prateekmedia commented 1 year ago

I will check it this weekend.

utkarsh2102 commented 1 year ago

I will check it this weekend.

Hey, have you had time to take a quick look at this?

prateekmedia commented 1 year ago

@utkarsh2102 can you create a PR for this patch, I will take a look.

prateekmedia commented 1 year ago

@utkarsh2102 I have created a PR #1572 for the same, can you check.