Re: Add private include area for Subversion-private APIs used between modules

On 8/7/06, Branko Èibej <brane@xbc.nu> wrote:
> Vlad Georgescu wrote:
> > On 8/5/06, Daniel Rall <dlr@collab.net> wrote:
> >> > Garrett Rooney wrote:
> >> > > Perhaps a new subversion/include/private directory for such
> >> things is
> >> > > in order, for headers that are used in various libraries but should
> >> > > not be installed. That would be somewhat nicer than the current
> >> > > practice of doing something like:
> >> > >
> >> > > #include "../libsvn_subr/atomics.h"
> >>
> >> +1! Inter-module APIs aren't necessarily always public. Introduction
> >> of a private include area (APR/APR-Util do something similar) would
> >> allow us with a clear way to share non-public APIs.
> >
> > I created subversion/include/private and added
> > subversion/include/private/*.h to private-includes in build.conf, but
> > ran into a slight problem. Running autogen.sh generates a warning
> > about missing header files when it encounters includes with a path
> > component (e.g. #include "private/svn_atomic.h"), if the path isn't
> > relative to the current directory:
> >
> > Creating build-outputs.mk...
> > WARNING: "private/svn_atomic.h" header not found, file
> > subversion/libsvn_subr/atomic.c
> > WARNING: "private/svn_atomic.h" header not found, file
> > subversion/libsvn_fs_base/bdb/env.c
> >
> > Any ideas on how to fix this? The relevant code is in
> > _scan_for_includes in gen_base.py.
> >
>
> It seems that the include scanning logic there doesn't realise that the
> preprocessor searches for "foo/bar.h" in the whole include path, not
> just the current file's directory. Try the following patch; it's not as
> generic as it could be, but it fixes this particular problem for me.
>
> [[[
> Tell the build generator about the private include area.
>
> * build.conf: Add the private include dir to private-includes.
>
> * build/generator/gen_base.py (_path_endswith): New helper function.
> (IncludeDependencyInfo._scan_for_includes):
> Account for relative path includes.
> ]]]
>
>
> Index: build.conf
> ===================================================================
> --- build.conf (revision 20993)
> +++ build.conf (working copy)
> @@ -28,6 +28,7 @@
> includes = subversion/include/*.h
> include-wildcards = *.h *.i *.swg
> private-includes =
> + subversion/include/private/*.h
> subversion/bindings/swig/include/*.swg
> subversion/libsvn_delta/compose_delta.c
> private-built-includes =
> Index: build/generator/gen_base.py
> ===================================================================
> --- build/generator/gen_base.py (revision 20993)
> +++ build/generator/gen_base.py (working copy)
> @@ -852,6 +852,18 @@
> return native_path(_re_public_include.sub(
> r"subversion/bindings/swig/proxy/\1_h.swg", build_path(fname)))
>
> +def _path_endswith(path, subpath):
> + """Check if SUBPATH is a true path suffix of PATH.
> + """
> + path_len = len(path)
> + subpath_len = len(subpath)
> +
> + return (subpath_len > 0 and path_len >= subpath_len
> + and path[-subpath_len:] == subpath
> + and (path_len == subpath_len
> + or (subpath[0] == os.sep and path[-subpath_len] == os.sep)
> + or path[-subpath_len - 1] == os.sep))
> +
> class IncludeDependencyInfo:
> """Finds all dependencies between a named set of headers, and computes
> closure, so that individual C and SWIG source files can then be scanned, and
> @@ -999,7 +1011,9 @@
> domain_fnames = self._domain.get(os.path.basename(include_param), [])
> if direct_possibility_fname in domain_fnames:
> self._upd_dep_hash(hdrs, direct_possibility_fname, type_code)
> - elif include_param.find(os.sep) == -1 and len(domain_fnames) == 1:
> + elif (len(domain_fnames) == 1
> + and (include_param.find(os.sep) == -1
> + or _path_endswith(domain_fnames[0], include_param))):
> self._upd_dep_hash(hdrs, domain_fnames[0], type_code)
> else:
> # None found
>
>
>
> -- Brane
>
>

Works great, thanks! This should be commited once the atomic patch gets in.

-- 
Vlad