On Sun, Jun 1, 2025 at 12:56 PM Branko Čibej <br...@apache.org> wrote:
> 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?
>
>
It uses released version 1.3.10, but with a manually backported cmake build
system. Like I copied the files from the trunk and adjusted file lists and
other stuff changed between those versions.
> 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.
>
It would be very useful to get cmake released in serf (and svn btw!) and
quit experimental status. I'm free to help in place of cmake.
--
Timofei Zhakov
