ascii-boxes/boxes

The GNU C Library no longer implements <regexp.h>

tim77 opened this issue · 22 comments

tim77 commented

If we set default, proper Fedora build flags Boxes won't compile.

To Reproduce

Try set %set_build_flags and:

%make_build                     \
    GLOBALCONF=%{cfgfile}       \
    CFLAGS='%{optflags}'        \
    LDFLAGS='%{build_ldflags}'  \

Result is:

In file included from boxes.h:33,
                 from boxes.c:31:
/usr/include/regexp.h:30:2: error: #error "The GNU C Library no longer implements <regexp.h>."
   30 | #error "The GNU C Library no longer implements <regexp.h>."
      |  ^~~~~
/usr/include/regexp.h:31:2: error: #error "Please update your code to use <regex.h> instead (no trailing 'p')."
   31 | #error "Please update your code to use <regex.h> instead (no trailing 'p')."
      |  ^~~~~
In file included from boxes.c:31:
boxes.h:84:5: error: unknown type name 'regexp'
   84 |     regexp *prog;                        /* compiled search pattern */
      |     ^~~~~~
boxes.c: In function 'apply_substitutions':
boxes.c:1266:25: warning: implicit declaration of function 'regcomp' [-Wimplicit-function-declaration]
 1266 |         rules[j].prog = regcomp (rules[j].search);
      |                         ^~~~~~~
boxes.c:1266:23: warning: assignment to 'int *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
 1266 |         rules[j].prog = regcomp (rules[j].search);
      |                       ^
boxes.c:1284:23: warning: implicit declaration of function 'myregsub' [-Wimplicit-function-declaration]
 1284 |             buf_len = myregsub (rules[j].prog, input.lines[k].text,
      |                       ^~~~~~~~

Expected behavior

Boxes successfully compiles with proper build flags and useful debuginfo.

Environment (please complete the following information):

  • Boxes version v1.3
  • Your operating system and architecture, e.g. Fedora 31

Additional info

full_build.log

tsjensen commented

Thanks for catching this!

It seems to me the core problem is

The GNU C Library no longer implements <regexp.h>. Please update your code to use <regex.h> instead (no trailing 'p').

I will try to make this change on my side, but it may take some time. Pull requests welcome! 😉

  • 👍1
tsjensen commented

I've looked at it some, and it seems that regexp.h refers to our own code, not a system library - it's just mistakenly enclosed in angle brackets. Will try to rename our code to bxregex or something, and we'll see if the problem goes away.

The cleaner solution would be to get rid of our own regex code, but it's hard to tell if that will break compilation on some of the more obscure platforms ...

  • 👍1
tsjensen commented

Ok, I tried to reproduce the problem, but couldn't. boxes appears to build just fine on Fedora.

But just to be sure I didn't miss anything, here's what I did:

docker pull fedora:latest
docker run -it --entrypoint=/bin/bash fedora:latest

Then, as root inside the Fedora container:

dnf install gcc bison flex vim make diffutils

Change into the boxes directory, where I just copied the current master from GitHub, then

make clean
make
make test

The full output of the make run is attached in make.log. It shows two warnings and a note, but produces a fine binary. The tests were green:

94 tests executed, 94 successful, 0 failed.

So now I need your help on reproducing the problem.

tim77 commented

Ok, I tried to reproduce the problem, but couldn't. boxes appears to build just fine on Fedora.

Build with default boxes flags fine for sure, but i believe we can improve it and build with default Fedora flags since this highly recommended according to guidelines.

Here is a list of Individual compiler flags with description.

Debuginfo info.

tsjensen commented

Thank you for the clarification. To be honest, I do not understand the linked page. It appears to refer to RPM package specifications, which in turn seem to be able to contain build instructions. I am not familiar enough with this to deduce a test case from it.
If you want me to continue investigating this, you will have to supply me with something far simpler, let's say, a ZIP file to extract, containing a script to run. Then I can analyze the error and see if it can be fixed.

Sorry about that - I don't want to be complicated, but one can't know everything, right? That's why I am very thankful for the package maintainers doing this work!

tim77 commented

Sorry about that - I don't want to be complicated, but one can't know everything, right? That's why I am very thankful for the package maintainers doing this work!

Not a problem at all!

If you want me to continue investigating this, you will have to supply me with something far simpler, let's say, a ZIP file to extract, containing a script to run. Then I can analyze the error and see if it can be fixed.

Here is an SPEC file where build forced to use canonical Fedora build flags. The result is fail. With this patch 7ef0b35

To build yourself src.rpm in clean environment on Fedora you need:

sudo dnf install @rpm-development-tools mock
sudo usermod -a -G mock $(whoami)
mock -r fedora-$(rpm -E %fedora)-$(uname -m) --rebuild boxes-1.3-5.fc31.src.rpm

More will come...

tsjensen commented

I tried going with this, but no luck so far. This is what I did:

docker pull fedora:latest
docker run -it --entrypoint=/bin/bash fedora:latest

Then, as root inside the Fedora container:

dnf install @rpm-development-tools mock
usermod -a -G mock $(whoami)
mock -r fedora-$(rpm -E %fedora)-$(uname -m) --rebuild boxes-1.3-5.fc31.src.rpm

This gave me

ERROR: Cannot find/open srpm: boxes-1.3-5.fc31.src.rpm. Error: [Errno 2] No such file or directory: 'boxes-1.3-5.fc31.src.rpm'

so I copied your linked src rpm into the same directory and ran the command again, which gave me:

INFO: mock.py version 1.4.21 starting (python version = 3.7.4)...
Start: init plugins
INFO: selinux disabled
Finish: init plugins
INFO: Signal handler active
Start: run
ERROR: Namespace unshare failed.
ERROR: It seems we are running inside of Docker. Let skip unsharing.
ERROR: You should *not* run anything but Mock in this container. You have been warned!
INFO: Start(boxes-1.3-5.fc31.src.rpm)  Config(fedora-31-x86_64)
Start: clean chroot
Finish: clean chroot
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled dnf cache
Start: cleaning dnf metadata
Finish: cleaning dnf metadata
INFO: enabled HW Info plugin
Mock Version: 1.4.21
INFO: Mock Version: 1.4.21
ERROR: Exception(boxes-1.3-5.fc31.src.rpm) Config(fedora-31-x86_64) 0 minutes 0 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-31-x86_64/result
ERROR: Command failed:
 # /bin/mount -n --bind /var/cache/mock/fedora-31-x86_64/dnf_cache/ /var/lib/mock/fedora-31-x86_64/root/var/cache/dnf/

There must still be something else that you type ...

tim77 commented

ERROR: You should not run anything but Mock in this container. You have been warned!

Mock is container itself. You don't need Docker for this. But you can try run Mock with --old-chroot (not sure if this works for you):

mock --old-chroot -r fedora-$(rpm -E %fedora)-$(uname -m) --rebuild boxes-1.3-5.fc31.src.rpm

But better use just mock in your Fedora, it's safe and clean. Or you can try this in VM with Fedora.

tim77 commented

Or if you prefer Docker there is easier way to do this probably.

In docker:

dnf install @rpm-development-tools
sudo dnf builddep boxes-1.3-5.fc31.src.rpm
rpmbuild --rebuild ~/rpmbuild/SRPMS/boxes-1.3-5.fc31.src.rpm
tsjensen commented

I haven't forgotten you, it's just that currently there are too many other things that need taking care of. It will be a few days until I can continue with this. I'll keep you posted here. Thank you for your patience! 🐌

tim77 commented

Same here. ☺️ Anyway Boxes works fine at this moment even in Rawhide so this is not critical.

tsjensen commented

Just tried it again, but it seems the download link for the source RPM has gone stale:
https://copr-be.cloud.fedoraproject.org/results/atim/playground/fedora-31-x86_64/01219249-boxes/boxes-1.3-5.fc31.src.rpm
returns 404 now. Would you be able to upload it here, as an attachment to a comment?

I will try the Docker variant, as I am on Windows, so all my Fedora must be on Docker. 😇

  • 😄1
tim77 commented

Just tried it again, but it seems the download link for the source RPM has gone stale

Ah, yes, Copr automatically removing after some time builds which failed. Re-uploaded in safe place:

SPEC: https://atim.fedorapeople.org/for-review/boxes.spec
SRPM: https://atim.fedorapeople.org/for-review/boxes-1.3-5.fc33.src.rpm

tsjensen commented

Thanks! I have used these files to create a simple Dockerfile that allows me to reproduce the error message (attached, remove .txt extension after download which was required by GitHub).

Now, for further analysis, I would like to be able to modify the boxes.spec file and analyze the changes. So, how do I get from a changed .spec file to a new source rpm?

tsjensen commented

Btw, one thing I noticed is that you probably want

BuildRequires:  make
BuildRequires:  diffutils

in addition to gcc etc. At least the build failed for me at first because of missing make.

tim77 commented

Btw, one thing I noticed is that you probably want

BuildRequires:  make
BuildRequires:  diffutils

in addition to gcc etc. At least the build failed for me at first because of missing make.

Sorry for delay: this handled automatically by DNF. BuildRequires: gcc is enough there and no need to add explicitly such additional decencies. mock handled this but since you can't using in in Docker -- sudo dnf builddep boxes-1.3-5.fc31.src.rpm should do this work for you.

tim77 commented

Now, for further analysis, I would like to be able to modify the boxes.spec file and analyze the changes. So, how do I get from a changed .spec file to a new source rpm?

rpmbuild -bs ~/rpmbuild/SPECS/boxes.spec

  • 👍1
tsjensen commented

Btw, one thing I noticed is that you probably want

BuildRequires:  make
BuildRequires:  diffutils

in addition to gcc etc. At least the build failed for me at first because of missing make.

Sorry for delay: this handled automatically by DNF. BuildRequires: gcc is enough there and no need to add explicitly such additional decencies. mock handled this but since you can't using in in Docker -- sudo dnf builddep boxes-1.3-5.fc31.src.rpm should do this work for you.

Well, that's actually what I did, and make was still missing. And why would it automagically know that we use diffutils in our test suite? But you know that better than me, I just wanted to let you know about this observation.

tsjensen commented

In the course of implementing #1, I plan to kick out the handcrafted regex stuff for pcre2, which should resolve this issue as well, hopefully.

  • 👍1
tsjensen commented

Released as v2.0.0. 🎉

I think this should be resolved because I removed the entire regex subpackage. Instead, we now depend on the PCRE2 library. But it would still be great if you could validate. Also let me know about any other quirks I might have inadvertently introduced for Fedora. 😊

  • 🚀1
tim77 commented

Can confirm, issue fixed in v2.0.0. And now Boxes compiles with proper Fedora build flags and debuginfo package generated. Thank you!

New version built for all current Fedora releases and ready for testing.

  • 👍1
tsjensen commented

That's great - thank you very much for taking care of boxes out there!

  • 👍1