Page MenuHomeFreeBSD

net/svnup: Fix libssl linking
ClosedPublic

Authored by brnrd on Jan 9 2016, 8:23 PM.

Details

Summary

Proposed commit log

net/svnup: Fix linking correct libssl

  - Use bsd.openssl.mk defined variable for libs

Reviewed_by:	koobs (mentor), feld (mentor)
Approved by:	(mentor)
Differential_Revision:	D4843
Test Plan

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

brnrd updated this revision to Diff 12081.Jan 9 2016, 8:23 PM
brnrd retitled this revision from to net/svnup: Fix libssl linking.
brnrd updated this object.
brnrd edited the test plan for this revision. (Show Details)
brnrd added reviewers: koobs, feld.
brnrd updated this object.Jan 9 2016, 8:23 PM
brnrd edited the test plan for this revision. (Show Details)Jan 9 2016, 8:32 PM

Maintainer contacted via email.

Hi John,
I've just opened a review for net/svnup. Please find it at https://reviews.freebsd.org/D4843 As there's no "maintainer approval" feature in Phabricator, I'm contacting you via mail.
Basically a trivial change that uses the ports framework to link to the OpenSSL libs required by the user via WITH_OPENSSL_PORT=yes and OPENSSL_PORT= security/{open,libre}ssl
With kind regards,
Bernard.
PS. Kubilay and Mark cc-ed as mentors

koobs added a subscriber: jilles.Jan 26 2016, 9:18 AM
koobs added inline comments.
net/svnup/Makefile
17 ↗(On Diff #12081)

I may be wrong, but I believe LIBS was recommended for these?

cc @jilles

jilles added inline comments.Jan 30 2016, 12:53 PM
net/svnup/Makefile
17 ↗(On Diff #12081)

It looks like both LIBS and LDFLAGS are used for -L and -l in ports. I don't know which one is best.

jgh added a subscriber: jgh.Feb 11 2016, 10:06 PM

I believe what this is trying to address, though, is respecting a library choice between BASE vs. PORT/PKG throughout the build process. The way it is now, I don't believe that is accomplished.

koobs added inline comments.Feb 12 2016, 2:36 AM
net/svnup/Makefile
5 ↗(On Diff #12081)

Bump PORTREVISION, since the port is not broken at the moment, and package will change depending on environment / user configuration settings

17 ↗(On Diff #12081)

@brnrd Can you test / tell us the difference between using LDFLAGS and LIBS for this port please. I know some 'autoconf' based software do/supports things slightly differently

Also, Does -I need be added here to account for the different include paths depending on whether ports/base openssl is used?

brnrd marked 4 inline comments as done.Feb 14 2016, 10:40 AM
brnrd added inline comments.
net/svnup/Makefile
5 ↗(On Diff #12081)

Fixed locally, will appear with next arc update

17 ↗(On Diff #12081)

Learned something again. Libs go into LIBS, Linker flags into LDFLAGS.

I've tried long and hard but it seems it will NOT build unless this is all placed in LDFLAGS.

LIBS=           -lmd -lssl
LDFLAGS=        -L${OPENSSLLIB}
===>  Building for svnup-1.07_2
cd /usr/ports/net/svnup/work/svnup-1.07 &&  cc -o svnup -O2 -fno-strict-aliasing -pipe -march=native  -fstack-protector svnup.c -L/usr/local/lib -Wl,-rpath,/usr/local/lib -fstack-protector
svnup.c:37:10: fatal error: 'openssl/ssl.h' file not found
#include <openssl/ssl.h>

This is a result of a base system with Private SSL libs

CFLAGS=         -I${OPENSSLINC}
LIBS=           -lmd -lssl
LDFLAGS=        -L${OPENSSLLIB}
===>  Building for svnup-1.07_2
cd /usr/ports/net/svnup/work/svnup-1.07 &&  cc -o svnup -I/usr/local/include -fstack-protector -fno-strict-aliasing svnup.c -L/usr/local/lib -Wl,-rpath,/usr/local/lib -fstack-protector
/tmp/svnup-195845.o: In function `main':
svnup.c:(.text+0x4dcc): undefined reference to `SSL_shutdown'
svnup.c:(.text+0x4dde): undefined reference to `SSL_CTX_free'
svnup.c:(.text+0x4dea): undefined reference to `SSL_free'
/tmp/svnup-195845.o: In function `reset_connection':
svnup.c:(.text+0x58d2): undefined reference to `SSL_library_init'
svnup.c:(.text+0x58f6): undefined reference to `SSL_load_error_strings'
svnup.c:(.text+0x58fb): undefined reference to `SSLv23_client_method'
svnup.c:(.text+0x5903): undefined reference to `SSL_CTX_new'
svnup.c:(.text+0x5931): undefined reference to `SSL_CTX_ctrl'
svnup.c:(.text+0x595e): undefined reference to `SSL_CTX_ctrl'
svnup.c:(.text+0x5972): undefined reference to `SSL_new'
svnup.c:(.text+0x59ad): undefined reference to `SSL_set_fd'
svnup.c:(.text+0x59c0): undefined reference to `SSL_connect'
svnup.c:(.text+0x59f0): undefined reference to `SSL_get_error'
/tmp/svnup-195845.o: In function `process_command_http':
svnup.c:(.text+0x64d4): undefined reference to `SSL_read'
/tmp/svnup-195845.o: In function `confirm_md5':
svnup.c:(.text+0x7d61): undefined reference to `MD5Init'
svnup.c:(.text+0x7da2): undefined reference to `MD5Update'
svnup.c:(.text+0x7db5): undefined reference to `MD5End'
svnup.c:(.text+0x7f33): undefined reference to `MD5Init'
svnup.c:(.text+0x7f4e): undefined reference to `MD5Update'
svnup.c:(.text+0x7f61): undefined reference to `MD5End'
/tmp/svnup-195845.o: In function `get_files':
svnup.c:(.text+0x9205): undefined reference to `MD5Init'
svnup.c:(.text+0x922d): undefined reference to `MD5Update'
svnup.c:(.text+0x9243): undefined reference to `MD5End'
/tmp/svnup-195845.o: In function `send_command':
svnup.c:(.text+0xa5fb): undefined reference to `SSL_write'
cc: error: linker command failed with exit code 1 (use -v to see invocation)

This only works like this

CFLAGS=         -I${OPENSSLINC}
LDFLAGS=        -L${OPENSSLLIB} -lmd -lssl
===>  Building for svnup-1.07_2
cd /usr/ports/net/svnup/work/svnup-1.07 &&  cc -o svnup -I/usr/local/include -fstack-protector -fno-strict-aliasing svnup.c -L/usr/local/lib -lmd -lssl -Wl,-rpath,/usr/local/lib -fstack-protector
===>  Staging for svnup-1.07_2 etc

The patch will be updated shortly to honor base/ports OpenSSL

brnrd updated this revision to Diff 13289.Feb 14 2016, 10:41 AM
brnrd marked 2 inline comments as done.

Fix using correct include path (OpenSSL)

feld accepted this revision.Feb 14 2016, 4:26 PM
feld edited edge metadata.
This revision is now accepted and ready to land.Feb 14 2016, 4:26 PM
This revision was automatically updated to reflect the committed changes.