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…