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

Build libraries with a suffix in the name that specifies the TLS library built against #151

Merged
merged 11 commits into from
Apr 13, 2018

Conversation

mrdeep1
Copy link
Collaborator

@mrdeep1 mrdeep1 commented Mar 8, 2018

LIBCOAP_API_VERSION is bumped to 2

Depending on the TLS library selection by the ./configure options, the libraries will be named as

libcoap-2-openssl.a
libcoap-2-openssl.so*
libcoap-2-gnutls.a
libcoap-2-gnutls.so*
libcoap-2-tinydtls.a
libcoap-2.a
libcoap-2.so*

Removal of old libcoap-1* files
New libcoap-2.pc.in file
libcoap-2.{map,sym} files are no longer part of git and get built automatically whenever ./configure is run.

Correction of some typos

Options such as '--without-openssl' now work as expected.

New file BUILDING to give an overview as to how to build libcoap

Copy link
Owner

@obgm obgm left a comment

Choose a reason for hiding this comment

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

Very nice! I have found minor nits, and there is the open discussion about the map files we need to resolve first. Otherwise, I am fine with these changes.

BUILDING Outdated
# With TinyDTLS
./configure --enable-tests --disable-documentation --enable-examples --with-tinydtls --disable-shared

# With OpeSSL
Copy link
Owner

Choose a reason for hiding this comment

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

Typo: "OpenSSL"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. Also added in

If you need to rebuild the libcoap-*.{map,sym} files to update any exposed
function changes, run

make update-map-file

prior to running 'make'.

autogen.sh Outdated
include/coap/coap.h
install-sh
libcoap-1.pc libtool ltmain.sh
libcoap-2.pc libtool ltmain.sh
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe extract this information from LIBCOAP_API_VERSION in configure.ac?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As it is only for deleting files with 'autogen.sh --clean' I have changed it to
+libcoap-*.pc libtool ltmain.sh

Copy link
Owner

Choose a reason for hiding this comment

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

Good, thanks!

configure.ac Outdated
@@ -583,6 +603,7 @@ libcoap configuration summary:
libcoap package version : "$PACKAGE_VERSION"
libcoap library version : "$LIBCOAP_SO_VERSION"
libcoap API version : "$LIBCOAP_API_VERSION"
libcoap DTLS lib ext'n : "$LIBCOAP_DTLS_LIB_EXTENSION_NAME"
Copy link
Owner

Choose a reason for hiding this comment

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

The single ' may break automation scripts. I am not sure if such things exist but maybe we can just be safe by avoiding it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

' is no more

Copy link
Owner

Choose a reason for hiding this comment

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

Good, thanks!

configure.ac Outdated
#
# Bring .map / .sym files up to date
#
make update-map-file > /dev/null 2>&1
Copy link
Owner

Choose a reason for hiding this comment

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

This requires exuberant-ctags which is not always present. I am therefore in favor of keeping the default map files as they should reflect the symbols contained in the stable API and thus do not need to change very often and only by the maintainers.

Please refer to the mailing list for ongoing discussion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

.PHONY does not cause them to get automatically updated / built, it just means that whenever 'make update-map-file' is called, it will always get run, even if there is a file called update-map-file.

When a built library is distributed, the distribution does not need to include the .map/.sym files - they are only used when creating the the library files.

As you are in favor of keeping / maintaining the .map/.sym files, I will revert out that change and re-install the distclean warning.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, correct.

@mrdeep1
Copy link
Collaborator Author

mrdeep1 commented Mar 9, 2018

Changes made, as well as adding in stripping out the coap_dtls_, coap_tls_ and coap_socket_* that do not need to be publicly exposed.

@mrdeep1
Copy link
Collaborator Author

mrdeep1 commented Apr 4, 2018

Rebased against develop

.gitignore Outdated
@@ -34,6 +34,11 @@ src/**/.dirstamp
src/**/.libs/
src/**/*.o
src/**/*.lo
src/*.deps/
src/*.dirstamp
src/*.libs/
Copy link
Owner

Choose a reason for hiding this comment

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

These files are already covered by the preceding patterns.

@mrdeep1
Copy link
Collaborator Author

mrdeep1 commented Apr 11, 2018

Rebased off develop and .gitignore now (I believe) correctly updated.

@obgm
Copy link
Owner

obgm commented Apr 12, 2018

So far, this PR looks good to me. @tijuca, are you ok with the proposed naming of the .pc files (such as libcoap-2-openssl.pc)? This would probably need some guesswork by applications that depend on libcoap.

@tijuca
Copy link
Contributor

tijuca commented Apr 12, 2018

I've done some quick test builds, looks also good to me.
We might need to change the installation folder of the public headers to /u/l/coap2 so the current tree can be co-installed on the system. Can be done later to not keep the pull requests open longer as needed. This PR is already a big change.

Replace LIBCOAP_API_VERSON with LIBCOAP_NAME_SUFFIX when refering to building
the library files.  configure.ac defines LIBCOAP_NAME_SUFFIX.

libcoap-$LIBCOAP_API_VERSION.{sym,map} remain.

Make distclean clean up temporary src/*.o and src/*.lo files which may be
left lying around from a previous './configure xxx ; make' that was using
a different TLS library.

Add in coap_dtls_*, coap_tls_*, coap_socket_* functions that do not need to
be publically exposed in the libraries
Update the new LIBCOAP_API_VERSION of the files
Include examples/Makefile and tests/Makefile that are built by autogen.sh
Make LIBCOAP_API_VERSION=2

Set LIBCOAP_NAME_SUFFIX to be $LIBCOAP_API_VERSION{-openssl,-gnutls,-tinydtls}
as appropriate based on the configure build options.

This means that library files will be named
libcoap-2-openssl.a
libcoap-2-openssl.so*
libcoap-2-gnutls.a
libcoap-2-gnutls.so*
libcoap-2-tinydtls.a
libcoap-2.a
libcoap-2.so*

Fix some typos

Use $withval instead of yes so that, say, --without-openssl works as expected.
This is done so we do not need to expose coap_dtls_startup()
publically
@mrdeep1
Copy link
Collaborator Author

mrdeep1 commented Apr 13, 2018

Updated BUILDING following raising of issue #164

@obgm
Copy link
Owner

obgm commented Apr 13, 2018

Okay, so be it.

@obgm obgm merged commit 576aeac into obgm:develop Apr 13, 2018
@mrdeep1 mrdeep1 deleted the builds branch April 13, 2018 13:11
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

3 participants