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

Do not include headers bearing OpenSSL copyrights by default #6658

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

georgthegreat
Copy link
Contributor

We use automatic license analysis and automatic source tracking when we import the code.

At the time license report for libgit2 is overcomplicated.
This PR reorganizes include schema in order to avoid #include-ing openssl_dynamic.h from default build.

It would be nice to add an option disabling openssl_dynamic.c and openssl_legacy.c compilation at all.

This is yet TBD.

@ethomson
Copy link
Member

It would be nice to add an option disabling openssl_dynamic.c and openssl_legacy.c compilation at all.

I think that this is the only way that this will work, since GIT_OPENSSL_LEGACY is enabled based on the version of OpenSSL that's in use:

# if (defined(OPENSSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER < 0x10100000L) || \
     (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x20700000L)
#  define GIT_OPENSSL_LEGACY
# endif
#endif

@georgthegreat
Copy link
Contributor Author

The above code would not work for us, it should be a CMake option, not compile-time CFLAG.

@georgthegreat
Copy link
Contributor Author

Anyway, this PR does not break anything (I hope so) and solves our problem.

@ethomson
Copy link
Member

At the moment, none of the OpenSSL builds in CI are working with this change. 😞

@georgthegreat
Copy link
Contributor Author

georgthegreat commented Nov 20, 2023

Hmm... I am unable to reproduce this locally.

$ contrib/libgit2@openssl-license$ cmake -DENABLE_WERROR=ON -DBUILD_EXAMPLES=ON -DBUILD_FUZZERS=ON -DUSE_STANDALONE_FUZZERS=ON -G "Ninja" -DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DDEBUG_STRICT_ALLOC=ON -DDEBUG_STRICT_OPEN=ON
$ ninja

I get a binary dynamically linked to openssl:

$ ldd git2
        linux-vdso.so.1 (0x00007ffd3b3fe000)
        libssl.so.1.1 => /usr/lib/x86_64-linux-gnu/libssl.so.1.1 (0x00007ff2a3587000)
        libcrypto.so.1.1 => /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1 (0x00007ff2a32b1000)
        libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007ff2a3295000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007ff2a3272000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007ff2a3080000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007ff2a3078000)
        /lib64/ld-linux-x86-64.so.2 (0x00007ff2a37f2000)

Do you have any idea on reproducing this?

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.

None yet

2 participants