-
Notifications
You must be signed in to change notification settings - Fork 507
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
Autotools #8
base: master
Are you sure you want to change the base?
Autotools #8
Conversation
even though i'm not a big fan of autotools (but its still better than alternatives: cmake, scons etc) this would probably at least fix the current issues in the build system:
each of those points is actually a bug that should be adressed and filed as an issue, but i can't be bothered to register at yet another |
doh. i forgot the 4th point that is:
|
(Just to be clear: I also hate autotools. To really know autotools is to really hate autotools. But it has been forged in the crucible of time and passed through to the other side.) Do you want me to try to forward-port this patch to your repo tip? |
@barak: i (ab)used this PR here to raise my concerns about the existing build system, since the github issue tracker here is locked down (probably the author invested so much time into getting his trac running that he feels somehow attached to it, even though the github issue tracker is much better integrated, and would free him of the maintenance work). |
only non-trivial conflict is the CC option -std=c99, which I defer
src/iodined.c: In function ‘read_dns’: src/iodined.c:2062:34: error: invalid application of ‘sizeof’ to incomplete type ‘struct in6_pktinfo’ char control[CMSG_SPACE(sizeof (struct in6_pktinfo))]; ^
I merged these changes to the development tip, and made a few small changes to integrate them with the intervening changes to the code and to the rest of the system. |
Maybe it is time to switch.. Building for windows or android is really hacky now. I also think the android version doesnt run on modern android already because of some link flags - maybe it will be easier to fix with some autotools magic. It seems the thing in the patch setting the -DDARWIN for mac includes a version number and the compiler does not like that. Regarding trac, I am fine with using the bug tracker here, I am just too lazy/busy to move the existing issues and wiki pages. |
SUCCESS!!! |
|
||
# Checks for headers | ||
|
||
AC_CHECK_HEADERS([arpa/inet.h fcntl.h netdb.h netinet/in.h stdint.h stdlib.h string.h sys/ioctl.h sys/param.h sys/socket.h sys/time.h syslog.h termios.h unistd.h systemd/sd-daemon.h selinux/selinux.h]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stdint.h stdlib.h string.h unistd.h are standard C headers (even since C89).
all the other headers apart from sys/ioctl.h, sys/param.h and the systemd and selinux headers are standard POSIX headers. so i see little point in testing for their existence.
|
||
AC_FUNC_FORK | ||
AC_FUNC_MALLOC | ||
AC_CHECK_FUNCS([alarm dup2 inet_ntoa memset select socket strcasecmp strchr strdup strerror strrchr strstr]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmh... if the C library doesn't even supply malloc and memset then you cannot compile anything.
out of the functions listed here all are standard C or POSIX funcs, apart from strdup, dup2 and inet_ntoa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually i just have a PR pending doing just that, removing nonsensical checks for functions that are in any C library that came out in the last 15 years: tinyproxy/tinyproxy#66
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, you should check for daemon() and replace the ifdef-hell in src/common.c with a single #ifndef HAVE_DAEMON
. apart from that this repo has a history of not checking for anything, so it's definitely not necessary to now introduce a ton of checks for standard functionality that just make the build much slower.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Go for it! Most of the checks are not really sensible, and the #ifdefs in the code proper could be normalized to use the HAVE_xxx properties generated by autoconf almost exclusively.
(I used autoscan and plugged along from there pretty brainlessly.)
Complete rewrite of the build system using autotools.