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 AC_DEFINE macros #5

Closed
wants to merge 1 commit into from
Closed

Fix AC_DEFINE macros #5

wants to merge 1 commit into from

Conversation

zdohnal
Copy link
Contributor

@zdohnal zdohnal commented Jan 16, 2020

Hi,

I was trying to compile LPrint for Fedora and played with configure.ac, but 'autoreconf -v -f -i' fails on AC_DEFINE/AC_DEFINE_UNQUOTED macros ('autoheader' in fact):

autoheader: warning: missing template: HAVE_AVAHI
autoheader: Use AC_DEFINE([HAVE_AVAHI], [], [Description])
autoheader: warning: missing template: HAVE_DNSSD
autoheader: warning: missing template: HAVE_LIBPNG
autoheader: warning: missing template: HAVE_LIBUSB
autoheader: warning: missing template: HAVE_SYS_RANDOM_H
autoheader: warning: missing template: LPRINT_VERSION
autoreconf: /usr/bin/autoheader failed with exit status: 1

The pull request fixes it. I did not commit changed configure and config.h.in, it takes some setting from my machine, so the full fix would be run 'autoreconf -i -v -f' after merging the pull request.

I used 'XY available' for description in AC_DEFINE.

Does it look ok?

@michaelrsweet
Copy link
Owner

Sigh... NEVER run autoreconf or autogen on any of my software. It will break things and you will not be happy...

@zdohnal
Copy link
Contributor Author

zdohnal commented Jan 17, 2020

@michaelrsweet I was manipulating with configure.ac because of linker errors which you see in LGTM.
They are not results of my pull request - I get the same errors when I cloned the project and tried to build it for the first time.
I thought it is an issue of our default Fedora flags, so I just added -fPIC into CFLAGS variable during configure, but it seems it is the issue in LGTM too.

Should the LGTM setup be fixed or configure.ac? AC_DEFINE fix is more cosmetic one - it is just annoying for someone, who needs to test some new code in configure.ac. Or is there:qa any other way how to test the new code in configure.ac then running autoreconf?

I found out the configure script does not check for libasan availability when --enable-sanitizer is used, but it seems strange - gcc allows -fsanitize=address but without libasan? It seems like gcc issue.

@michaelrsweet
Copy link
Owner

[master ebc8781] Fix PIE issue with some Linux compilers (Issue #5)

@michaelrsweet
Copy link
Owner

I added -fPIC - -fPIE should do this automatically (and this seems to happen with GCC as shipped in Ubuntu 18.04 LTS and Clang as shipped from Apple) but it doesn't hurt to add it and I'd like to keep any LGTM/Snap/etc. customization to a minimum...

As for the address sanitizer stuff, that is odd but it is just there for developers to hammer lprint for debug purposes - I don't see that being an issue in general since it isn't an option you'd use for shipping binaries.

@michaelrsweet michaelrsweet self-assigned this Jan 17, 2020
@michaelrsweet michaelrsweet added the bug Something isn't working label Jan 17, 2020
@michaelrsweet michaelrsweet added this to the v1.0.x milestone Jan 17, 2020
@zdohnal
Copy link
Contributor Author

zdohnal commented Jan 20, 2020

I added -fPIC - -fPIE should do this automatically (and this seems to happen with GCC as shipped in Ubuntu 18.04 LTS and Clang as shipped from Apple) but it doesn't hurt to add it and I'd like to keep any LGTM/Snap/etc. customization to a minimum...

Ok, no problem, thank you!

As for the address sanitizer stuff, that is odd but it is just there for developers to hammer lprint for debug purposes - I don't see that being an issue in general since it isn't an option you'd use for shipping binaries.

Makes sense, I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants