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

qrintf issues with autotools #11

Open
dhobsd opened this issue Dec 26, 2014 · 4 comments
Open

qrintf issues with autotools #11

dhobsd opened this issue Dec 26, 2014 · 4 comments

Comments

@dhobsd
Copy link
Contributor

dhobsd commented Dec 26, 2014

I'm not a fan of autotools, but can't work around using it in this case. I encountered several issues when integrating qrintf into our Varnish build:

Because qrintf.h is included very early in the compile process, a number of header files are pulled in before any chance of including config.h is allowed. This breaks packages that self-define C environment options like _GNU_SOURCE or _XOPEN_SOURCE or the like.

In my specific case, this was solved by hacking qrintf.h to include:

#ifdef HAVE_CONFIG_H
#include "config.h"
#endif

fixes the issue. I would submit a PR, but I'm sure that someone has an autotools build where the relevant guard is _HAVE_CONFIG_H and the file is also named differently.

I ran into a couple of other problems related to autohell:

  1. When qrintf-gcc is specified as the CC to use for configure, a number of header feature tests break. (A ffsl(3) test failed in jemalloc, for a specific case.) I worked around this by specifying CC as an argument to make.
  2. CFLAGS are not passed to qrintf-gcc. This requires one to set relevant build options a la CC="qrintf-gcc -std=gnu99 -Werror -Wall" make.

All of these things would be more easily resolved if it was easier / safer to include qrintf in projects built using these "tools". (In the meantime, I've worked around these issues, so this is not a high priority issue or anything. I'd be happy to submit patches if you have better ideas than the hacky ones I made.)

@dhobsd
Copy link
Contributor Author

dhobsd commented Dec 26, 2014

(I thought one solution might simply be to have a mode in which qrintf-pp only provides transforms if the file has included qrintf.h as a possible solution to the main issue and the header test issue; not sure how to fix the CFLAGS issue still.)

@kazuho
Copy link
Member

kazuho commented May 9, 2015

@dhobsd Sorry, I have overlooked the issue.

We could possibly add a macro that can be used to stop automatically including qrintf.h (something like qrintf $(CC) -DQRINTF_NO_AUTO_INCLUDE source-file-that-manually-includes-qrintf_h.c). Does that sound like a good idea?

Regarding the other issues, maybe we could address the problem if you could extract the failures as test cases.

@dhobsd
Copy link
Contributor Author

dhobsd commented May 9, 2015

No worries!

It's been a while since I looked into this so I had worked around it in the meantime. I'll try to put this up on the prio list and get the feature test issues into test cases.

Regarding your idea for the first issue, I think that would be fine for my purposes. Would you like me to submit a PR?

@kazuho
Copy link
Member

kazuho commented May 9, 2015

@dhobsd Thank you for your cooperation!

Regarding your idea for the first issue, I think that would be fine for my purposes. Would you like me to submit a PR?

No problem! I have done it in #14. It would be great if you could give it a try.

Please note that the command line interface has been changed in #13, please refer to the README for how to use the software.

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

No branches or pull requests

2 participants