On 8. 1. 25 09:47, Timofei Zhakov wrote:
On Sun, 22 Dec 2024 at 12:42 AM, Daniel Sahlberg <
daniel.l.sahlb...@gmail.com> wrote:

HI Timofei,

Den fre 20 dec. 2024 kl 14:21 skrev Timofei Zhakov<t...@chemodax.net>:

Hi,

I just noticed that the CMake build system doesn't enable the
SPNEGO/SSPI module.

This could be fixed by adding the SERF_HAVE_SSPI definition when running
it on the Windows platform. I made a patch that fixes it. You may find it
below and attached to the email as 'serf-fix-sspi.patch.txt'.

[[[
cmake: Enable SPNEGO/SSPI module on Windows by adding the SERF_HAVE_SSPI
compile definition.

* CMakeLists.txt
   (windows compile options): Define SERF_HAVE_SSPI.

Patch by: rinrab

Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt (revision 3393)
+++ CMakeLists.txt (revision 3394)


@@ -252,6 +252,7 @@
      "/DNOUSER" "/DNOGDI" "/DNONLS" "/DNOCRYPT"
      "/D_CRT_SECURE_NO_WARNINGS"
      "/D_CRT_NONSTDC_NO_WARNINGS"
+    "/DSERF_HAVE_SSPI"
    )
    if(SERF_WIN64)
      add_definitions("/DWIN64")
]]]

CMakeList.txt already has this [1]:

[[[
add_definitions("-DSERF_HAVE_SSPI")
]]]
(added by Kotkov in r1901041).

Barring my (lack of) understanding of CMake, shouldn't this accomplish
what you try to do (although, it is under if (SERF_WINDOWS) compared to
your patch if (MSVC)).

The revision numbers and line numbers in your patch doesn't really match
with our repository, I assume they come from the Poshsvn repo. The
CMakeList.txt file there differs a bit from what we have on trunk. Can you
take a second look at this?

Cheers,
Daniel


[1]
https://svn.apache.org/viewvc/serf/trunk/CMakeLists.txt?revision=1909406&view=markup#l181

Hi Daniel!

Thanks for responding this email.

Sorry, I didn’t notice that it has been already added in the trunk before.
I’ve checked it, but didn’t find this definition set up, since I was using
the search at GitHub, which shows only the exact matches, following that
search for SERF_HAVE_SSPI didn’t show any result.

Yes, this patch was made from the PoshSvn repo, which had this committed to
its Serf copy. However, I used cmake of the 1.7.x stable versions there,
which didn’t have this definition added. I think that it might be helpful
to backport this change to the stable branch, so we have the feature
enabled. What do you think?

I think that using an experimental build system is quite a daring proposition, hm? Especially given that the 1.4.x version is quite ancient by now, and was never released. The only "stable" branch is 1.3.x, which doesn't support a CMake build.

So what does PoshSvn actually use? 1.3.x with nailed-on CMake, or the unreleased 1.4.x?

Anyway, yes, that should be backported from trunk, even though it has already been superseded. As should all the other CMake build changes that are on trunk but not in 1.4.x.

I'll try to update 1.4.x/STATUS with all the myriad changes now in flight, but I suspect that at some point, we'll just decide to bulk-merge from trunk to that branch.

-- Brane

Reply via email to