Skip to content

Conversation

@hdholm
Copy link
Contributor

@hdholm hdholm commented Jul 4, 2021

Relatively small changes to check for host_machine vs. build_machine as the target to account for possible cross compilation. Additionally removes some explicit dependencies on OpenSSL for FreeBSD which required OpenSSL to be installed via pkg or ports rather than relying on the "built-in" OpenSSL version shipped by default. Consequently discovered that the issue-75 test depends on version 1.1.0 or higher of OpenSSL being installed on the build machine for OPENSSL_init_ssl, .so made that explicit in the dependency.

hdholm and others added 7 commits July 3, 2021 17:41
Change build_machine to host_machine for freebsd specific configuration.
Allow building the test on a FreeBSD build host even if no "OpenSSL" since the base OpenSSL will suffice.
OPENSSL_init_ssl only exists since version 1.1.0.
Dependency on libcrypto neither necessary nor desirable if using the builtin openssl (it world require the ports version to be installed instead.)
@sarroutbi
Copy link
Collaborator

sarroutbi commented Oct 6, 2021

Hello @hdholm. Thanks for PR. Changes LGTM. Please, could you rebase on top of the current tree to fix the conflicts? Thanks in advance

@sarroutbi sarroutbi self-requested a review October 6, 2021 12:45
Signed-off-by: Sergio Arroutbi Braojos <sarroutb@redhat.com>
hdholm and others added 2 commits November 26, 2021 23:48
Depend on host_machine rather than build_machine to account for potential cross compilation.
Look for the openSSL libs in /usr/local first.
Copy link

@ngie-eign ngie-eign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some comments -- thank you for pushing this :).

Whatever in meson/FreeBSD was unhappy about the basic library dependency for libcrypto seems to have been fixed because it's working for me in all tests I've done in it's basic form. Also removing the version check since nothing should be using the wildly out of date openssl.
@hdholm
Copy link
Contributor Author

hdholm commented May 8, 2023

The CI checks are stalled on this, which I believe is fixed by PR #129

Copy link

@ngie-eign ngie-eign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 💚!!

@sarroutbi sarroutbi requested a review from sergio-correia May 17, 2023 17:49
…private change is the right

answer.  I did not make changes to the depends method(s) since that seemed to make the Debian
based builds very unhappy (couldn't find libcrypto) and everything else seems to generate reasonable
pkg_config files without invoking method.
@sarroutbi
Copy link
Collaborator

Hello @hdholm. Thanks for the PR. A new release of jose is being planned and we would like this change to be merged ... would you mind fixing the conflicts? Or do you prefer me to do it? I am OK with both.

@sarroutbi
Copy link
Collaborator

As conflict is easily fixable, I will sort it out myself. Thanks for your PR @hdholm.

Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
@sarroutbi sarroutbi merged commit 5a81837 into latchset:master Feb 2, 2024
@hdholm
Copy link
Contributor Author

hdholm commented Feb 2, 2024

Thanks for taking care of it. It's been a very busy week. Just catching up on email now.

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.

5 participants