On Sun, 2009-05-24 at 15:24 -0700, David Brownell wrote:
> On Sunday 24 May 2009, Zach Welch wrote:
> > > I'd suggest instead creating a
> > > new include/ area to hold only files which will be getting
> > > commitment for such public usage.
> >
> > This is common and I suppose that I should have listed it (to be fair),
> > but I am not a particular fan for the fact that it splits the interface
> > and implementation into different trees.
>
> Well, yes ... what eventually goes into /usr/include should
> be strongly limited to the interfaces. How anything gets
> implemented inside the library is nobody's busienss except
> for the library maintainers'. They should be able change
> how they do things easily. But that won't happen if all
> kinds of random implementation detail are cast in concrete
> as part of the public /usr/include interface to the library.
AFAIK, we are not installing any headers. I agree with your sentiments
completely; private headers should never be installed. Moreover, I
added a test suite during this work to ensure that the public interfaces
are not polluted by private headers (now or in the future). If changes
to those files add a private header, it should break the test suite.
It also is a developer convenience; the files are all in the same
directory available for browsing and editing. The new tree would split
up the source code, making it more impossible to do `vim jtag.[ch]`.
The changes you request are technically superfluous, and they would
create more patches than required by moving files needlessly. The
thrust of my proposal was to cure the paths for the headers themselves,
and to avoid reorganizing the tree any more than absolutely required.
Where the files live in the source tree has little to no impact on where
they live once installed in the filesystem. To know where they will end
up (and thus which are public), developers should look at Makefile.am or
(after this work is done) the doxygen manual. Location in the tree is
not a reliable indicator of such.
To make this discussion slightly more concrete, I have attached the
patches that I had started to create before posting my original plan.
While these changes build fine (for me), they are far from being ready
to be committed (for _many_ different reasons); however, I think they
will provide a concrete demonstration of my attempt to minimize change.
Cheers,
Zach
Index: src/flash/Makefile.am
===================================================================
--- src/flash/Makefile.am (revision 1906)
+++ src/flash/Makefile.am (working copy)
@@ -1,4 +1,5 @@
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/helper \
-I$(top_srcdir)/src/jtag \
-I$(top_srcdir)/src/target
Index: src/helper/Makefile.am
===================================================================
--- src/helper/Makefile.am (revision 1906)
+++ src/helper/Makefile.am (working copy)
@@ -1,4 +1,5 @@
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/server \
-I$(top_srcdir)/src/target \
-DPKGDATADIR=\"$(pkgdatadir)\" \
Index: src/jtag/Makefile.am
===================================================================
--- src/jtag/Makefile.am (revision 1906)
+++ src/jtag/Makefile.am (working copy)
@@ -1,4 +1,5 @@
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/helper \
-I$(top_srcdir)/src/target
Index: src/pld/Makefile.am
===================================================================
--- src/pld/Makefile.am (revision 1906)
+++ src/pld/Makefile.am (working copy)
@@ -1,4 +1,5 @@
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/server \
-I$(top_srcdir)/src/helper \
-I$(top_srcdir)/src/jtag
Index: src/server/Makefile.am
===================================================================
--- src/server/Makefile.am (revision 1906)
+++ src/server/Makefile.am (working copy)
@@ -1,4 +1,5 @@
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/helper \
-I$(top_srcdir)/src/target \
-I$(top_srcdir)/src/flash \
Index: src/svf/Makefile.am
===================================================================
--- src/svf/Makefile.am (revision 1906)
+++ src/svf/Makefile.am (working copy)
@@ -1,4 +1,5 @@
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/server \
-I$(top_srcdir)/src/helper \
-I$(top_srcdir)/src/jtag
Index: src/target/Makefile.am
===================================================================
--- src/target/Makefile.am (revision 1906)
+++ src/target/Makefile.am (working copy)
@@ -6,6 +6,7 @@
endif
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/helper \
-I$(top_srcdir)/src/jtag \
-I$(top_srcdir)/src/xsvf
Index: src/xsvf/Makefile.am
===================================================================
--- src/xsvf/Makefile.am (revision 1906)
+++ src/xsvf/Makefile.am (working copy)
@@ -1,4 +1,5 @@
AM_CPPFLAGS = \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/server \
-I$(top_srcdir)/src/helper \
-I$(top_srcdir)/src/jtag
Index: src/flash/flash.h
===================================================================
--- src/flash/flash.h (revision 1906)
+++ src/flash/flash.h (working copy)
@@ -26,8 +26,8 @@
#ifndef FLASH_H
#define FLASH_H
-#include "target.h"
-#include "log.h"
+#include <helper/log.h>
+#include <target/target.h>
struct image_s;
Index: src/jtag/jtag.h
===================================================================
--- src/jtag/jtag.h (revision 1906)
+++ src/jtag/jtag.h (working copy)
@@ -23,8 +23,8 @@
#ifndef JTAG_H
#define JTAG_H
-#include "binarybuffer.h"
-#include "log.h"
+#include <helper/binarybuffer.h>
+#include <helper/log.h>
#ifdef _DEBUG_JTAG_IO_
Index: src/server/server.h
===================================================================
--- src/server/server.h (revision 1906)
+++ src/server/server.h (working copy)
@@ -26,7 +26,7 @@
#ifndef SERVER_H
#define SERVER_H
-#include "log.h"
+#include <helper/log.h>
#ifdef HAVE_NETINET_IN_H
#include <netinet/in.h>
Index: src/target/algorithm.h
===================================================================
--- src/target/algorithm.h (revision 1906)
+++ src/target/algorithm.h (working copy)
@@ -20,7 +20,7 @@
#ifndef ALGORITHM_H
#define ALGORITHM_H
-#include "types.h"
+#include <helper/types.h>
enum param_direction
{
Index: src/target/breakpoints.h
===================================================================
--- src/target/breakpoints.h (revision 1906)
+++ src/target/breakpoints.h (working copy)
@@ -20,7 +20,7 @@
#ifndef BREAKPOINTS_H
#define BREAKPOINTS_H
-#include "types.h"
+#include <helper/types.h>
struct target_s;
Index: src/target/target.h
===================================================================
--- src/target/target.h (revision 1906)
+++ src/target/target.h (working copy)
@@ -26,9 +26,9 @@
#ifndef TARGET_H
#define TARGET_H
-#include "breakpoints.h"
-#include "algorithm.h"
-#include "command.h"
+#include <target/breakpoints.h>
+#include <target/algorithm.h>
+#include <helper/command.h>
struct reg_s;
struct trace_s;
Index: configure.in
===================================================================
--- configure.in (revision 1906)
+++ configure.in (working copy)
@@ -930,4 +930,17 @@
AC_MSG_RESULT([$EXEEXT_FOR_BUILD])
AC_SUBST(EXEEXT_FOR_BUILD)
-AC_OUTPUT(Makefile src/Makefile src/helper/Makefile src/jtag/Makefile src/xsvf/Makefile src/svf/Makefile src/target/Makefile src/server/Makefile src/flash/Makefile src/pld/Makefile doc/Makefile)
+AC_OUTPUT([
+ Makefile
+ doc/Makefile
+ src/Makefile
+ src/helper/Makefile
+ src/jtag/Makefile
+ src/svf/Makefile
+ src/pld/Makefile
+ src/xsvf/Makefile
+ src/target/Makefile
+ src/flash/Makefile
+ src/server/Makefile
+ tests/Makefile
+ ])
Index: Makefile.am
===================================================================
--- Makefile.am (revision 1906)
+++ Makefile.am (working copy)
@@ -9,7 +9,7 @@
contrib/libdcc/README \
contrib/openocd.udev
-SUBDIRS = src doc
+SUBDIRS = src doc tests
EXTRA_DIST = \
Doxyfile.in \
Index: tests/900_server.c
===================================================================
--- tests/900_server.c (revision 0)
+++ tests/900_server.c (revision 0)
@@ -0,0 +1,9 @@
+#define HAVE_SYS_TYPES_H
+#define HAVE_STDBOOL_H
+#define HAVE_NETINET_IN_H
+#include <server/server.h>
+#include "000_test_harness.h"
+
+#define TEST_BODY() TEST_BODY_EMPTY
+#define TEST_RESULT() 0
+DEFINE_TEST_MAIN_ENTRY
Property changes on: tests/900_server.c
___________________________________________________________________
Added: svn:eol-style
+ native
Index: tests/100_helper.c
===================================================================
--- tests/100_helper.c (revision 0)
+++ tests/100_helper.c (revision 0)
@@ -0,0 +1,9 @@
+#define HAVE_SYS_TYPES_H
+#define HAVE_STDBOOL_H
+#include <helper/command.h>
+#include <helper/log.h>
+#include "000_test_harness.h"
+
+#define TEST_BODY() TEST_BODY_EMPTY
+#define TEST_RESULT() 0
+DEFINE_TEST_MAIN_ENTRY
Property changes on: tests/100_helper.c
___________________________________________________________________
Added: svn:eol-style
+ native
Index: tests/700_flash.c
===================================================================
--- tests/700_flash.c (revision 0)
+++ tests/700_flash.c (revision 0)
@@ -0,0 +1,7 @@
+#define HAVE_STDBOOL_H
+#include <flash/flash.h>
+#include "000_test_harness.h"
+
+#define TEST_BODY() TEST_BODY_EMPTY
+#define TEST_RESULT() 0
+DEFINE_TEST_MAIN_ENTRY
Property changes on: tests/700_flash.c
___________________________________________________________________
Added: svn:eol-style
+ native
Index: tests/000_test_harness.h
===================================================================
--- tests/000_test_harness.h (revision 0)
+++ tests/000_test_harness.h (revision 0)
@@ -0,0 +1,13 @@
+#ifndef OOCD_TEST_HARNESS_H
+#define OOCD_TEST_HARNESS_H
+
+#define DEFINE_TEST_MAIN_ENTRY \
+ int main(int argc, char **argv) \
+ { \
+ TEST_BODY(); \
+ return TEST_RESULT(); \
+ }
+
+#define TEST_BODY_EMPTY do { /* nothing */ } while (0)
+
+#endif // OOCD_TEST_HARNESS_H
Property changes on: tests/000_test_harness.h
___________________________________________________________________
Added: svn:eol-style
+ native
Index: tests/300_jtag.c
===================================================================
--- tests/300_jtag.c (revision 0)
+++ tests/300_jtag.c (revision 0)
@@ -0,0 +1,8 @@
+#define HAVE_STDBOOL_H
+#define HAVE_STDINT_H
+#include <jtag/jtag.h>
+#include "000_test_harness.h"
+
+#define TEST_BODY() TEST_BODY_EMPTY
+#define TEST_RESULT() 0
+DEFINE_TEST_MAIN_ENTRY
Property changes on: tests/300_jtag.c
___________________________________________________________________
Added: svn:eol-style
+ native
Index: tests/Makefile.am
===================================================================
--- tests/Makefile.am (revision 0)
+++ tests/Makefile.am (revision 0)
@@ -0,0 +1,21 @@
+AM_CPPFLAGS = -I$(top_srcdir)/src
+
+TESTS = \
+ 100_helper \
+ 300_jtag \
+ 500_target \
+ 700_flash \
+ 900_server
+
+noinst_PROGRAMS = $(TESTS)
+
+noinst_HEADERS = 000_test_harness.h
+
+100_helper_SOURCES = 100_helper.c
+300_jtag_SOURCES = 300_jtag.c
+500_target_SOURCES = 500_target.c
+700_flash_SOURCES = 700_flash.c
+900_server_SOURCES = 900_server.c
+
+MAINTAINERCLEANFILES = Makefile.in
+
Property changes on: tests/Makefile.am
___________________________________________________________________
Added: svn:eol-style
+ native
Index: tests/500_target.c
===================================================================
--- tests/500_target.c (revision 0)
+++ tests/500_target.c (revision 0)
@@ -0,0 +1,7 @@
+#define HAVE_STDBOOL_H
+#include <target/target.h>
+#include "000_test_harness.h"
+
+#define TEST_BODY() TEST_BODY_EMPTY
+#define TEST_RESULT() 0
+DEFINE_TEST_MAIN_ENTRY
Property changes on: tests/500_target.c
___________________________________________________________________
Added: svn:eol-style
+ native
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
