Re: [PATCH 1/1] Fix detection of pwritev support for OSX.

On Fri, Jan 22, 2021 at 1:53 PM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> On Thu, Jan 21, 2021 at 11:38 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > James Hilliard <james.hilliard1@gmail.com> writes:
> > > On Wed, Jan 20, 2021 at 4:07 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >> I'm not sure that the case of not having the "command line tools"
> > >> installed is interesting for our purposes.  AFAIK you have to have
> > >> that in order to have access to required tools like bison and gmake.
> > >> (That reminds me, I was intending to add something to our docs
> > >> about how-to-build-from-source to say that you need to install those.)
> >
> > > Yeah, not 100% sure but I was able to build just fine after deleting my
> > > command line tools.
> >
> > Hm.  I've never been totally clear on what's included in the "command line
> > tools", although it's now apparent that one thing that gets installed is
> > an SDK matching the host OS version.  However, Apple's description at [1]
> > says
> >
> >     Command Line Tools
> >
> >     Download the macOS SDK, headers, and build tools such as the Apple
> >     LLVM compiler and Make. These tools make it easy to install open
> >     source software or develop on UNIX within Terminal. macOS can
> >     automatically download these tools the first time you try to build
> >     software, and they are available on the downloads page.
> >
> > which certainly strongly implies that gmake is not there otherwise.
> > At this point I lack any "bare" macOS system to check it on.  I wonder
> > whether you have a copy of make available from MacPorts or Homebrew.
> > Or maybe uninstalling the command line tools doesn't really remove
> > everything?
> >
> > > It would be pretty annoying to have to install an outdated SDK just to
> > > build postgres for no other reason than the autoconf feature detection
> > > being broken.
> >
> > It's only as "outdated" as your host system ;-).  Besides, it doesn't
> > look like Apple's really giving you a choice not to.
> >
> > The long and short of this is that I'm unwilling to buy into maintaining
> > our own substitutes for standard autoconf probes in order to make it
> > possible to use the wrong SDK version.  The preadv/pwritev case is already
> > messy enough, and I fear that trying to support such scenarios is going to
> > lead to more and more pain in the future.
>
> I found a cleaner alternative to the compile test that appears to work:
> https://postgr.es/m/20210122193230.25295-1-james.hilliard1%40gmail.com
>
> Best I can tell the target deployment version check logic requires that the
> <sys/uio.h> header be included in order for the check to function properly.
>
> It seems we just need to avoid AC_REPLACE_FUNCS for these cases since
> it doesn't allow for passing headers.

I did manage to verify that AC_REPLACE_FUNCS generates an incorrect conftest.c
for OSX which is why it is incorrectly detecting the availability of pwritev.

conftest.c:
/* confdefs.h */
#define PACKAGE_NAME "PostgreSQL"
#define PACKAGE_TARNAME "postgresql"
#define PACKAGE_VERSION "14devel"
#define PACKAGE_STRING "PostgreSQL 14devel"
#define PACKAGE_BUGREPORT "pgsql-bugs@lists.postgresql.org"
#define PACKAGE_URL "https://www.postgresql.org/"
#define CONFIGURE_ARGS ""
#define PG_MAJORVERSION "14"
#define PG_MAJORVERSION_NUM 14
#define PG_MINORVERSION_NUM 0
#define PG_VERSION "14devel"
#define DEF_PGPORT 5432
#define DEF_PGPORT_STR "5432"
#define BLCKSZ 8192
#define RELSEG_SIZE 131072
#define XLOG_BLCKSZ 8192
#define ENABLE_THREAD_SAFETY 1
#define PG_KRB_SRVNAM "postgres"
#define STDC_HEADERS 1
#define HAVE_SYS_TYPES_H 1
#define HAVE_SYS_STAT_H 1
#define HAVE_STDLIB_H 1
#define HAVE_STRING_H 1
#define HAVE_MEMORY_H 1
#define HAVE_STRINGS_H 1
#define HAVE_INTTYPES_H 1
#define HAVE_STDINT_H 1
#define HAVE_UNISTD_H 1
#define HAVE_PTHREAD_PRIO_INHERIT 1
#define HAVE_PTHREAD 1
#define HAVE_STRERROR_R 1
#define HAVE_GETPWUID_R 1
#define STRERROR_R_INT 1
#define HAVE_LIBM 1
#define HAVE_LIBREADLINE 1
#define HAVE_LIBZ 1
#define HAVE_SPINLOCKS 1
#define HAVE_ATOMICS 1
#define HAVE__BOOL 1
#define HAVE_STDBOOL_H 1
#define HAVE_COPYFILE_H 1
#define HAVE_EXECINFO_H 1
#define HAVE_GETOPT_H 1
#define HAVE_IFADDRS_H 1
#define HAVE_LANGINFO_H 1
#define HAVE_POLL_H 1
#define HAVE_SYS_EVENT_H 1
#define HAVE_SYS_IPC_H 1
#define HAVE_SYS_RESOURCE_H 1
#define HAVE_SYS_SELECT_H 1
#define HAVE_SYS_SEM_H 1
#define HAVE_SYS_SHM_H 1
#define HAVE_SYS_SOCKIO_H 1
#define HAVE_SYS_UIO_H 1
#define HAVE_SYS_UN_H 1
#define HAVE_TERMIOS_H 1
#define HAVE_WCTYPE_H 1
#define HAVE_NET_IF_H 1
#define HAVE_SYS_UCRED_H 1
#define HAVE_NETINET_TCP_H 1
#define HAVE_READLINE_READLINE_H 1
#define HAVE_READLINE_HISTORY_H 1
#define PG_PRINTF_ATTRIBUTE printf
#define HAVE_FUNCNAME__FUNC 1
#define HAVE__STATIC_ASSERT 1
#define HAVE_TYPEOF 1
#define HAVE__BUILTIN_TYPES_COMPATIBLE_P 1
#define HAVE__BUILTIN_CONSTANT_P 1
#define HAVE__BUILTIN_UNREACHABLE 1
#define HAVE_COMPUTED_GOTO 1
#define HAVE_STRUCT_TM_TM_ZONE 1
#define HAVE_UNION_SEMUN 1
#define HAVE_STRUCT_SOCKADDR_UN 1
#define HAVE_STRUCT_SOCKADDR_STORAGE 1
#define HAVE_STRUCT_SOCKADDR_STORAGE_SS_FAMILY 1
#define HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN 1
#define HAVE_STRUCT_SOCKADDR_SA_LEN 1
#define HAVE_STRUCT_ADDRINFO 1
#define HAVE_LOCALE_T 1
#define LOCALE_T_IN_XLOCALE 1
#define restrict __restrict
#define pg_restrict __restrict
#define HAVE_STRUCT_OPTION 1
#define HAVE_X86_64_POPCNTQ 1
#define SIZEOF_OFF_T 8
#define SIZEOF_BOOL 1
#define PG_USE_STDBOOL 1
#define HAVE_INT_TIMEZONE 1
#define ACCEPT_TYPE_RETURN int
#define ACCEPT_TYPE_ARG1 int
#define ACCEPT_TYPE_ARG2 struct sockaddr *
#define ACCEPT_TYPE_ARG3 socklen_t
#define WCSTOMBS_L_IN_XLOCALE 1
#define HAVE_BACKTRACE_SYMBOLS 1
#define HAVE_CLOCK_GETTIME 1
#define HAVE_COPYFILE 1
#define HAVE_FDATASYNC 1
#define HAVE_GETIFADDRS 1
#define HAVE_GETRLIMIT 1
#define HAVE_KQUEUE 1
#define HAVE_MBSTOWCS_L 1
#define HAVE_MEMSET_S 1
#define HAVE_POLL 1
#define HAVE_PTHREAD_IS_THREADED_NP 1
#define HAVE_READLINK 1
#define HAVE_READV 1
#define HAVE_SETSID 1
#define HAVE_SHM_OPEN 1
#define HAVE_STRSIGNAL 1
#define HAVE_SYMLINK 1
#define HAVE_USELOCALE 1
#define HAVE_WCSTOMBS_L 1
#define HAVE_WRITEV 1
#define HAVE__BUILTIN_BSWAP16 1
#define HAVE__BUILTIN_BSWAP32 1
#define HAVE__BUILTIN_BSWAP64 1
#define HAVE__BUILTIN_CLZ 1
#define HAVE__BUILTIN_CTZ 1
#define HAVE__BUILTIN_POPCOUNT 1
#define HAVE_FSEEKO 1
#define HAVE_DECL_POSIX_FADVISE 0
#define HAVE_DECL_FDATASYNC 0
#define HAVE_DECL_STRLCAT 1
#define HAVE_DECL_STRLCPY 1
#define HAVE_DECL_STRNLEN 1
#define HAVE_DECL_F_FULLFSYNC 1
#define HAVE_DECL_RTLD_GLOBAL 1
#define HAVE_DECL_RTLD_NOW 1
#define HAVE_IPV6 1
#define HAVE_DLOPEN 1
#define HAVE_FLS 1
#define HAVE_GETOPT 1
#define HAVE_GETPEEREID 1
#define HAVE_GETRUSAGE 1
#define HAVE_INET_ATON 1
#define HAVE_LINK 1
#define HAVE_MKDTEMP 1
#define HAVE_PREAD 1
#define HAVE_PREADV 1
#define HAVE_PWRITE 1
/* end confdefs.h.  */
/* Define pwritev to an innocuous variant, in case <limits.h> declares pwritev.
   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
#define pwritev innocuous_pwritev

/* System header to define __stub macros and hopefully few prototypes,
    which can conflict with char pwritev (); below.
    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
    <limits.h> exists even on freestanding compilers.  */

#ifdef __STDC__
# include <limits.h>
#else
# include <assert.h>
#endif

#undef pwritev

/* Override any GCC internal prototype to avoid an error.
   Use char because int might match the return type of a GCC
   builtin and then its argument prototype would still apply.  */
#ifdef __cplusplus
extern "C"
#endif
char pwritev ();
/* The GNU C library defines this for functions which it implements
    to always fail with ENOSYS.  Some functions are actually named
    something starting with __ and the normal name is an alias.  */
#if defined __stub_pwritev || defined __stub___pwritev
choke me
#endif

int
main ()
{
return pwritev ();
  ;
  return 0;
}

The correct declaration for pwritev on OSX is:
ssize_t pwritev(int, const struct iovec *, int, off_t)
__DARWIN_NOCANCEL(pwritev) __API_AVAILABLE(macos(11.0), ios(14.0),
watchos(7.0), tvos(14.0));
while the conftest.c generated by AC_REPLACE_FUNCS declares:
char pwritev ();
which results in a broken conftest binary.

On OSX if the declaration is missing __API_AVAILABLE then the target
deployment version will not be checked properly and you might end up
with a broken binary.

>
> >
> >                         regards, tom lane
> >
> > [1] https://developer.apple.com/xcode/features/