Skip to content
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.

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? Sign in to your account

Compile against openssl 3 #18

Merged
merged 7 commits into from
May 5, 2023
Merged

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented May 3, 2023

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@traversaro
Copy link
Contributor Author

@conda-forge-admin, please rerender

@traversaro
Copy link
Contributor Author

It is not using the correct openssl:

-- Found OpenSSL: optimized;D:/bld/libpcap_1683109473806/_h_env/Library/lib/libcrypto.lib;debug;C:/Program Files/OpenSSL/lib/VC/libcrypto64MDd.lib (found version "1.1.1t")  

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@traversaro
Copy link
Contributor Author

It is not using the correct openssl:

-- Found OpenSSL: optimized;D:/bld/libpcap_1683109473806/_h_env/Library/lib/libcrypto.lib;debug;C:/Program Files/OpenSSL/lib/VC/libcrypto64MDd.lib (found version "1.1.1t")  

No, actually it is the correct one for the optimized build, it is founding something else for the debug library, but that should not be a problem as we are doing a Release build, that is using optimized libraries.

@traversaro
Copy link
Contributor Author

traversaro commented May 3, 2023

@conda-forge/libpcap @nehaljwani The PR is ready for review, thanks! Actually not, for some reason the PR did not re-render.

@traversaro traversaro changed the title Try to compile against openssl 3 Compile against openssl 3 May 3, 2023
@traversaro
Copy link
Contributor Author

@conda-forge-admin, please rerender

@traversaro
Copy link
Contributor Author

The error is:

[21/78] Building C object CMakeFiles\pcap.dir\pcap-rpcap.c.obj
FAILED: CMakeFiles/pcap.dir/pcap-rpcap.c.obj 
C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\cl.exe  /nologo -DBUILDING_PCAP -DHAVE_CONFIG_H -DYY_NO_UNISTD_H -D_CRT_SECURE_NO_WARNINGS -D__STDC__ -Dpcap_EXPORTS -I%SRC_DIR%\build -I%SRC_DIR% -external:I%PREFIX%\Library\include -external:W0 /DWIN32 /D_WINDOWS /W3 /MT /O2 /Ob2 /DNDEBUG  /utf-8 -wd4646 /showIncludes /FoCMakeFiles\pcap.dir\pcap-rpcap.c.obj /FdCMakeFiles\pcap.dir\ /FS -c %SRC_DIR%\pcap-rpcap.c
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2146: syntax error: missing ')' before identifier 'offset'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2081: 'off_t': name in formal parameter list illegal
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2061: syntax error: identifier 'offset'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ';'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ','
%PREFIX%\Library\include\openssl/ssl.h(1974): error C2059: syntax error: ')'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ';'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ','
%PREFIX%\Library\include\openssl/ssl.h(1974): error C2059: syntax error: ')'
ninja: build stopped: subcommand failed.

That is familiar as I had already discussed it in #14 .

@traversaro
Copy link
Contributor Author

The error is:

[21/78] Building C object CMakeFiles\pcap.dir\pcap-rpcap.c.obj
FAILED: CMakeFiles/pcap.dir/pcap-rpcap.c.obj 
C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\cl.exe  /nologo -DBUILDING_PCAP -DHAVE_CONFIG_H -DYY_NO_UNISTD_H -D_CRT_SECURE_NO_WARNINGS -D__STDC__ -Dpcap_EXPORTS -I%SRC_DIR%\build -I%SRC_DIR% -external:I%PREFIX%\Library\include -external:W0 /DWIN32 /D_WINDOWS /W3 /MT /O2 /Ob2 /DNDEBUG  /utf-8 -wd4646 /showIncludes /FoCMakeFiles\pcap.dir\pcap-rpcap.c.obj /FdCMakeFiles\pcap.dir\ /FS -c %SRC_DIR%\pcap-rpcap.c
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2146: syntax error: missing ')' before identifier 'offset'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2081: 'off_t': name in formal parameter list illegal
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2061: syntax error: identifier 'offset'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ';'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ','
%PREFIX%\Library\include\openssl/ssl.h(1974): error C2059: syntax error: ')'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ';'
%PREFIX%\Library\include\openssl/ssl.h(1973): error C2059: syntax error: ','
%PREFIX%\Library\include\openssl/ssl.h(1974): error C2059: syntax error: ')'
ninja: build stopped: subcommand failed.

That is familiar as I had already discussed it in #14 .

Indeed, the issue was the one described in openssl/openssl#18310 . While the proper solution is for openssl to avoid exposing in headers functions that are not on Windows, the problem that we had specifically in pcap is that off_t is not defined by Windows UCRT headers under certain conditions, that apparently are verified in the pcap build. Anyhow, to define off_t it is sufficient to define the _CRT_DECLARE_NONSTDC_NAMES macro, see https://learn.microsoft.com/en-us/cpp/c-runtime-library/compatibility?view=msvc-170 . So, in 331f347 we just added a macro that did exactly that, and now the build is green. @conda-forge/libpcap @nehaljwani the PR is ready for review, for real this time.

@nehaljwani
Copy link
Member

Could you please update the build matrix to build for both versions of openssl?

@traversaro
Copy link
Contributor Author

Could you please update the build matrix to build for both versions of openssl?

Do you have any specific motivation for this? conda-forge stopped supporting openssl 1.* once it completed the openssl3 migration (see conda-forge/conda-forge-pinning-feedstock#3892). Continuing to compile again openssl 1.* can work for some time, but at some point it can break once there is some ABI migration or similar that will not occur for openssl 1.* .

@nehaljwani
Copy link
Member

Ah, I didn't know that. Thanks, merging.

@nehaljwani nehaljwani merged commit 4088728 into conda-forge:main May 5, 2023
@traversaro
Copy link
Contributor Author

Ah, I didn't know that. Thanks, merging.

Perfect, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants