client/ups: add missing include

[yann-morin-1998]

struct timeval is declared in sys/time.h, so we need to #include it.

Signed-off-by: "Yann E. MORIN" yann.morin.1998@free.fr

[clepple]

I agree that struct timeval needs a header, but inside the NUT code, we have typically used #include "timehead.h" for that. Is that installed with upsclient.h on your system when you build NUT?

In the long term, we should be more formal about requiring a certain minimum set of POSIX features to build, so things like timehead.h can go away.

[clepple]

Never mind, I see that timehead.h is listed as dist_noinst_HEADERS in include/Makefile.am.

[yann-morin-1998]

@clepple Thanks for merging!

However, I am not sure I understood that sentence of yours:

Is that installed with upsclient.h on your system when you build NUT?

My build issue was while cross-compiling, so even if my hpst machine had
the nut headers installed (which were not), they would not have been used.

But, since you merged that, I guess all is good. Thanks! :-)

[clepple]

However, I am not sure I understood that sentence of yours:
Is that installed with upsclient.h on your system when you build NUT?

My build issue was while cross-compiling, so even if my hpst machine had
the nut headers installed (which were not), they would not have been used.

Ah, I assumed the build error was from including upsclient.h in a (non-cross-compiled) client application that had not included sys/time.h already. Either way, sys/time.h is the current POSIX recommendation, so I created a new issue for dealing with timehead.h later.

[yann-morin-1998]

@clepple OK, thanks for the explanations.
Keep up the good work! :-)

[jimklimov]

I had issues compiling that expectation at least on some systems, e.g.

# ls -la /usr/include/sys/time.h
ls: cannot access /usr/include/sys/time.h: No such file or directory

# ls -la /usr/include/time.h
-rw-r--r-- 1 root root 13724 Jun 18 11:06 /usr/include/time.h

# cat /etc/debian_version 
8.9

[yann-morin-1998]

@jimklimov In debian, the system include path is in /usr/include/ARCH-TUPLE
(where for example ARCH-TUPLE is x86_6 4-linux-gnu), so you would expect
sys/time.h to be in there.

And on a freshly installed Jessie as of today, it works for me:

$ cat ess.c
#include <sys/time.h>
$ ls -l /usr/include/sys/time.h
ls: cannot access /usr/include/sys/time.h: No such file or directory
$ gcc -c -o ess.o ess.c
[no error]

So there seems to be some kind of misconfiguration on your side. Can you
double-check, please?

[jimklimov]

Interesting... unfortunately, I did not have reproduction notes to the issue - even though I haven't hit it in the years before. It is possible that something blew up while I was trying to set up experimental cross-compilation and explicitly listed the -isystem... paths to include directories, and did so without the triplet for Linux builds. But I am not sure, the night was late then ;(

Also, one matter is living on systems that follow the "current POSIX recommendation" and another is supporting those systems which do not (e.g. old releases) that can be easily supported otherwise.

[yann-morin-1998]

So, a few things...

First, don't point -isystem to system directories, it breaks the build, especially with recent gcc, as it borks the heuristic gcc uses to search for headers. -isystem can only bne used to add search paths that are not yet system search paths.

Second, for cross-compilation, the cross-compiler already knows where to look for its own system headers, no need to tell it where they are. Any decently recent cross-compler is sysrooted nowadays, and it means it can be relocated without issue. a non-sysrooted cross-compiler is so old that it is at least a decade old now, if not more. I highly doubt current nut would even build on such an ancient cross-compiler (gcc 4.2 was already fully sysrooted, and that was released more than 10 years ago now...) (Sorry, no pointer, just my long experience doing cross-toolchain...)

Third, sys/time.h has existed, like, forever now, it is definitely not something new. Systems that do not have it are so ancient (at least three decades I believe) that I highly doubt we should care about those.

[jimklimov]

I feel puzzled now :) And so much the worse that I can't really detail how I stumbled on this. I think the story went like this: in our project we used Linaro GCC + toolkit for some parts like firmware. Given that native ARM compilations are slow, I looked at how much of our project's pieces can be built by cross-compiling, and started out with Linaro as known to work for us elsewhere. Maybe I should have used some native Debian GCC package for cross-compiling? We had some poor experience with those about 3 years ago, so did not look again since :\

I believe one of the bigger issues that led me to -isystem hacks was that the *-dev packages for different architectures sometimes did install, but mostly claimed conflicts and refused to coexist. So on the same system, I could build either for x84_64, or for armv7l - missing symlinks to libs under their dedicated arch-triplet subdirs (easily reproduced manually), or headers, or pkg-config style scripts. Also there were issues like glibc headers not passing compiler complaints, unless they were known as system headers and tolerated so. This reinforced distrust to Debian native readiness for cross-compilation of a larger project ;) (My next idea was to use containers - a purely ARM container ran its binaries under QEMU so was slow as hell, and I have yet to try an X86 container with :armhf packages duplicating the :amd64 packages, and having preference in case of conflict - so I'd build x86 and arm variants in different environments with link/search/... paths from proper OS packaging... but I have yet to do so).

So one way or another, during that journey I stumbled on dstate.h not having a struct timeval (so found and added the timehead.h there), and looking at precedents in codebase I found that upsclient includes both sys/time.h and timehead.h seemingly superfluously.

If 8de8b8e was wrong to remove it, there may be a point in just copy-pasting the timehead.h contents (a dozen lines or so) into the places where we include the (sys/)time.h based on autoconf inspection of the build host, and drop the small include file altogether?