McArcady / lnp-forge

A Dwarf Fortress starter pack builder for Linux & MacOS
Other
240 stars 18 forks source link

bash is not sh #88

Closed magnus-ISU closed 1 year ago

magnus-ISU commented 1 year ago

/bin/sh should only use POSIX-compliant shell features. configure failed to run for me until I thought to check and realized this was happening.

francoijs commented 1 year ago

What was the error exactly?

magnus-ISU commented 1 year ago

That sh is being executed instead of bash despite using some bash-only test features. The shebang is the problem.

I know configure is somehow being generated, so it is distasteful to edit it though. Idk if autoconf has an option for this (try a newer version?)

francoijs commented 1 year ago

Never has any problem on recent Debian and Ubuntu. Did you try this syntax: CONFIG_SHELL=/bin/bash ./configure ...

magnus-ISU commented 1 year ago

Debian and Ubuntu must symlink sh to bash. I use dash instead.

Running bash configure would work, but the point is the shebang (the first line of the configure script) is incorrect.

See https://stackoverflow.com/questions/5725296/difference-between-sh-and-bash

magnus-ISU commented 1 year ago

Here is the exact error. You should probably be able to reproduce by running dash configure.

$ dash configure
...
checking for java 1.8 or 11... configure: 6658: [: 0: unexpected operator
configure: WARNING: Java 1.8 or more is missing, LegendsBrowser will not be available
no
checking for java 1.7... no
configure: error: could not find Java JRE >=1.7
magnus-ISU commented 1 year ago

This is probably because line 326 of configure.ac. I read some and it seems configure is supposed to generate shell-agnostic code, and

On the other hand, if the problems you are experiencing are a result of your own shell code in configure.ac not being portable (eg, you're using bashisms) then the solution is to either stop using non-portable code or require the user to explicitly set SHELL or CONFIG_SHELL at configure time.

This is kind of silly that this is the recommendation when changing the shebang would work, or just saying bash configure is the proper way to run it. But figured more information would be useful.

McArcady commented 1 year ago

Well dash configure works too on my Debian system. I'm a bit reluctant to only patch configure which will be re-generated from configure.ac anyway (it has only been versioned for convenience). If the syntax bash configure works on your system, I propose we make it the official workaround for non-bash systems.

magnus-ISU commented 1 year ago

Hmm. The error seems to be related to java versioning. Java 8 was not my default - perhaps this is why it works for you?

I will look in configure.ac and see if I can figure out how it works. I think some bashisms are being used there, but don't worry about this as it is not very hard to work around.

McArcady commented 1 year ago

I tried the following with the latest Debian:

Did bash configure allowed you to run configure on your system?