Bug 225082 - [tcf][releng] Improve TCF Agent build directory structure
Summary: [tcf][releng] Improve TCF Agent build directory structure
Status: NEW
Alias: None
Product: TCF
Classification: Tools
Component: Core (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks: 244451 257261
  Show dependency tree
 
Reported: 2008-04-01 09:07 EDT by Martin Oberhuber CLA
Modified: 2011-04-28 00:14 EDT (History)
4 users (show)

See Also:


Attachments
Proposal patch for TCF plugins system. (7.34 KB, patch)
2009-07-29 16:14 EDT, Philippe Proulx CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2008-04-01 09:07:38 EDT
The TCF Agent Makefile currently uses a flat folder for all its contents. This has several drawbacks:

* Browsing the code is not as easy as it could be, and it's harder for 
  newbies to find the stuff they are looking for. We should separate the
  headers from the implementation, and probably separate the Core from
  the Services/Extensions/Main Programs.

* Build Output should go to different folders depending on target architecture,
  such that multiple architectures or build/release variants can be built from
  the same sources.

One option for achieving both goals would be to switch to using CDT Managed Build; but since the Agent is to be used in very constrained runtime environments (VxWorks or other RTOS), I'd prefer keeping the simple single Makefile and rewrite it to support
  - Output Directories WinDebug/WinRelease/CygwinDebug/CygwinRelease/
    CygwinDebug/CygwinRelease/LinuxDebug/LinuxRelease/...
  - Separate Headers from implementation ("src", "include" folders)

I'm not sure if any further separation into Core/Services/Mains makes sense so I'd like to defer this to a second step for now.

Any comments or feedback?
Comment 1 Martin Oberhuber CLA 2008-04-01 09:17:19 EDT
Note that daytime build system also needs to adapt to any restructuring.
Comment 2 Martin Oberhuber CLA 2008-04-03 12:36:02 EDT
...And service-adding agents should be able to link against precompiled agent lib rather than having to build all the agent again themselves.
Comment 3 Felix Burton CLA 2008-04-16 02:59:11 EDT
Proposal for new agent directory structure:
SVNROOT
+ projects
| + common
| | + framework
| | | +	protocol.c
| | | + protocol.h
| | | + channel.c
| | | + channel.h
| | | + …
| | + services
| | | + runctrl.c
| | | + runctrl.h
| | | + memoryservice.c
| | | + memoryservice.h
| | | + …
| | + mdep
| | | + arch.c
| | | + arch.h
| | | + arch_x86.c
| | | + arch_x86.h
| | | + arch_ppc.c
| | | + arch_ppc.h
| | | + mdep.c
| | | + mdep.h
| | | + mdep_thread.c
| | | + mdep_thread.h
| | | + mdep_socket.c
| | | + mdep_socket.h
| + agent
| | + Makefile
| | + config.h
| | + main.c
| + value-add
| | + Makefile
| | + config.h
| | + main.c
| + tcf-log
| | + Makefile
| | + config.h
| | + …

Notes:
- The Makefile in each project directory (other than common) would create a sub directory containing the object and/or archive files and generated executable.

- Specific functionality in mdep.[ch] will be separated since most files should not depend on it.  For example threading and socket handling should be kept localized as much as possible.

- The mdep directory contains both processor architecture specific as well as runtime environment (machine) specific information.  This could be split up, but since the directory will be relative small it may not be necessary.

Comments are welcome!
Comment 4 Michael Scharf CLA 2008-04-21 22:52:06 EDT
Looks much better than the flat structure now. But I'd suggest to create a directory per service. Or do  the services depend on each other? And why are the services under common?


Comment 5 Felix Burton CLA 2008-04-22 02:37:47 EDT
(In reply to comment #4)
> Looks much better than the flat structure now. But I'd suggest to create a
> directory per service. Or do the services depend on each other? 

Service interfaces are designed to have minimal dependencies, but the underlying implementation may have dependencies.

> And why are the services under common?

It is under common because we wanted to reduce the number of top level directories because each would map to an eclipse project.
Comment 6 Martin Oberhuber CLA 2008-04-22 03:33:48 EDT
(In reply to comment #5)
Given that each service is typically just one header and one c file, I'm ok with keeping the proposed structure rather than fostering an explosion of directories.

I do not think that top level directories necessarily map to Eclipse projects. We can still have all the TCF plain C implementation in one Eclipse project just as we do now, and actually I think I'd prefer that over having separate Eclipse projects just for the value-add or for the agent, with just 4-5 files in them.

Let's keep the administrative overhead as small as possible. Having services under "common" seems OK for me considering that the common stuff is re-used by the various main programs (tcflog, agent, value-add) and thus makes up kind of a tcf "library".
Comment 7 Renan Le Padellec CLA 2008-08-28 05:39:05 EDT
With the current proposal, most OS dependent stuff is localized to mdep.[ch]. Which means that if someone wants to port the agent to a proprietary OS, he has to add proprietary OS dependent code into those files. By doing this, he will expose some of his private APIs/data structures, which is not acceptable.

To resolve this, I propose to split those files into OS dependent files. For example: mdep_win32.[ch], mdep_unix.[ch], ...

Comment 8 Michael Scharf CLA 2008-11-13 10:32:49 EST
I would also like to see some uniform naming convention. 	t would be great if the header files would have some structure when included.
Instead of

#include "link.h"

I'd like to see a tcf prefix:

either

#include "tcf_link.h"

or

#include "tcf/link.h"

or

#include "tcf/framework/link.h"

The flat include can easily lead to unwanted name clashes, because names like 
  link.h
  channel.h
  events.h
  myalloc.h
  ...
Have a high chance to clash with other includes.
Comment 9 Michael Scharf CLA 2008-12-02 15:30:39 EST
...I think it is time to fix this bug (see bug 257261)
Comment 10 Martin Oberhuber CLA 2008-12-04 13:37:13 EST
There was an idea to wrap the TCF agent as one or more RTSC components. RTSC could actually help managing the services installed into the agent, as well as the target build platforms -- plus, it could probably help offloading some tracing/logging/debugging code from the agent into the host, to make the agent even smaller.
Comment 12 Philippe Proulx CLA 2009-07-29 16:14:07 EDT
Created attachment 142943 [details]
Proposal patch for TCF plugins system.

Here is the patch for a basic plugins system support. In the modified Makefile, the plugins system is disabled by default. To enable it, you have to specifity a plugins path, where the compiled agent is going to search for valid plugins. The plugins path may be specified to `make' like this:

make PATH_Plugins="/absolute/path/to/the/plugins"

TCF plugins have to be compiled/linked as shared libraries (see http://www.ibiblio.org/pub/Linux/docs/HOWTO/other-formats/html_single/Program-Library-HOWTO.html#AEN95 ). For example, a plugin `myplugin.so' made of `myplugin.c' and `tools.c' should be built this way:

$ gcc -fPIC -c -Wall -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_GNU_SOURCE -Wmissing-prototypes -Wno-parentheses -I path/to/tcf-agent/ myplugin.c tools.c
$ gcc -shared -Wl,-soname,myplugin.so -o myplugin.so myplugin.o tools.o -lc

When the agent loads, the plugins system will call, for each .so found, the function "tcf_init_plugin". Thus, the only function that's going to be called by the system in your plugin should have this prototype:

void tcf_init_plugin(Protocol *, TCFBroadcastGroup *, TCFSuspendGroup *);

The plugin's function "tcf_init_plugin" has the same purpose as the services initialization functions.

This patch has been applied and tested on SVN revision 760 of the C TCF agent under Linux. The plugins system is only implemented on UNIX-based systems for now (because it uses libdl functions), but could possibly be extended to other systems using other mechanisms.

What do you think about it? Comments, suggestions?

Legal Message: I, Philippe Proulx, declare that I developed
attached code from scratch, without referencing any 3rd party materials
except material licensed under the EPL and EDL. I am authorized by my
employer, École Polytechnique de Montréal, to make this contribution
under the EPL and EDL.

The École Polytechnique de Montréal is a member of the Eclipse Associate
and Eclipse Membership At Large communities.
Comment 13 Eugene Tarassov CLA 2009-07-30 16:01:08 EDT
I believe plugins support is very valuable feature.
I have committed the patch.
I renamed EN_Plugins to ENABLE_Plugins for consistency, and made few other cosmetic changes.

Thanks for the contribution.
Eugene.
Comment 14 Martin Oberhuber CLA 2009-07-31 05:11:26 EDT
Philippe, this is a great contribution! Many thanks for it.

My only small concern is, that the plugins system actually solves a different problem than we are discussing on this bug (which is meant to deal with the build directory structure, and static componentization). There are many types of system which may not even have shared library support, so keeping your excellent contributino separate from the static build would be helpful.

Philippe, could you create a new enhancement request ("TCF plugins system"), with your same text from comment 12 as description, and attach your code there? - This would help us in tracking everything properly. Thanks!
Comment 15 Martin Oberhuber CLA 2009-08-20 07:20:50 EDT
Comment on attachment 142943 [details]
Proposal patch for TCF plugins system.

The TCF plugins system has been committed via bug 286149.