You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@serf.apache.org by Timofei Zhakov <ti...@chemodax.net> on 2024/12/20 13:19:15 UTC
[PATCH] cmake: Enable SPNEGO/SSPI module on Windows
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")
]]]
Thanks!
--
Timofei Zhakov
Re: [PATCH] cmake: Enable SPNEGO/SSPI module on Windows
Posted by Branko Čibej <br...@apache.org>.
On 1. 6. 25 23:00, Daniel Sahlberg wrote:
> Den sön 1 juni 2025 kl 13:58 skrev Branko Čibej <br...@apache.org>:
>
> On 1. 6. 25 13:20, Timofei Zhakov wrote:
> > On Sun, Jun 1, 2025 at 12:56 PM Branko Čibej <br...@apache.org>
> wrote:
> > 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.
>
> Cool! CMake on trunk is "done", in the sense that it has feature
> parity
> with the SCons build. But it needs a lot of testing and review --
> see my
> other mail, subject "CMake build status" from 30 May. I've tested
> it on
> Windows, but it would be nice to have more eyes on that. It would be
> even nicer to test a Subversion build with serf-trunk, I didn't get
> around to that yet.
>
> And backport to 1.4.x, of course.
>
> And finally a release from 1.4.x ... it's time.
>
> -- Brane
>
>
> Would it make sense to completely replace the current 1.4.x with
> /trunk, or even declare 1.4.x "not released" and prepare a 1.5.x
> branch instead?
*shrug* Version numbers are cheap.
> Could someone summarize, or help me how to figure out, the major
> differences between 1.3.x and trunk? I think I've seen something about
> HTTPv2 support, but I don't know how complete it is. Would there be
> any major issues for our users?
Quite a lot of changes there. Let's see, off the top of my head -- most
of these are also on the 1.4.x branch:
* Support for Brotli decoding (but not encoding).
* HTTP/2, and you'd have to ask the authors how complete it is.
* Support for OCSP verification (1.3.x has only OCSP stapling, IIRC,
trunk/1.4.x has the full OCSP request/response workflow)
* Python ctypes bindings, which have probably already bitroted; I
don't know if anyone is using them.
* The CMake build
* The new protocols and connections API, which Bert was working on,
but it's disabled by default (#if 0) -- that's meant for acutal
serf-2.0, I think, not for 1.x.
* I've started working on a simple mechanism to allow users to
register new authentication scheme implementations at runtime. This
grew from the discussion with Peter Balogh about 2FA; turns out that
authentication schemes are currently hard-coded in the library, to
add a new one you have to implement it in Serf, users can't just
register their own handlers.
Plus bug fixes and so on.
Also it appears that the SCons build doesn't work with SCons 4.x earlier
than 4.7, now I'll have to set up Debian-stable VM to debug that. :(
-- Brane
Re: [PATCH] cmake: Enable SPNEGO/SSPI module on Windows
Posted by Daniel Sahlberg <da...@gmail.com>.
Den sön 1 juni 2025 kl 13:58 skrev Branko Čibej <br...@apache.org>:
> On 1. 6. 25 13:20, Timofei Zhakov wrote:
> > On Sun, Jun 1, 2025 at 12:56 PM Branko Čibej <br...@apache.org> wrote:
> > 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.
>
> Cool! CMake on trunk is "done", in the sense that it has feature parity
> with the SCons build. But it needs a lot of testing and review -- see my
> other mail, subject "CMake build status" from 30 May. I've tested it on
> Windows, but it would be nice to have more eyes on that. It would be
> even nicer to test a Subversion build with serf-trunk, I didn't get
> around to that yet.
>
> And backport to 1.4.x, of course.
>
> And finally a release from 1.4.x ... it's time.
>
> -- Brane
Would it make sense to completely replace the current 1.4.x with /trunk, or
even declare 1.4.x "not released" and prepare a 1.5.x branch instead?
Could someone summarize, or help me how to figure out, the major
differences between 1.3.x and trunk? I think I've seen something about
HTTPv2 support, but I don't know how complete it is. Would there be any
major issues for our users?
Kind regards,
Daniel
Re: [PATCH] cmake: Enable SPNEGO/SSPI module on Windows
Posted by Branko Čibej <br...@apache.org>.
On 1. 6. 25 13:20, Timofei Zhakov wrote:
> 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.sahlberg@gmail.com> wrote:
> >
> >> HI Timofei,
> >>
> >> Den fre 20 dec. 2024 kl 14:21 skrev Timofei
> Zhakov<ti...@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
> <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.
OK.
> 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.
Cool! CMake on trunk is "done", in the sense that it has feature parity
with the SCons build. But it needs a lot of testing and review -- see my
other mail, subject "CMake build status" from 30 May. I've tested it on
Windows, but it would be nice to have more eyes on that. It would be
even nicer to test a Subversion build with serf-trunk, I didn't get
around to that yet.
And backport to 1.4.x, of course.
And finally a release from 1.4.x ... it's time.
-- Brane
Re: [PATCH] cmake: Enable SPNEGO/SSPI module on Windows
Posted by Timofei Zhakov <ti...@chemodax.net>.
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.sahlberg@gmail.com> wrote:
> >
> >> HI Timofei,
> >>
> >> Den fre 20 dec. 2024 kl 14:21 skrev Timofei Zhakov<ti...@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
Re: [PATCH] cmake: Enable SPNEGO/SSPI module on Windows
Posted by Branko Čibej <br...@apache.org>.
On 8. 1. 25 09:47, Timofei Zhakov wrote:
> On Sun, 22 Dec 2024 at 12:42 AM, Daniel Sahlberg <
> daniel.l.sahlberg@gmail.com> wrote:
>
>> HI Timofei,
>>
>> Den fre 20 dec. 2024 kl 14:21 skrev Timofei Zhakov<ti...@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
Re: [PATCH] cmake: Enable SPNEGO/SSPI module on Windows
Posted by Timofei Zhakov <ti...@chemodax.net>.
On Sun, 22 Dec 2024 at 12:42 AM, Daniel Sahlberg <
daniel.l.sahlberg@gmail.com> wrote:
> HI Timofei,
>
> Den fre 20 dec. 2024 kl 14:21 skrev Timofei Zhakov <ti...@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…
Re: [PATCH] cmake: Enable SPNEGO/SSPI module on Windows
Posted by Daniel Sahlberg <da...@gmail.com>.
HI Timofei,
Den fre 20 dec. 2024 kl 14:21 skrev Timofei Zhakov <ti...@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