search

kburtch / SparForte

Bourne shell, template engine, scripting language reliable, scalable projects. Based a ISO standard proven effective for large, mission-critical projects, SparForte is designed for fast development while, at the same time, providing easier designing, maintenance and bug removal. About 130.000 lines of code.
https://www.sparforte.com
GNU General Public License v2.0
50 stars 6 forks source link

configure logic is incorrect for database detection #7

Closed qunying closed 7 years ago

qunying commented 8 years ago

Hi Ken,

I noticed you revert one of my patch regarding the database configuration. But the current logic is not right.

I have both MySQL and PostgreSQL installed in the system, given the following commands: configure --arch=x86_64 --without-sql or configure --arch=x86_64 --without-postgres will result in no database being configure.

I added a line just before the database configuration to show the variables' value: for --without-mysql NO_MYSQL 1 NO_POSTGRES HAS_MYSQL HAS_POSTGRES 1 for --without-postgres NO_MYSQL NO_POSTGRES 1 HAS_MYSQL 1 HAS_POSTGRES

As the test [ -z "$NO_MYSQL" -a -z "$NO_POSTGRES" ] will always failed when you have one of the without parameter set, "-z" only test whether the variable is empty or not, it does not care it contains "0" or "1".

When you have --without-mysql defined, HAS_MYSQL is always empty and when --without-postgres is given, HAS_POSTGRES is always empty.

If system has only one DB installed, and run with "./configure --arch=x86_64" , will result in calling the APQ configure with full DB support, and it will prompt when one of the DB is not found, breaking automatic building script.

Using HAS_MYSQL and HAS_MYPOSTGRES will result in clearer logic flow of configure the APQ lib.

kburtch commented 8 years ago

Yes, there is a bug in the current logic. I turning off PostgreSQL worked but turning off MySQL did not. "-z" is working correctly as far as I know...I don't use it to check for 1 or 0. I will check into this more in a few days when I have more time.

qunying commented 8 years ago

You current testing actually make HAS_MYSQL and HAS_POSTGRES have the right value when at the stage of configuring APQ.

At your checking in line 928 of configure, only when NO_MYSQL is not set will it try to detect HAS_MYSQL, otherwise it is always empty regardless the system has it or not. The same goes to PosgreSQL, so it is more logical to to use HAS_MYSQL or HAS_POSTGRES in the APQ configure. The logic is more clear IMHO.

kburtch commented 8 years ago

Checked and merged. Replaced some ! -z with -n. Added comments.

kburtch commented 7 years ago

I've added a new set of tests to verify the configure flags work correctly for the databases. Let me know if this issue can be closed or if more work is needed.

kburtch commented 7 years ago

closing issue after 10 days. no response from reporter.