8000 cmake: fix openssl build on win32 by lazka · Pull Request #6626 · libgit2/libgit2 · GitHub
[go: up one dir, main page]
Skip to content

cmake: fix openssl build on win32 #6626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? 8000 Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 19, 2023
Merged

cmake: fix openssl build on win32 #6626

merged 1 commit into from
Aug 19, 2023

Conversation

lazka
Copy link
Contributor
@lazka lazka commented Aug 19, 2023

since f15c8ac libgit unconditionally depends on secur32 on Windows but only added it in cmake for the winhttp and schannel variants. In case libgit is built against openssl it would fail to link.

This moves secur32 out of the https backend selection code into the global win32 condition (and while at it also adds ws2_32 to the .pc file)

@lazka
cmake: fix openssl build on win32
8687283 
since f15c8ac libgit unconditionally depends on secur32 on Windows
but only added it in cmake for the winhttp and schannel variants.
In case libgit is built against openssl it would fail to link.

This moves secur32 out of the https backend selection code into
the global win32 condition (and while at it also adds ws2_32 to the .pc file)
@lazka
Copy link
Contributor Author
lazka commented Aug 19, 2023 •
edited
Loading

I'm not sure if auth_sspi is supposed to be used/built with openssl in the first place. If not then this probably isn't the correct fix.

This is the build error btw:

[2/2] Linking C shared library libgit2-1.7.dll
FAILED: libgit2-1.7.dll libgit2.dll.a
cmd.exe /C "cd . && C:\msys64\ucrt64\bin\gcc.exe -D_GNU_SOURCE -march=nocona -msahf -mtune=generic -O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wall -Wextra -Wno-documentation-deprecated-sync -Wno-missing-field-initializers -Wmissing-declarations -Wstrict-aliasing -Wstrict-prototypes -Wdeclaration-after-statement -Wshift-count-overflow -Wunused-const-variable -Wunused-function -Wint-conversion -Wc99-c11-compat -Wno-format -Wno-format-security -O3 -DNDEBUG  -pipe -shared -o libgit2-1.7.dll -Wl,--out-implib,libgit2.dll.a -Wl,--major-image-version,1,--minor-image-version,7 @CMakeFiles\libgit2package.rsp  && cd ."
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x91): undefined reference to `__imp_FreeCredentialsHandle'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0xbd): undefined reference to `__imp_DeleteSecurityContext'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0xf3): undefined reference to `FreeContextBuffer'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x341): undefined reference to `__imp_InitializeSecurityContextW'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x539): undefined reference to `__imp_AcquireCredentialsHandleW'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x5e6): undefined reference to `FreeContextBuffer'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x60f): undefined reference to `CompleteAuthToken'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x784): undefined reference to `__imp_QuerySecurityPackageInfoW'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x83d): undefined reference to `FreeContextBuffer'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x904): undefined reference to `__imp_QuerySecurityPackageInfoW'
C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src/libgit2/CMakeFiles/libgit2.dir/transports/auth_sspi.c.obj:auth_sspi.c:(.text+0x9bd): undefined reference to `FreeContextBuffer'
collect2.exe: error: ld returned 1 exit status

@ethomson
Copy link
Member

Thanks for the fix! OpenSSL on Windows is a bit of an uncommon setup, so we don't do enough testing on it. Something we should add to our CI matrix.

Cheers!

@ethomson ethomson added the v1.7 label Aug 19, 2023
@ethomson ethomson merged commit 80efb3a into libgit2:main Aug 19, 2023
@lazka
Copy link
Contributor Author
lazka commented Aug 19, 2023 •
edited
Loading

Thanks!

for context, we package all variants in MSYS2: https://packages.msys2.org/base/mingw-w64-libgit2 So there is no particular reason we need openssl, just that we want to provide all variants to our users.

@ethomson ethomson added the build label Dec 14, 2023
@branchvincent branchvincent mentioned this pull request Mar 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lazka @ethomson
0