Re: [PATCH 4/9] build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build

From: Jeff Mahoney <jeffm@suse.com>
To: Eryu Guan <eguan@redhat.com>, "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: fstests@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/9] build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build
Date: Thu, 15 Mar 2018 17:25:46 -0400	[thread overview]
Message-ID: <ead4a973-966a-1c79-4301-4090a4dbc974@suse.com> (raw)
In-Reply-To: <20171214055102.GE2749@eguan.usersys.redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 2815 bytes --]

On 12/14/17 12:51 AM, Eryu Guan wrote:
> On Tue, Dec 12, 2017 at 04:45:14PM -0800, Luis R. Rodriguez wrote:
>> Modern gdbm-devel packages bundle together gdbm.h and ndbm.h.
>> The old m4 macro had detection support for some old gdbm libraries
>> but not for new ones.
>>
>> We fix compilation of src/dbtest.c by making the autoconf helper
>> check for this new arrangement:
>>
>> If both gdbm.h and gdbm.h are found define set both gdbm_ndbm_=true,
>                      ^^^^^^ ndbm.h?
>> and have_db=true, and define HAVE_GDBM_H. The src/dbtest.c already
>> had a HAVE_GDBM_H but there was never a respective autoconf settter for
>> it. We can just re-use this and fix it for new arrangement.
>>
>> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> 
> This looks fine to me.
> 
> The only system I have by hand that have both <gdbm.h> and <ndbm.h> but
> not any <gdbm/[gn]dbm.h> is openSUSE Tumbleweed. Without this patch,
> dbtest was not built on openSUSE, and was built successfully with this
> patch applied. And dbtest is still built on RHEL6/7 and Fedora.
> 
> BTW, I'll queue patch 3 and this patch for next fstests release, while
> other patches seem not necessary, I agreed with Dave that groups are not
> for excluding tests, the required tools and environments should be
> detected by tests and _notrun if not met. (The README change looks fine,
> but it doesn't apply due to the "fsgqa-381" change, so I drop it too for
> now.)

Hi guys -

This change breaks on older releases like SLES 11 where both <ndbm.h>
and <gdbm.h> define datum, so we get build failures.  The failure is
new, but not because it used to pass and now doesn't.  It's apparently
never built on SLES releases since we ship /usr/include/ndbm.h and then
we notrun the test that uses.  Now that we're looking for gdbm.h and
find it, we attempt to build src/dbtest and fail.

This fix isn't the right solution.  The problem is that we have a couple
layers of old cruft that needs to be cleaned out.

1) As Luis notes, nothing sets HAVE_GDBM_H.  The thing is that there is
no version of gdbm.h that exports the NDBM interface.  Further, looking
at the git history, nothing has ever set HAVE_GDBM_H.  It was dead code
when it was committed initially as best I can tell.
2) openSUSE Tumbleweed doesn't need <gdbm.h> at all.  It needs <ndbm.h>
and this fix works because Luis added it to the HAVE_GDBM_H stanza.
3) AC_PACKAGE_WANT_NDBM used to check for <ndbm.h> but it was a check
for IRIX and the caller was removed ages ago.  It wouldn't matter if it
were called anyway since libndbm is an IRIX library.  Linux, IIRC, has
never shipped a libndbm.

I'll post a few patches following this to clean it up and get it working
on SLES11.

-Jeff

-- 
Jeff Mahoney
SUSE Labs

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]