Fix port/pg_iovec.h building extensions on x86_64-darwin
When building pg_cron [1]https://hydra.nixos.org/build/276421287/nixlog/1 or pg_hll [2]https://hydra.nixos.org/build/276419879/nixlog/1 for PG 17 on x86_64-darwin, we
encounter the following build failure in nixpkgs:
/nix/store/2clw9wg933c40f871d8iqbl909lg2yis-postgresql-17.0-dev/include/server/port/pg_iovec.h:71:12:
error: comparison of integers of different signs: 'ssize_t' (aka 'long')
and 'const size_t' (aka 'const unsigned long') [-Werror,-Wsign-compare]
if (part < iov[i].iov_len)
~~~~ ^ ~~~~~~~~~~~~~~
/nix/store/2clw9wg933c40f871d8iqbl909lg2yis-postgresql-17.0-dev/include/server/port/pg_iovec.h:110:12:
error: comparison of integers of different signs: 'ssize_t' (aka 'long')
and 'const size_t' (aka 'const unsigned long') [-Werror,-Wsign-compare]
if (part < iov[i].iov_len)
~~~~ ^ ~~~~~~~~~~~~~~
2 errors generated.
The attached patch fixes those.
Hopefully this can make it into the minor release next week.
Best,
Wolfgang
[1]: https://hydra.nixos.org/build/276421287/nixlog/1
[2]: https://hydra.nixos.org/build/276419879/nixlog/1
Attachments:
0001-Fix-build-on-x86_64-darwin.patchtext/x-patch; charset=UTF-8; name=0001-Fix-build-on-x86_64-darwin.patchDownload
From c906b182b5503c9ceebf58e25e3496157c29dedb Mon Sep 17 00:00:00 2001
From: Wolfgang Walther <walther@technowledgy.de>
Date: Fri, 8 Nov 2024 19:41:00 +0100
Subject: [PATCH] Fix pg_cron/pg_hll build on x86_64-darwin
In file included from src/hll.c:28:
In file included from
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/funcapi.h:21:
In file included from
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/executor/executor.h:17:
In file included from
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/executor/execdesc.h:18:
In file included from
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/nodes/execnodes.h:46:
In file included from
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/utils/sharedtuplestore.h:17:
In file included from
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/storage/fd.h:46:
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/port/pg_iovec.h:71:12:
error: comparison of integers of different signs: 'ssize_t' (aka 'long')
and 'const size_t' (aka 'const unsigned long') [-Werror,-Wsign-compare]
if (part < iov[i].iov_len)
~~~~ ^ ~~~~~~~~~~~~~~
/nix/store/24rwyvsc90nhww5dfdp37cwq4c1vycgi-postgresql-jit-17.0-dev/include/server/port/pg_iovec.h:110:12:
error: comparison of integers of different signs: 'ssize_t' (aka 'long')
and 'const size_t' (aka 'const unsigned long') [-Werror,-Wsign-compare]
if (part < iov[i].iov_len)
~~~~ ^ ~~~~~~~~~~~~~~
2 errors generated.
---
src/include/port/pg_iovec.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/include/port/pg_iovec.h b/src/include/port/pg_iovec.h
index 7255c1bd911..e5fe677b371 100644
--- a/src/include/port/pg_iovec.h
+++ b/src/include/port/pg_iovec.h
@@ -68,7 +68,7 @@ pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
}
sum += part;
offset += part;
- if (part < iov[i].iov_len)
+ if ((size_t) part < iov[i].iov_len)
return sum;
}
return sum;
@@ -107,7 +107,7 @@ pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
}
sum += part;
offset += part;
- if (part < iov[i].iov_len)
+ if ((size_t) part < iov[i].iov_len)
return sum;
}
return sum;
--
2.47.0
On Fri, Nov 08, 2024 at 08:08:06PM +0100, Wolfgang Walther wrote:
@@ -68,7 +68,7 @@ pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
}
sum += part;
offset += part;
- if (part < iov[i].iov_len)
+ if ((size_t) part < iov[i].iov_len)
return sum;
}
return sum;
@@ -107,7 +107,7 @@ pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
}
sum += part;
offset += part;
- if (part < iov[i].iov_len)
+ if ((size_t) part < iov[i].iov_len)
return sum;
}
return sum;
This looks correct to me. At this point in the code, we know that part >=
0, so casting it to an unsigned long ought to be okay.
--
nathan
Here is a new version of the patch with an updated commit message. The
cfbot results [0]https://cirrus-ci.com/build/6599760059039744 look good, so I plan to commit this one shortly unless
someone objects.
[0]: https://cirrus-ci.com/build/6599760059039744
--
nathan
Attachments:
v2-0001-Fix-sign-compare-warnings-in-pg_iovec.h.patchtext/plain; charset=us-asciiDownload
From 5e7156defd3a169323e2bc354c3a3c4cb232fa67 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 8 Nov 2024 15:16:41 -0600
Subject: [PATCH v2 1/1] Fix sign-compare warnings in pg_iovec.h.
The code in question (pg_preadv() and pg_pwritev()) has been around
for a while, but commit 15c9ac3629 moved it to a header file. If
third-party code that includes this header file is built with
-Wsign-compare on a system without preadv() or pwritev(), warnings
ensue. This commit fixes said warnings by casting the result of
pg_pread()/pg_pwrite() to size_t, which should be safe because we
have already checked for a negative value.
Author: Wolfgang Walther
Discussion: https://postgr.es/m/16989737-1aa8-48fd-8dfe-b7ada06509ab%40technowledgy.de
Backpatch-through: 17
---
src/include/port/pg_iovec.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/include/port/pg_iovec.h b/src/include/port/pg_iovec.h
index 7255c1bd91..e5fe677b37 100644
--- a/src/include/port/pg_iovec.h
+++ b/src/include/port/pg_iovec.h
@@ -68,7 +68,7 @@ pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
}
sum += part;
offset += part;
- if (part < iov[i].iov_len)
+ if ((size_t) part < iov[i].iov_len)
return sum;
}
return sum;
@@ -107,7 +107,7 @@ pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
}
sum += part;
offset += part;
- if (part < iov[i].iov_len)
+ if ((size_t) part < iov[i].iov_len)
return sum;
}
return sum;
--
2.39.5 (Apple Git-154)
On Fri, Nov 08, 2024 at 03:20:00PM -0600, Nathan Bossart wrote:
Here is a new version of the patch with an updated commit message. The
cfbot results [0] look good, so I plan to commit this one shortly unless
someone objects.
Committed.
--
nathan
On Sat, Nov 9, 2024 at 8:08 AM Wolfgang Walther <walther@technowledgy.de> wrote:
When building pg_cron [1] or pg_hll [2] for PG 17 on x86_64-darwin, we
encounter the following build failure in nixpkgs:
Out of curiosity, is nixos deliberately using an old macOS deployment
target or SDK, 10.something? I'm just wondering if our feature
detection is working correctly on macOS/x86, because I'd expect real
preadv/pwritev to be there from 11 onwards, and 11 is already out of
support by Apple.
On Sat, Nov 09, 2024 at 11:26:44AM +1300, Thomas Munro wrote:
Out of curiosity, is nixos deliberately using an old macOS deployment
target or SDK, 10.something? I'm just wondering if our feature
detection is working correctly on macOS/x86, because I'd expect real
preadv/pwritev to be there from 11 onwards, and 11 is already out of
support by Apple.
It appears to be working for longfin [0]https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=longfin&dt=2024-11-08%2021%3A36%3A02&stg=configure.
--
nathan
Thomas Munro:
Out of curiosity, is nixos deliberately using an old macOS deployment
target or SDK, 10.something? I'm just wondering if our feature
detection is working correctly on macOS/x86, because I'd expect real
preadv/pwritev to be there from 11 onwards, and 11 is already out of
support by Apple.
There has been a lot of work on the macOS SDK in nixpkgs lately, but it
seems that the default for x86-64 is still v10 [1]https://nixos.org/manual/nixpkgs/unstable/#sec-darwin, yes:
x86_64-darwin uses the 10.12 SDK by default, but some software is not
compatible with that version of the SDK. In that case, the 11.0 SDK
used by aarch64-darwin is available for use on x86_64-darwin. [...]
It seems like the goal is to bump x86-64 to v11 in the next release
cycle [2]https://discourse.nixos.org/t/on-the-future-of-darwin-sdks-or-how-you-can-stop-worrying-and-put-the-sdk-in-build-inputs/50574/11.
Best,
Wolfgang
[1]: https://nixos.org/manual/nixpkgs/unstable/#sec-darwin
[2]: https://discourse.nixos.org/t/on-the-future-of-darwin-sdks-or-how-you-can-stop-worrying-and-put-the-sdk-in-build-inputs/50574/11
https://discourse.nixos.org/t/on-the-future-of-darwin-sdks-or-how-you-can-stop-worrying-and-put-the-sdk-in-build-inputs/50574/11