Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] CCextractor uses bashism in configure script #1549

Closed
utkarsh2102 opened this issue Aug 17, 2023 · 7 comments · Fixed by #1574
Closed

[BUG] CCextractor uses bashism in configure script #1549

utkarsh2102 opened this issue Aug 17, 2023 · 7 comments · Fixed by #1574

Comments

@utkarsh2102
Copy link

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
Copy link
Author

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
Copy link
Author

utkarsh2102 commented Aug 18, 2023

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
Copy link
Author

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
Copy link
Member

I will check it this weekend.

@utkarsh2102
Copy link
Author

I will check it this weekend.

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

@prateekmedia
Copy link
Member

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

prateekmedia added a commit to prateekmedia/ccextractor that referenced this issue Sep 28, 2023
@prateekmedia
Copy link
Member

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

cfsmp3 pushed a commit that referenced this issue Oct 23, 2023
* fix: #1549 backticks

* fix: use single equal to
prateekmedia added a commit to prateekmedia/ccextractor that referenced this issue Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants