lgblgblgb / xemu

Emulations (running on Linux/Unix/Windows/macOS, utilizing SDL2) of some - mainly - 8 bit machines, including the Commodore LCD, Commodore 65, and the MEGA65 as well.
https://github.com/lgblgblgb/xemu/wiki
GNU General Public License v2.0
201 stars 31 forks source link

BUILD: configure: incorrect way of testing for `grep` #396

Closed Rhialto closed 4 months ago

Rhialto commented 7 months ago

Describe the bug For pkgsrc I package snapshots of xemu, and in the latest one (for hash 8aa261c07ec7c2f9820e99f9c8fb1930fd42e86a) the configure script claims that it goesn't find grep.

Used version of the project 8aa261c07ec7c2f9820e99f9c8fb1930fd42e86a (yes, git commit hashes are horrible)

To Reproduce Check out https://github.com/NetBSD/pkgsrc.git and https://github.com/NetBSD/pkgsrc-wip inside that. Probably you need to run bootstrap.sh if you're not on NetBSD. cd pkgsrc/wip make

murthe.0:.../pkgsrc/wip/xemu$ make
[...]
=> Bootstrap dependency digest>=20211023: found digest-20220214
===> _pkgformat-check-vulnerable [xemu-0.0.2023.12.20] ===> Checking for vulnerabilities in xemu-0.0.2023.12.20
===> wrapper-message [xemu-0.0.2023.12.20] ===> Creating toolchain wrappers for xemu-0.0.2023.12.20
===> configure-message [xemu-0.0.2023.12.20] ===> Configuring for xemu-0.0.2023.12.20
=> Inserting DESTDIR into INSTALL_BINDIR
=> readline
=> Replacing /usr/local
=> Generating pkg-config file for builtin expat package.
=> Generating pkg-config files for builtin xz package.
=> Replacing bash interpreter in build/configure/configure.
=> Checking for portability problems in extracted files
cd /tmpfs/wip/xemu/work.x86_64/xemu-8aa261c07ec7c2f9820e99f9c8fb1930fd42e86a/build/configure && bash ./configure --arch=native
*** Running xemu-configure ***
Testing needed UNIX tools for the build: grep 
FATAL: UNIX utility 'grep' does not seem to work or cannot be found
*** Error code 1

Stop.
make[1]: stopped in /mnt/vol1/rhialto/cvs/pkgsrc/wip/xemu
*** Error code 1

Stop.
make: stopped in /mnt/vol1/rhialto/cvs/pkgsrc/wip/xemu

Expected behavior Of course we have grep. I suspect that the test uses non-POSIX GNU-isms, though.

Screenshots

Computer/Device (please complete the following information):

Additional context

Rhialto commented 7 months ago

Actually I already added a patch to pkgsrc-wip, to remove the check for grep. With that the build proceeds fine. Which is another indicator that the test is testing a wrong thing.

Rhialto commented 7 months ago

I looked into it in more detail, and it seems that it is in fact NetBSD's grep that doesn't work with this combination of options -oEia. It appears to be fixed in NetBSD-current but not in NetBSD-10.

Fortunately, I found only grep -v in the configure script, so I guess the configure test could be limited to that option.

lgblgblgb commented 7 months ago

@Rhialto Thanks for the report! To be honest, I have never tried to compile Xemu for/on NetBSD, though I've tried on OpenBSD and FreeBSD once or twice, I believe, also speaking of UNIX/UNIX-like (outside of the Linux world, I mean) I regularly build it for/on MacOS. And no, of course I don't have anything "against" NetBSD, just somehow it's a bit out of scope for me.

You're probably right, I didn't have extensive testings on how my ugly configure script works on different "UNIX-class" operating systems, since I usually build only for the official targets (Linux, Windows, MacOS, for Windows, it's cross compiled on Linux, so it's not a problem too much). Surely, of course I like the idea to help other platforms as well just because of my laziness to try on everything is not an excuse :)

Actually, grep is used also with the -i and in some cases with the -o option (though IIRC only on building debian packages for this second, which is obviously not a big problem for NetBSD ....) . I also had some thoughts what would be better: using egrep and fgrep or plain grep with -E and -F option.

Anyway, I guess my test for grep is a bit overkill and usless: if a system have GNU make (which is a must, BSD make won't work - as far as I am aware), usual tools etc, not grep could be something which is missing, so probably I can either eliminate the check or at least I should make it more "relaxed" than now it is.

Rhialto commented 7 months ago

So the annoying story is that there are two greps in the NetBSD source tree. One with a BSD provenance, the other with GNU provenance. The latter is the one that is actually built by default, but it was imported very very long ago, before the -i -o bug was fixed upstream... Maybe an updated version will be imported, we will have to see.

lgblgblgb commented 7 months ago

Probably it's time for me to install NetBSD in a VM at least, to be honest I've never tried that (I am not so rare OpenBSD user though besides my daily driver Linux OS).

lgblgblgb commented 5 months ago

So the annoying story is that there are two greps in the NetBSD source tree. One with a BSD provenance, the other with GNU provenance. The latter is the one that is actually built by default, but it was imported very very long ago, before the -i -o bug was fixed upstream... Maybe an updated version will be imported, we will have to see.

@Rhialto Sorry, I am kind of busy lately and totally forgot on this one. Do you have any new info, that I really should change how detecting grep works?

Rhialto commented 5 months ago

No worries, there is no rush, for pkgsrc I have it patched out anyway.

You mention that grep is also used with more flags than is apparent from just the configure script. Perhaps it is possible to replace the current check, which tests for all options at once, with 2 invocations of grep that don't use the -i and -o option together. If -i and -o are used together only for building Debian packages, then only Debian really needs to supply a fully capable grep, after all. And they do.

So something like this could work:

if [ "`echo 'alma,korte,banan' | grep -oEa ',k[^,]+,'`" != ",korte," ]; then
    echo
    echo "FATAL: UNIX utility 'grep' does not seem to work or cannot be found (1)" >&2
    exit 1
fi

if [ "`echo 'alma,korte,banan' | grep -iaE ',K[^,]+,'`" != "alma,korte,banan" ]; then
    echo
    echo "FATAL: UNIX utility 'grep' does not seem to work or cannot be found (2)" >&2
    exit 1
fi

Possibly if you want to be extra careful, you could keep the original test after this, with the messaged changed to WARNING: Unix utility 'grep' does not seem to support options -o and -i simultaneously, and the exit removed.

lgblgblgb commented 5 months ago

@Rhialto Thanks for the tips. I would still need to install NetBSD (even just for fun!), it's s shame, that it's the only "main" BSD (well it depends if I cound DragonFly BSD now as a distinct BSD member ...) which I haven't used by me yet ever, ehmm, blush ... By the way, I am curious now, what kind of patching is needed for pkgsrc? Sorry for my stupidity in this regard, just I am thinking that it's possible to integrate those changes, or those are just too much NetBSD specific things which would disturb things on other OSes.

Rhialto commented 5 months ago

You can see the patches in https://github.com/NetBSD/pkgsrc-wip/tree/master/xemu/patches . Just 3 small ones.

Apart from what we have been discussing here, the other two are both the same: the command /usr/pkg/bin/sdl2-config --libs doesn't include the -Wl,-R, option to find the libraries at run time (pkgsrc likes to build binaries with explicit RPATHs, to help avoid finding wrong libraries). That is more a bug in how SDL2 is packaged, so I don't think you need to do anything about this.

And looking at the details, the pathname it creates here might be wrong anyway, do I need to revisit this and maybe even can drop both those patches entirely.

lgblgblgb commented 4 months ago

@Rhialto

Sorry for the slow progress, it takes some time till some patch goes into the git repo, as I am in the middle of some bigger change, and I am very much mono-tasking guy it seems :-O But I haven't forgotten this report, for sure.

By the way, I recommend using make RELEASE=yes to do a build, unless it's built for an Xemu developer. It avoids compiling in some debug features which is only useful for me, or anyone who wants to develop Xemu, but results in very slightly slower and larger binaries. Oh, in case of BSD I guess it's gmake RELEASE=yes though, instead of make RELEASE=yes, as my custom makefiles wouldn't work with BSD make, as far as I am aware at least.