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.

Sign up for GitHub

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

Jump to bottom

Configure uses which which is not POSIX compliant #262

Closed
Kangie opened this issue Oct 1, 2024 · 7 comments · May be fixed by #263
Closed

Configure uses which which is not POSIX compliant #262

Kangie opened this issue Oct 1, 2024 · 7 comments · May be fixed by #263
Labels
bug Something isn't working build Related with the build process good first issue Good for newcomers
Milestone
Release 3.2.0

Comments

@Kangie
Copy link
Contributor

Kangie commented Oct 1, 2024 •
edited
Loading

Issue

checking FLTK 1.3... yes
checking whether to link to X11... yes
checking for jpeglib.h... yes
checking for jpeg_destroy_decompress in -ljpeg... yes
checking for zlib.h... yes
checking for zlibVersion in -lz... yes
checking for libpng-config... ./configure: line 7202: which: command not found
./configure: line 7204: which: command not found
./configure: line 7207: which: command not found

which is not listed as a required command by POSIX and should be replaced by command -v.

See also: https://bugs.gentoo.org/940568

Reproducing

Attempt to configure dillo on a machine without which.

Resolution

Exorcise which from the codebase.

Examples:

dillo/autogen.sh

Line 18 in 4f30333

which $1 > /dev/null 2>&1

dillo/configure.ac

Lines 298 to 313 in 4f30333

dnl Check if the user hasn't set the variable $PNG_CONFIG
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng16-config`
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng14-config`
fi
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng12-config`
fi
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng-config`
fi
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng10-config`
fi
fi

@Kangie
Copy link
Contributor Author

Kangie commented Oct 1, 2024

Would you accept a PR that switched to the meson build system rather than patching autotools?

It will prove to be more maintainable, will eliminate this issue entirely, and in terms of 'needs to run anywhere' there are muon and samurai which are C99 implementations of Meson and Ninja respectively.

@Kangie
Copy link
Contributor Author

Kangie commented Oct 1, 2024

Re:

dillo/configure.ac

Lines 298 to 313 in 4f30333

dnl Check if the user hasn't set the variable $PNG_CONFIG
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng16-config`
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng14-config`
fi
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng12-config`
fi
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng-config`
fi
if test -z "$PNG_CONFIG"; then
PNG_CONFIG=`which libpng10-config`
fi
fi

If autotools is to be retained this whole section (and the checks afterwards) could be trivially simplified to 'query pkg-config'.

@Kangie Kangie mentioned this issue Oct 1, 2024
Draft
@rodarima
Copy link
Member

rodarima commented Oct 1, 2024

I commented on the meson build in the PR. Let's keep this issue to solve only the problem of removing which to command -v, which should be fairly easy to do.

We don't have a build dependency over pkg-config, so I don't think this particular problem needs to pull it.

@rodarima rodarima added bug Something isn't working good first issue Good for newcomers build Related with the build process labels Oct 1, 2024
@rodarima rodarima added this to the Release 3.2.0 milestone Oct 1, 2024
@eli-schwartz
Copy link

Instead of using command -v, there is preexisting code earlier in this file:

AC_PATH_PROG(FLTK_CONFIG,fltk-config)

indicating the correct way to check for a program while respecting pre-set environment variables, is already known.

We don't have a build dependency over pkg-config, so I don't think this particular problem needs to pull it.

"which" isn't the problem that is solved by using pkg-config. You still have to check for and find pkg-config as well.

The problem that is solved by using pkg-config is the cross-compilation problem. :) There are plenty of other dependencies that can use the same treatment... openssl, zlib, jpeg...

@eli-schwartz
Copy link

It's possible to check pkg-config first and fall back to foobar-config programs only when that fails.

@rodarima
Copy link
Member

rodarima commented Oct 2, 2024

...indicating the correct way to check for a program...

The which command is not needed, so I prefer to switch to command -v which should be available in a POSIX shell.

The problem that is solved by using pkg-config is the cross-compilation problem

Then I recommend you address it in another issue. I think this may also help #258 and #34. It would be nice to have a cross-compilation build in the CI, but I'm not sure if I know how to set it up.

@rodarima
Copy link
Member

rodarima commented Oct 3, 2024

Fixed in 9880c1b

@rodarima rodarima closed this as completed Oct 3, 2024
@rodarima rodarima mentioned this issue Oct 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working build Related with the build process good first issue Good for newcomers
Projects
None yet
Milestone
Release 3.2.0
Development

Successfully merging a pull request may close this issue.

Meson Build Kangie/dillo
3 participants
@rodarima @eli-schwartz @Kangie