Unify include paths for headers between core, internal plugins, and external plugins.

[SolidWallOfCode]

Updated based on feedback

• include/ts for C API headers.
• include/tscpp for C++ API headers (link with libtscpp.so)
• include/tsutil for C++ utility headers (link with libtsutil.so)

lib/ts is moved to lib/tscore which builds the libtscore library. A new directory, "lib/tsutil", is created for "dual use" utility C++ headers which builds libtsutil. These are headers used in the TS core and intended to be available to plugins. The goal of this restructuring is to unify include paths for core, internal plugins, and external plugins. That in turn enables exporting the dual use utility headers such as TextView.h. With the include paths the same, secondary includes will work as expected.

This only moves the installed version of TextView.h to include/tsutil as a demonstration and to validate the feasibility of making the move. This current structure is the result of many discussions on this PR, the mailing list, and the IRC channel.

UPDATE

All references to "atscppapi" have been changed to "tscpp". This includes the library name and the internal library namespace. The "README` file in this PR has been updated to reflect the re-arrangement.

• ts/... : C API functions.
• tscpp/... : C++ API functions.
• tsutil/... : Internally used utilities not dependent on TS specifically.
• tscore/... : Internal use only libraries.

These paths become the same for core and plugins so that internal plugins have less of a privileged position over external plugins and serve as better examples for plugin developers.

lib/ts was split into libtscore and libtsutil so that the dual use headers were in a separate library than the purely core headers. This was recommended by both @bryancall and @dragon512.

Key issues to handle for exporting dual use C++ headers -

• The include path for a given header (e.g. TextView.h) must be the same between the core and a plugin because of headers including other headers.
• Care must be taken to not impose core based requirements on plugins (e.g., not using assert at all).
• I would like to minimize the number of -I include options on the compiler line. Relatedly, having include paths that have semantic meaning would help ease the pain of them being longer.
• Avoid having "magic" or other "don't really include this" headers. Dual use headers need to not drag in other entangled headers. Let's not be Boost.

[zwoop]

Also, bigger question: How does this fit in with the existing (public) C++ APIs? It feels like we ought to have one set of includes / directory for all C++ level include files.

[zwoop]

I don' like the top-level directory name "src" here. To me, when I see that, it implies that all source code goes in src/. Seeing that all under src/ is API, maybe shuffle things up?

[d2r]

Same here: When I see /src I initially expect non-header source files there (implementations). I see this proposes subtrees under /src expressly for different categories of header files.

[SolidWallOfCode]

I'm trying to look forward to a future where the source files are moved under this location. E.g. "It provides scope for, over the lifetime of 8.0, restructuring more of the source code in to less insane places". When we don't do that, we get stuck. I was hoping to plan ahead for once and not forbid anything that's not immediately useful.

[zwoop]

I think we should continue the pattern of using includes for new Makefile (and keep working on fixing the existing ones). As an example, see how it's done in the plugins/ tree. This still preserves the Makefile modularity (one Makefile per "level"), but allows for much better parallelism.

[SolidWallOfCode]

I disagree about having all of the C++ headers in a single directory. Bryan and I touched on this in our IRC discussion and I've become more convinced that splitting is the correctly approach. Among many other reasons, one issue is "I've included header leif.h. Do I need to link to a library too?". In the proposed scheme, if the header is in src/api/core, you don't. If it is in src/api/tscpp you do and the library is libatscppapi.so.

[bryancall]

This doesn't fall with the standard way of laying out source code. Normally external headers go into an include directory at the top level (e.g. include/ts & include/tscpp). Also, I agree with @zwoop about having all the C++ headers in one directory. Users shouldn't have to figure out what directory the header is in and what lib to link with. There should only be one include directory and one library.

[dragon512]

The standard way to layout headers is to have you stuff under a directory so you have to say

#include "dir/header"
This proposal means we have a clear

#include "api/tscpp/header.h"
#include "api/ts/ts.h"
#include "ts/ts.h"

and all you have in our build for include path is ./src

the dev package/ install move headers under a include directory with the same source layout

[bryancall]

What I would propose would be:

lib/cppapi/include/atscppap moves to include/tscpp and would be installed at /usr/local/include/tscpp
proxy/api/ts moves to include/ts and would be installed at /usr/local/include/ts

Rename lib/ts to lib/ats so it doesn't conflict with the external api directory.

There would be one library for cpp APIs (e.g. TextView & Url) would change to:
/usr/local/lib/libtscpp.so

And this is how you would include headers:

#include "ats/ink_time.h" - internal C
#include "ats/Regex.h" - internal C++
#include "ts/ts.h" - exetrnal C API
#include "tscpp/TextView.h" - external C++ (also used internally)
#include "tscpp/Url.h" - external C++ API

[dragon512]

So the current release (which @bryancall makes a great point on as we are about to release) I am good with @bryancall proposal. I still believe that it will be better long term to move code under src/

[bryancall]

@dragon512 I agree long term moving most of the code under src/ is a good idea.

It makes me nervous when I wanted to branch last week and people are wanting to get stuff in last minute. We need to have a better plan and discuss it appropriately. Either we do a big push on moving things around before 9.0.0 or do it gradually after 8.0.0 is branched.

I would be in favor or one big push on moving things around right before 9.0.0 to make it easier on LTS maintainers.

[SolidWallOfCode]

The goal of the way this PR was set up was precisely to provide for gradual movement during the 8.0 lifetime. The net effect of this PR is to create some directories and move 3-4 header files. That is as minimally disruptive as possible, IMHO, to achieve the stated goal.

"There would be one library for cpp APIs (e.g. TextView & Url)"

That is not possible without some major restructuring. At minimum almost everything would have to link with libtsutil and libtscpp instead of just the former. We would also be linking in the pure CPPAPI code which is currently not anywhere in the core. Even if were willing to do that, then it be much better to not have that code talk back through the C API but directly interact with the core. If this is a real problem then we should put the dual use headers in the C API directory because the linkage for them for a plugin is identical to that of the C API.

I am fine with using a top level include as suggested.

[SolidWallOfCode]

I think we should change lib/ts to lib/tsutil so that libtsutil.so is compiled in lib/tsutil. Then we can just directly export headers from there to install/include/tsutil in parallel with include/ts and include/tscpp. In addition to distinguishing based on linkage, this would also separate pure utility classes in tsutil from API related classes in tscpp and ts.

[SolidWallOfCode]

I updated the main comment. As noted I had two people push for putting the dual use headers in a separate library. I named that "libtsutil" because it's the utility library. The former "libtsutil" was moved to "libtscore" and put in "lib/core". Plugins don't need to link with "libtsutil" as that is pulled in by the traffic_server binary but it can be used to do linkage verification during build rather than after installation.

[bryancall]

I still think it is a bad idea to split up the C++ API into to directories. I am -0 on this.

lost interest

[zwoop]

Ship it.

[bryancall]

@SolidWallOfCode This doesn't cleanly cherry pick to 8.0.x. If you want it in 8.0.0 you will need to make a PR off the 8.0.x branch.