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?

Again sorry for not double-checking the patch…

Reply via email to