You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/08/04 22:03:37 UTC
Add private include area for Subversion-private APIs used between modules
On Fri, 04 Aug 2006, Branko Čibej wrote:
> Garrett Rooney wrote:
> > On 8/4/06, Branko Čibej <br...@xbc.nu> wrote:
> >> Vlad Georgescu wrote:
> >> > [[[
> >> > Extract the atomic initialization code from libsvn_fs_base/bdb/env.c
> >> > and convert
> >> > it to a generic implementation that can be used to initialize other
> >> > libraries
> >> > (*cough* Cyrus SASL *cough*). Make bdb use the new implementation.
> >> >
> >> > * subversion/include/svn_atomic.h: New file.
> >> >
> >> > * subversion/libsvn_subr/atomic.c: New file.
> >> >
> >> > * subversion/libsvn_fs_base/bdb/env.c
> >> > Include svn_atomic.h instead of apr_atomic.h.
> >> > (svn__atomic_t,
> >> > svn__atomic_read,
> >> > svn__atomic_set,
> >> > svn__atomic_cas): These are now in subversion/include/svn_atomic.h
> >> > and have a single underscore. All calls were changed to reflect this.
> >> > (BDB_CACHE_UNINITIALIZED,
> >> > BDB_CACHE_START_INIT,
> >> > BDB_CACHE_INIT_FAILED,
> >> > BDB_CACHE_INITIALIZED): Deleted. Similar definitions are now in
> >> > svn_atomic.h.
> >> I cannot for the life of me imagine why these definitions should be part
> >> of Subversion's public API. In fact, they shouldn't be. They should
> >> either go into svn_private_config.h, or, if that's not appropriate
> >> (being a generated header), we should introduce a new _private_ header
> >> for them.
> >>
> >> -1 on putting them in include/svn_atomic.h, they have no use outside the
> >> library implementation.
> >
> > 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.
Re: Add private include area for Subversion-private APIs used between modules
Posted by Vlad Georgescu <vg...@gmail.com>.
On 8/7/06, Branko Čibej <br...@xbc.nu> wrote:
> Vlad Georgescu wrote:
> > On 8/5/06, Daniel Rall <dl...@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
Re: Add private include area for Subversion-private APIs used between modules
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/6/06, Branko Čibej <br...@xbc.nu> wrote:
> Vlad Georgescu wrote:
> > On 8/5/06, Daniel Rall <dl...@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
Since I'm looking at committing Vlad's patch for the atomic stuff and
this is a prerequisite for it, I just committed the gen_base.py part
of this.
-garrett
Re: Add private include area for Subversion-private APIs used between
modules
Posted by Branko Čibej <br...@xbc.nu>.
Vlad Georgescu wrote:
> On 8/5/06, Daniel Rall <dl...@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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Add private include area for Subversion-private APIs used between modules
Posted by Vlad Georgescu <vg...@gmail.com>.
On 8/5/06, Daniel Rall <dl...@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.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org