Skip to content
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

util: detect all possible qsort_r and qsort_s variants #6555

Merged
merged 1 commit into from
May 9, 2023

Conversation

DimitryAndric
Copy link
Contributor

As reported in https://bugs.freebsd.org/271234, recent versions of FreeBSD have adjusted the prototype for qsort_r() to match the POSIX interface. This causes libgit2's CMake configuration check to fail to detect qsort_r(), making it fall back to qsort_s(), which in libgit2 also has an incompatible interface. With recent versions of clang this results in a "incompatible function pointer types" compile error.

Summarizing, there are four variations of 'qsort-with-context':

  • old style BSD qsort_r(), used in FreeBSD 13 and earlier, where the comparison function has the context parameter first
  • GNU or POSIX qsort_r(), also used in FreeBSD 14 and later, where the comparison function has the context parameter last
  • C11 qsort_s(), where the comparison function has the context parameter last
  • Microsoft qsort_s(), where the comparison function has the context parameter first

Add explicit detections for all these variants, so they get detected as (in the same order as above):

  • GIT_QSORT_R_BSD
  • GIT_QSORT_R_GNU
  • GIT_QSORT_S_C11
  • GIT_QSORT_S_MSC

An additional complication is that on FreeBSD 14 and later, <stdlib.h> uses the C11 _Generic() macro mechanism to automatically select the correct qsort_r() prototype, depending on the caller's comparison function argument. This breaks CMake's check_prototype_definition() functionality, since it tries to redefine the function, and the _Generic macro is expanded inline causing a compile error.

Work around that problem by putting the function names in parentheses, to prevent the preprocessor from using a macro to replace the function name.

@emaste
Copy link

emaste commented May 3, 2023

Do both the GIT_QSORT_R_BSD and GIT_QSORT_R_GNU tests succeed on FreeBSD 14 (via _Generic)?

"" "stdlib.h" GIT_QSORT_R_BSD)

# new-style BSD and GNU qsort_r() has the 'thunk' parameter as the last
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is maybe "new style FreeBSD qsort_r" -- I don't know that other BSDs have picked it up?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use POSIX qsort_r? (reference: https://www.austingroupbugs.net/view.php?id=900 )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither NetBSD nor OpenBSD even have qsort_r() (and they don't have qsort_s() either), so technically it is more correct to use the term 'FreeBSD', or indeed just 'POSIX' here.

I guess the "old-style BSD" comment should then really be "old-style FreeBSD", but I'm inclined to leave the GIT_QSORT_R_BSD as it has always been.

src/util/util.c Outdated
@@ -726,7 +727,9 @@ void git__qsort_r(
qsort_r(els, nel, elsize, &glue, git__qsort_r_glue_cmp);
#elif defined(GIT_QSORT_R_GNU)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code uses glue when dealing with traditional BSD qsort_r. When both are present, it seems that GIT_QSORT_R_GNU should be preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They won't both be present, either the old variant or the new variant will be detected at configure time, but not both. Still, putting the most preferred variants at the top is less ambiguous, I guess.

Copy link

@delphij delphij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the detection should prefer the POSIX/GNU qsort_r() variant when both are available, because the support of traditional BSD style qsort_r() is done with glues.

@DimitryAndric
Copy link
Contributor Author

Do both the GIT_QSORT_R_BSD and GIT_QSORT_R_GNU tests succeed on FreeBSD 14 (via _Generic)?

Not at the moment, but this is a bug, see below:

-- Checking prototype qsort_r for GIT_QSORT_R_BSD
-- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
-- Checking prototype qsort_r for GIT_QSORT_R_GNU
-- Checking prototype qsort_r for GIT_QSORT_R_GNU - True
-- Checking prototype qsort_s for GIT_QSORT_S_C11
-- Checking prototype qsort_s for GIT_QSORT_S_C11 - False
-- Checking prototype qsort_s for GIT_QSORT_S_MSC
-- Checking prototype qsort_s for GIT_QSORT_S_MSC - False

The way CMake tests prototypes is by using a template program (in /usr/local/share/cmake/Modules/CheckPrototypeDefinition.c.in), containing the required headers, and it pastes the prototype in there, with an empty body:

@CHECK_PROTOTYPE_DEFINITION_PROTO@ {
  return @CHECK_PROTOTYPE_DEFINITION_RETURN@;
}

If this 'fake' definition does not exactly match the declaration in the header, it causes a compile error. For example, for the GIT_QSORT_R_BSD variant:

        Run Build Command(s):/usr/local/bin/cmake -E env VERBOSE=1 /usr/local/bin/gmake -f Makefile cmTC_bb3e2/fast && /usr/local/bin/gmake  -f CMakeFiles/cmTC_bb3e2.dir/build.make CMakeFiles/cmTC_bb3e2.dir/build
        gmake[1]: Entering directory '/share/dim/src/libgit2/build/CMakeFiles/CMakeScratch/TryCompile-b5ZoXk'
        Building C object CMakeFiles/cmTC_bb3e2.dir/CheckPrototypeDefinition.c.o
        /usr/bin/cc   -D_GNU_SOURCE  -Wall -Wextra -fvisibility=hidden -fPIC -Wdocumentation -Wno-documentation-deprecated-sync -Wno-missing-field-initializers -Wmissing-declarations -Wstrict-aliasing -Wstrict-prototypes -Wdeclaration-after-statement -Wshift-count-overflow -Wunused-const-variable -Wunused-function -Wint-conversion -Wc11-extensions -Wformat -Wformat-security  -MD -MT CMakeFiles/cmTC_bb3e2.dir/CheckPrototypeDefinition.c.o -MF CMakeFiles/cmTC_bb3e2.dir/CheckPrototypeDefinition.c.o.d -o CMakeFiles/cmTC_bb3e2.dir/CheckPrototypeDefinition.c.o -c /share/dim/src/libgit2/build/CMakeFiles/CMakeScratch/TryCompile-b5ZoXk/CheckPrototypeDefinition.c
        /share/dim/src/libgit2/build/CMakeFiles/CMakeScratch/TryCompile-b5ZoXk/CheckPrototypeDefinition.c:14:7: error: conflicting types for 'qsort_r'
        void (qsort_r)(void *base, size_t nmemb, size_t size, void *thunk, int (*compar)(void *, const void *, const void *)) {
              ^
        /usr/include/stdlib.h:314:7: note: previous declaration is here
        void     qsort_r(void *, size_t, size_t,
                 ^
        1 error generated.

I see now that I used the CMake check_prototype_definition() function incorrectly, as you have to specify a suitable return value in the third argument, in this case plain 0 will suffice for a errno_t.

When I fix that, both GIT_QSORT_R_GNU and GIT_QSORT_S_C11 get detected:

-- Checking prototype qsort_r for GIT_QSORT_R_BSD
-- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
-- Checking prototype qsort_r for GIT_QSORT_R_GNU
-- Checking prototype qsort_r for GIT_QSORT_R_GNU - True
-- Checking prototype qsort_s for GIT_QSORT_S_C11
-- Checking prototype qsort_s for GIT_QSORT_S_C11 - True
-- Checking prototype qsort_s for GIT_QSORT_S_MSC
-- Checking prototype qsort_s for GIT_QSORT_S_MSC - False

From the logic in https://github.com/libgit2/libgit2/blob/main/src/util/util.c#L724, you can see that GIT_QSORT_R_GNU is checked first, so if both GIT_QSORT_R_GNU and GIT_QSORT_S_C11 are detected at configure time, the GIT_QSORT_R_GNU variant is actually used.

Copy link

@delphij delphij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

As reported in https://bugs.freebsd.org/271234, recent versions of
FreeBSD have adjusted the prototype for qsort_r() to match the POSIX
interface. This causes libgit2's CMake configuration check to fail to
detect qsort_r(), making it fall back to qsort_s(), which in libgit2
also has an incompatible interface. With recent versions of clang this
results in a "incompatible function pointer types" compile error.

Summarizing, there are four variations of 'qsort-with-context':
* old style BSD qsort_r(), used in FreeBSD 13 and earlier, where the
  comparison function has the context parameter first
* GNU or POSIX qsort_r(), also used in FreeBSD 14 and later, where the
  comparison function has the context parameter last
* C11 qsort_s(), where the comparison function has the context parameter
  last
* Microsoft qsort_s(), where the comparison function has the context
  parameter first

Add explicit detections for all these variants, so they get detected as
(in the same order as above):

* `GIT_QSORT_R_BSD`
* `GIT_QSORT_R_GNU`
* `GIT_QSORT_S_C11`
* `GIT_QSORT_S_MSC`

An additional complication is that on FreeBSD 14 and later, <stdlib.h>
uses the C11 _Generic() macro mechanism to automatically select the
correct qsort_r() prototype, depending on the caller's comparison
function argument. This breaks CMake's check_prototype_definition()
functionality, since it tries to redefine the function, and _Generic
macro is expanded inline causing a compile error.

Work around that problem by putting the function names in parentheses,
to prevent the preprocessor from using a macro to replace the function
name.

Also, in `git__qsort_r()`, change the `#if` order so the variants that
do not have to use glue are preferred.
@ethomson
Copy link
Member

ethomson commented May 9, 2023

Thanks for the PR! I'm not seeing a system sort get selected on our Linux or macOS CI builds, which I would have expected. Is my expectation wrong or do we need to adjust the cmake magic a bit?

@DimitryAndric
Copy link
Contributor Author

Thanks for the PR! I'm not seeing a system sort get selected on our Linux or macOS CI builds, which I would have expected. Is my expectation wrong or do we need to adjust the cmake magic a bit?

Hmm that's an interesting one, I see for example with the Ubuntu Xenial build (https://github.com/libgit2/libgit2/actions/runs/4918385982/jobs/8796807358?pr=6555):

 -- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
-- Checking prototype qsort_r for GIT_QSORT_R_GNU - False
-- Checking prototype qsort_s for GIT_QSORT_S_C11 - False
-- Checking prototype qsort_s for GIT_QSORT_S_MSC - False

But when I spin up a local Xenial container, and run the build in there, it works as expected (it only finds the GNU/POSIX variant):

-- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
-- Checking prototype qsort_r for GIT_QSORT_R_GNU - True
-- Checking prototype qsort_s for GIT_QSORT_S_C11 - False
-- Checking prototype qsort_s for GIT_QSORT_S_MSC - False

Similar for the macOS build (https://github.com/libgit2/libgit2/actions/runs/4918385982/jobs/8796807907?pr=6555) which shows:

 -- Checking prototype qsort_r for GIT_QSORT_R_BSD
-- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
-- Checking prototype qsort_r for GIT_QSORT_R_GNU
-- Checking prototype qsort_r for GIT_QSORT_R_GNU - False
-- Checking prototype qsort_s for GIT_QSORT_S_C11
-- Checking prototype qsort_s for GIT_QSORT_S_C11 - False
-- Checking prototype qsort_s for GIT_QSORT_S_MSC
-- Checking prototype qsort_s for GIT_QSORT_S_MSC - False

whereas on my own Mac (M1 with macOS 13.3.1 and Xcode 14.3) it correctly detects only the old-style BSD variant:

-- Checking prototype qsort_r for GIT_QSORT_R_BSD
-- Checking prototype qsort_r for GIT_QSORT_R_BSD - True
-- Checking prototype qsort_r for GIT_QSORT_R_GNU
-- Checking prototype qsort_r for GIT_QSORT_R_GNU - False
-- Checking prototype qsort_s for GIT_QSORT_S_C11
-- Checking prototype qsort_s for GIT_QSORT_S_C11 - False
-- Checking prototype qsort_s for GIT_QSORT_S_MSC
-- Checking prototype qsort_s for GIT_QSORT_S_MSC - False

Can anybody get at the build/CMakeFiles/CMakeError.log files in the failing CI containers? Whatever goes wrong should be visible in there.

@DimitryAndric
Copy link
Contributor Author

Ah, I could pull the container image used for building, and reproduce the problem in there, but it's due to -DENABLE_WERROR=ON being used in these CI jobs:

FAILED: /usr/bin/cc    -D_GNU_SOURCE  -Werror -Werror -Wall -Wextra -fvisibility=hidden -fPIC -Wno-documentation-deprecated-sync -Wno-missing-field-initializers -Wmissing-declarations -Wstrict-aliasing -Wstrict-prototypes -Wdeclaration-after-statement -Wshift-count-overflow -Wunused-function -Wint-conversion -Wc99-c11-compat -Wformat -Wformat-security -o CMakeFiles/cmTC_da6d8.dir/CheckPrototypeDefinition.c.o   -c CheckPrototypeDefinition.c
CheckPrototypeDefinition.c: In function 'qsort_r':
CheckPrototypeDefinition.c:14:22: error: unused parameter 'base' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                      ^
CheckPrototypeDefinition.c:14:35: error: unused parameter 'nmemb' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                   ^
CheckPrototypeDefinition.c:14:49: error: unused parameter 'size' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                 ^
CheckPrototypeDefinition.c:14:61: error: unused parameter 'compar' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                             ^
CheckPrototypeDefinition.c:14:112: error: unused parameter 'context' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                                                                                ^
CheckPrototypeDefinition.c: At top level:
cc1: error: unrecognized command line option '-Wno-documentation-deprecated-sync' [-Werror]
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

The test function added by CMake always has an empty body, so the incoming parameters are indeed purposefully unused. The warning should be suppressed, but it's tricky to do this in a platform-independent way, i..e __attribute__((unused)) is specific to gcc and clang, and not supported by MSVC.

Note also the error on -Wno-documentation-deprecated-sync, but I have no idea where that comes from, not from the changes in this pull request, in any case.

2 similar comments
@DimitryAndric
Copy link
Contributor Author

Ah, I could pull the container image used for building, and reproduce the problem in there, but it's due to -DENABLE_WERROR=ON being used in these CI jobs:

FAILED: /usr/bin/cc    -D_GNU_SOURCE  -Werror -Werror -Wall -Wextra -fvisibility=hidden -fPIC -Wno-documentation-deprecated-sync -Wno-missing-field-initializers -Wmissing-declarations -Wstrict-aliasing -Wstrict-prototypes -Wdeclaration-after-statement -Wshift-count-overflow -Wunused-function -Wint-conversion -Wc99-c11-compat -Wformat -Wformat-security -o CMakeFiles/cmTC_da6d8.dir/CheckPrototypeDefinition.c.o   -c CheckPrototypeDefinition.c
CheckPrototypeDefinition.c: In function 'qsort_r':
CheckPrototypeDefinition.c:14:22: error: unused parameter 'base' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                      ^
CheckPrototypeDefinition.c:14:35: error: unused parameter 'nmemb' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                   ^
CheckPrototypeDefinition.c:14:49: error: unused parameter 'size' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                 ^
CheckPrototypeDefinition.c:14:61: error: unused parameter 'compar' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                             ^
CheckPrototypeDefinition.c:14:112: error: unused parameter 'context' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                                                                                ^
CheckPrototypeDefinition.c: At top level:
cc1: error: unrecognized command line option '-Wno-documentation-deprecated-sync' [-Werror]
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

The test function added by CMake always has an empty body, so the incoming parameters are indeed purposefully unused. The warning should be suppressed, but it's tricky to do this in a platform-independent way, i..e __attribute__((unused)) is specific to gcc and clang, and not supported by MSVC.

Note also the error on -Wno-documentation-deprecated-sync, but I have no idea where that comes from, not from the changes in this pull request, in any case.

@DimitryAndric
Copy link
Contributor Author

Ah, I could pull the container image used for building, and reproduce the problem in there, but it's due to -DENABLE_WERROR=ON being used in these CI jobs:

FAILED: /usr/bin/cc    -D_GNU_SOURCE  -Werror -Werror -Wall -Wextra -fvisibility=hidden -fPIC -Wno-documentation-deprecated-sync -Wno-missing-field-initializers -Wmissing-declarations -Wstrict-aliasing -Wstrict-prototypes -Wdeclaration-after-statement -Wshift-count-overflow -Wunused-function -Wint-conversion -Wc99-c11-compat -Wformat -Wformat-security -o CMakeFiles/cmTC_da6d8.dir/CheckPrototypeDefinition.c.o   -c CheckPrototypeDefinition.c
CheckPrototypeDefinition.c: In function 'qsort_r':
CheckPrototypeDefinition.c:14:22: error: unused parameter 'base' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                      ^
CheckPrototypeDefinition.c:14:35: error: unused parameter 'nmemb' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                   ^
CheckPrototypeDefinition.c:14:49: error: unused parameter 'size' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                 ^
CheckPrototypeDefinition.c:14:61: error: unused parameter 'compar' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                             ^
CheckPrototypeDefinition.c:14:112: error: unused parameter 'context' [-Werror=unused-parameter]
 void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context) {
                                                                                                                ^
CheckPrototypeDefinition.c: At top level:
cc1: error: unrecognized command line option '-Wno-documentation-deprecated-sync' [-Werror]
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

The test function added by CMake always has an empty body, so the incoming parameters are indeed purposefully unused. The warning should be suppressed, but it's tricky to do this in a platform-independent way, i..e __attribute__((unused)) is specific to gcc and clang, and not supported by MSVC.

Note also the error on -Wno-documentation-deprecated-sync, but I have no idea where that comes from, not from the changes in this pull request, in any case.

@ethomson
Copy link
Member

ethomson commented May 9, 2023

Ah, I could pull the container image used for building, and reproduce the problem in there, but it's due to -DENABLE_WERROR=ON being used in these CI jobs:

Aha. Thanks for digging deep on this. That makes sense.

I don't see an easy way around this. 🙃

Thanks again for the fix.

@ethomson ethomson merged commit 251408c into libgit2:main May 9, 2023
@DimitryAndric
Copy link
Contributor Author

Actually I have a fix lined up for this, but I'll put it in another pull request :)

@DimitryAndric
Copy link
Contributor Author

See #6558.

netgate-git-updates pushed a commit to pfsense/FreeBSD-ports that referenced this pull request May 14, 2023
Clang 16 has a new error about incompatible function types, which shows
up when building devel/libgit2:

  /wrkdirs/usr/ports/devel/libgit2/work/libgit2-1.5.2/src/util/util.c:731:28: error: incompatible function pointer types passing 'int (void *, const void *, const void *)' to parameter of type 'int (*)(const void *, const void *, void *)' [-Wincompatible-function-pointer-types]
          qsort_s(els, nel, elsize, git__qsort_r_glue_cmp, &glue);
                                    ^~~~~~~~~~~~~~~~~~~~~
  /usr/include/stdlib.h:397:11: note: passing argument to parameter here
      int (*)(const void *, const void *, void *), void *);
            ^

Clang is indeed right, as the version of qsort_s(3) in FreeBSD 13 and
later has the 'void *payload' parameter last:

  errno_t  qsort_s(void *, rsize_t, rsize_t,
      int (*)(const void *, const void *, void *), void *);

This could be fixed by putting the arguments in the right place for
qsort_s(3), but it turns out the rabbit hole goes a bit deeper: on
14-CURRENT, libgit2's CMake configuration is not able to detect
qsort_r(3), which is actually why it chooses qsort_s():

  -- Checking prototype qsort_r for GIT_QSORT_R_BSD
  -- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
  -- Checking prototype qsort_r for GIT_QSORT_R_GNU
  -- Checking prototype qsort_r for GIT_QSORT_R_GNU - False
  -- Looking for qsort_s
  -- Looking for qsort_s - found

The problem with the GIT_QSORT_R_BSD detection is due to the check in
libgit2's src/CMakeLists.txt, where it does:

  check_prototype_definition(qsort_r
          "void qsort_r(void *base, size_t nmemb, size_t size, void
          *thunk, int (*compar)(void *, const void *, const void *))"
          "" "stdlib.h" GIT_QSORT_R_BSD)

and CMake attempts to define a function with a similar prototype in its
test program, which then fails to compile, at least on 14-CURRENT:

  /wrkdirs/share/dim/ports/devel/libgit2/work/.build/CMakeFiles/CMakeScratch/TryCompile-tILE28/CheckPrototypeDefinition.c:14:6:
  error: expected identifier or '('
  void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int
  (*compar)(void *, const void *, const void *)) {
       ^
  /usr/include/stdlib.h:357:5: note: expanded from macro 'qsort_r'
      __generic(arg5, int (*)(void *, const void *, const void *),
       \\
      ^
  /usr/include/sys/cdefs.h:323:2: note: expanded from macro '__generic'
          _Generic(expr, t: yes, default: no)
          ^

This is because in https://cgit.freebsd.org/src/commit/?id=af3c78886fd8d
Ed Schouten changed the prototype of qsort_r(3) to match POSIX, using a
C11 _Generic macro. When CMake tries to compile its own custom
definition of qsort_r, that fails with the above compile error, because
the macro gets expanded in place of the function declaration.

So the summarized situation is:

* On 12.x and 13.x, qsort_r(3) is a plain function, and uses the old
  comparison function type: 'int (*compar)(void *thunk, const void
  *elem1, const void *elem2)'.
  Therefore, CMake detects GIT_QSORT_R_BSD, and libgit2's
  src/util/util.c uses the correct comparison function type.
* On 14.x, qsort_r(3) is a macro, and uses the POSIX comparison function
  type: 'int (*compar)(const void *elem1, const void *elem2, void *thunk)'.
  Therefore, CMake fails to detect GIT_QSORT_R_BSD, and detects
  GIT_QSORT_S instead, and libgit2's src/util/util.c uses an incorrect
  comparison function type.

I submitted libgit2/libgit2#6555 and
libgit2/libgit2#6558 upstream to remedy the
situation and correctly detect all qsort variants, and these were merged
with a few minor changes.

This is an adjusted version of the upstream commits that applies on top
of libgit-1.5.2.

PR:		271234
Approved by:	mfechner (maintainer)
MFH:		2023Q2
@ethomson ethomson added the build label Jul 15, 2023
@mfechner
Copy link

@ethomson do you plan to back-port this also to 1.5 and 1.6 branch? Should I create a new Merge Request for this?
It is needed e.g. for gitaly on FreeBSD as gitaly currently depends on git2go which depends on libgit2 1.5.

@mfechner
Copy link

I added the required patches for FreeBSD for version 1.5 now here:
https://github.com/mfechner/libgit2/tree/maint/v1.5

This version is also used for gitaly to make to compilable on FreeBSD 14 which is included here:
https://cgit.freebsd.org/ports/commit/?id=64cfdbdb78b0806923a161f5a14d967d75b9fe43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants