tpm2-software / tpm2-tss

OSS implementation of the TCG TPM2 Software Stack (TSS2)
https://tpm2-software.github.io
BSD 2-Clause "Simplified" License
730 stars 359 forks source link

#include <tpm20.h> VS #include <sapi/tpm20.h> #545

Closed liuqun closed 6 years ago

liuqun commented 6 years ago

There were some 3rd party projects that are still using #include <tpm20.h> in their stable code against tpm2-tss-1.2.x, such as strongswan:

The changes on sapi.pc from PR https://github.com/01org/tpm2-tss/pull/499 would obviously break their code, perhaps we'd better place another fake "tpm20.h" at the top level of @includedir@


TCTI header include path:

tcti-socket.pc and tcti-device.pc are pointing to @includedir@ after changes from PR https://github.com/01org/tpm2-tss/pull/499

tcti-tabrmd.pc is pointing to subdir @includedir@/tcti, see:

flihp commented 6 years ago

I didn't realize there was a disagreement between the PC files in the TSS libraries and the tabrmd. I'll fix up the tabrmd TCTI PC file. Thanks for the pointer.

The feedback from people packaging for distros is that they don't like having files dropped directly into $includedir. Is there any down side to putting both $(includedir) and $(includedir)/sapi in CFLAGS via the pkgconfig file?

liuqun commented 6 years ago

Currently, I can't figure out any down side to putting both $(includedir) and $(includedir)/sapi in CFLAGS.

Thanks, filip!

flihp commented 6 years ago

I'm reopening this to remind me to get a PR together for this change.

flihp commented 6 years ago

I was looking back through the history for the pkg-config metadata and I noticed that a patch from @liuqun is where the -I@includedir@/sapi was removed: https://github.com/01org/tpm2-tss/commit/ca8b438d3de90719658c6fe1ac03b61be10abbd5#diff-2313da1a3ad0d1087173b64ff3febb6c

In the commit message you note that pointing to the sub directories are wrong and everything I can find on the internet seems to agree with you. Since the next release we do from master will be a major version bump we'll be breaking backward compatibility anyways so if we want to do this "right" then we shouldn't be adding these subdirectories back. Can you provide some clarification @liuqun?

liuqun commented 6 years ago

Many C/C++ tool kit such as libxml put their headers into a sub-directory under /usr/include and adding a release version as part of the directory name, for example:

$ pkg-config --cflags libxml-2.0
-I/usr/include/libxml2

$ tree /usr/include/libxml2/
/usr/include/libxml2/
└── libxml
    ├── c14n.h
    ├── catalog.h
    ├── chvalid.h
    ├── debugXML.h
    ├── dict.h
    ├── DOCBparser.h
    ├── encoding.h
    ├── entities.h
    ├── globals.h
    ├── hash.h
    ├── HTMLparser.h
    ├── HTMLtree.h
    ├── list.h
    ├── nanoftp.h
    ├── nanohttp.h
    ├── parser.h

Then application developers doesn't have to adjust their existing code, which means #include<libxml/parser.h> also works for libxml-2.0. It would be possible to install two different major release (libxml and libxml2) into one system since they were using different directory names. Though this should be not recommended to.

libpng and GTK:

$ pkg-config --cflags libpng
-I/usr/include/libpng16
$ ls /usr/include/libpng16/
pngconf.h  png.h  pnglibconf.h
$ ls -l /usr/include/libpng
lrwxrwxrwx 1 root root 8 Jan 10  2017 /usr/include/libpng -> libpng16

$ pkg-config --cflags glib-2.0 
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
$ ls -p /usr/include/glib-2.0/
gio/  glib/  glib.h  glib-object.h  glib-unix.h  gmodule.h  gobject/

Some other tool kit such as libjpeg8 and libjpeg9 didn't create any sub-directory, and just put their header files directly under /usr/include. Sometimes, when 3rd-party developers need a special version of libjpeg, I guess their should build a standalone libjpeg inside their own source tree, and carefully avoiding using wrong header files carried along with the target OS.

liuqun commented 6 years ago

Since PR #862 tpm20.h is now completely removed, I think this issue should be closed anyway.

AndreasFuchsTPM commented 6 years ago

Agreed