Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix generating bison output for pars.y #48

Closed
wants to merge 1 commit into from

Conversation

ccawley2011
Copy link

The generated pars.c file requires the header file specified by the --defines argument, but it's renamed before the source file can be compiled.

@th-otto
Copy link
Contributor

th-otto commented Feb 20, 2023

I recently changed that rule for a simple reason: if you let the rule write directly to pars.h, then pars.c might be compiled before bison is finished writing pars.h when you run parallel makes, and then including an incomplete file, resulting in compile errors.

So i think your change will just introduce the previous behaviour, which i attempted to fix. That does not mean that the current rule is absolutely correct, maybe we should check the documentation of make again. IIRC, that scenario is mentioned somewhere there.

@mikrosk
Copy link
Member

mikrosk commented Feb 22, 2023

Actually @th-otto, you did break something there. When trying to reproduce scummvm/dockerized-bb#50 I could easily get the following (Ubuntu 22.04.2 LTS):

make[4]: Entering directory '/home/mikro/m68k-atari-mint-build/mintlib-master/syscall'
gcc -O -Wall -c check.c -o check.o
gcc -O -Wall -c generate.c -o generate.o
gcc -O -Wall -c main.c -o main.o
if bison --defines=pars.h.tmp$$ --output=pars.c pars.y; then mv pars.h.tmp$$ pars.h; else rm -f pars.h.tmp$$; exit 1; fi
gcc -O -Wall -c pars.c -o pars.o
pars.c:149:10: fatal error: pars.h.tmp19480: No such file or directory
  149 | #include "pars.h.tmp19480"
      |          ^~~~~~~~~~~~~~~~~
compilation terminated.
make[4]: *** [../checkrules:50: pars.o] Error 1
make[4]: Leaving directory '/home/mikro/m68k-atari-mint-build/mintlib-master/syscall'
make[3]: *** [Makefile:46: gen-syscall0] Error 2
make[3]: Leaving directory '/home/mikro/m68k-atari-mint-build/mintlib-master/syscall'
make[2]: *** [Makefile:162: all-recursive] Error 1
make[2]: Leaving directory '/home/mikro/m68k-atari-mint-build/mintlib-master'
make[1]: *** [Makefile:238: mintlib] Error 2
make[1]: Leaving directory '/home/mikro/m68k-atari-mint-build'

So I'd say it's the other way around, unless you fix it, we'll have to revert your patch. ;)

@th-otto
Copy link
Contributor

th-otto commented Feb 22, 2023

Damn, ok, i'll have a look into that. Didn't realize that bison uses the --defines filename also to generate the include fro it,so that is definitely wrong now. Just wonder why it works her, and also in the build docker from github.

But the previous rule was also wrong. It only gives errors when running parallel makes, and isn't always reproducible, but it happens that pars.c is compiled before bison finishes writing pars.h. Maybe need a temp file here that is created when bison is finished, and add it as dependency for pars.o. Or do you have any better idea?

@th-otto
Copy link
Contributor

th-otto commented Feb 22, 2023

I've pushed a fix now. Could please trigger a build of scummvm (without your patch) and see if it works now?

@mikrosk
Copy link
Member

mikrosk commented Feb 23, 2023

Confirmed, it seems fixed.

@mikrosk mikrosk closed this Feb 23, 2023
@ccawley2011 ccawley2011 deleted the syscall-bison branch February 23, 2023 09:49
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants