Tarsnap / tarsnap

Command-line client code for Tarsnap.
https://tarsnap.com
Other
865 stars 60 forks source link

Build fails on NetBSD #522

Closed gperciva closed 2 years ago

gperciva commented 2 years ago

A rather convoluted chain of headers leads to "sysendian.h" beind included before <sys/endian.h> on NetBSD 8.1. That's a bit old (2019-05-31), but tarsnap 1.0.39 works on it, so this would be a build failure on a platform which previously worked.

Our "sysendian.h" avoids namespace collisions with BSD if it's included after <sys/endian.h>, but that method doesn't help if <sys/endian.h> is included first.

How this arises:

I think this problem arose in 2019-05-07 lib/crypto: remove unnecessary "bsdtar_platform.h" 1d4e106295bcf1453c46b28eba514aea1a1561fc

Solution 1) at any rate, if we slap a

#ifdef __NetBSD__
#include "bsdtar_platform.h"
#endif

at the top of crypto_file.c and crypto_session.c, then it compiles fine on NetBSD. (I haven't tracked down exactly which included header from bsdtar_platform.h includes <sys/endian.h>, but there's a bunch of files included, including archive.h and archive_entry.h, so I'm sure it's in there somewhere.)

Solution 2) would be to add

#ifdef __NetBSD__
#include "sys/endian.h"
#endif

to the top of our sysendian.h. That has the advantage of completely taking care of any potential problems from the order of headers, and being a NetBSD-specific solution which won't affect any other platforms. It has the disadvantage of being platform-specific code in libcperciva, which we don't want.

Solution 3) would be to shove the platform-specific

#ifdef __NetBSD__
/* Make sure this is included before our "sysendian.h". */
#include "sys/endian.h"
#endif

to crypto_file.c and crypto_session.c. That way, libcperciva remains pure.

gperciva commented 2 years ago

I poked around a little bit in the OpenSSL and NetBSD github repositories. I can't see that

#ifdef __NetBSD__
#include <sys/types.h>
#endif

in <openssl/aes.h>, but I also didn't notice a commit that said anything like "remove NetBSD include". I'm completely willing to believe that just didn't notice one. (I didn't look very hard, since I'm not certain if such code would exist in openssl, or netbsd, or a separate "netbsd ports" git repository.)

Still, tarsnap 1.0.39 worked in netbsd 8.1, so it would be nice if our git master still worked on that platform.

cperciva commented 2 years ago

Our "sysendian.h" avoids namespace collisions with BSD if it's included after <sys/endian.h>, but that method doesn't help if <sys/endian.h> is included first.

I don't think this says what you meant to say.

cperciva commented 2 years ago

Possible solution:

  1. Remove <openssl/aes.h> from crypto_internal.h since it doesn't seem to be used any more.
  2. Replace RSA * with void * in the APIs and drop <openssl/rsa.h> from that file too.
gperciva commented 2 years ago

Fixed in #524.