Max Bowsher wrote:
> Philip Martin wrote:
>> maxb@tigris.org writes:
>>> * subversion/include/svn_error_codes.h: Remove unused #include
>>> "svn_props.h".
>>
>> Removing an unsed #include from the public header files means the new
>> version is not "source compatible" with the old, if we follow the
>> rules strictly then this change needs to wait until 2.0.
Yes.
> /me would rather say "naughty source, #include what you need, don't be
> lazy", but given what we've promised, it seems I need to go revert r14002.
It is a bit naughty of the C files, but not unreasonable, so yes you do.
Note that it would be perfectly good and proper to re-commit the other parts of
your r14002, i.e. adding "#include svn_props.h" to C files that were previously
relying on it being included through svn_error_codes.h.
Note also that this is just one of many unused #includes within our public
headers. I have the attached patch sitting around, waiting for version 2,
which removes a whole bunch of them. It will need re-checking of course, and
the C files will need updating, when that time comes.
I was thinking about whether we should mark the unused #includes in some way.
I don't think a comment like "only for source compatibilty" would be easily
maintainable, because a future version of the header could start to use
something from the included header or stop using anything from a different
included header, and there would be no easy way of noticing that the comments
are then wrong. We could instead enclose them in something like "#if
SOURCE_COMPATIBILITY", or, in practice,
#ifndef LOSE_SOURCE_COMPATIBILTY
#include "svn_props.h"
#endif
which would allow a maintainer occasionally to build with
"LOSE_SOURCE_COMPATIBILTY" defined to check that all the C files are looking
after their own needs, but there still isn't a way of checking that that "#if"
has been placed on all the #includes that it should have.
So I don't think it's worth trying to track the set of unused #include
statements, because the set may change (though very little) over time, and it
would be just something else to maintain.
We just need to do this as part of the API overhaul when v2 comes.
- Julian
Rationalize #includes in public C header files.
### Do this when preparing version 2.
Remove each #include that is neither directly used nor provided on
purpose for convenience. Include a lower-level header instead where
necessary.
Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h (revision 11885)
+++ subversion/include/svn_error_codes.h (working copy)
@@ -42,11 +42,8 @@
#if defined(SVN_ERROR_BUILD_ARRAY) || !defined(SVN_ERROR_ENUM_DEFINED)
-#include <apr.h>
#include <apr_errno.h> /* APR's error system */
-#include "svn_props.h" /* For SVN_PROP_EXTERNALS. */
-
#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */
Index: subversion/include/svn_diff.h
===================================================================
--- subversion/include/svn_diff.h (revision 11885)
+++ subversion/include/svn_diff.h (working copy)
@@ -48,7 +48,6 @@
#include <apr_file_io.h>
#include "svn_types.h"
-#include "svn_error.h"
#include "svn_io.h"
#include "svn_version.h"
Index: subversion/include/svn_test.h
===================================================================
--- subversion/include/svn_test.h (revision 11885)
+++ subversion/include/svn_test.h (working copy)
@@ -24,10 +24,7 @@
#include <apr_pools.h>
#include "svn_delta.h"
-#include "svn_path.h"
#include "svn_types.h"
-#include "svn_error.h"
-#include "svn_string.h"
#ifdef __cplusplus
Index: subversion/include/svn_path.h
===================================================================
--- subversion/include/svn_path.h (revision 11885)
+++ subversion/include/svn_path.h (working copy)
@@ -39,7 +39,6 @@
#include <apr_tables.h>
#include "svn_string.h"
-#include "svn_error.h"
#ifdef __cplusplus
Index: subversion/include/svn_xml.h
===================================================================
--- subversion/include/svn_xml.h (revision 11885)
+++ subversion/include/svn_xml.h (working copy)
@@ -28,7 +28,6 @@
#include <apr_pools.h>
#include <apr_hash.h>
-#include "svn_error.h"
#include "svn_string.h"
#ifdef __cplusplus
Index: subversion/include/svn_config.h
===================================================================
--- subversion/include/svn_config.h (revision 11885)
+++ subversion/include/svn_config.h (working copy)
@@ -27,7 +27,6 @@
#include <apr_pools.h>
#include "svn_types.h"
-#include "svn_error.h"
#ifdef __cplusplus
Index: subversion/include/svn_hash.h
===================================================================
--- subversion/include/svn_hash.h (revision 11885)
+++ subversion/include/svn_hash.h (working copy)
@@ -29,7 +29,6 @@
#include "svn_types.h"
#include "svn_io.h"
-#include "svn_error.h"
#ifdef __cplusplus
Index: subversion/include/svn_md5.h
===================================================================
--- subversion/include/svn_md5.h (revision 11885)
+++ subversion/include/svn_md5.h (working copy)
@@ -24,8 +24,7 @@
#include <apr_pools.h>
#include <apr_md5.h>
-#include "svn_error.h"
-#include "svn_pools.h"
+#include "svn_types.h"
#ifdef __cplusplus
extern "C" {
Index: subversion/include/svn_pools.h
===================================================================
--- subversion/include/svn_pools.h (revision 11885)
+++ subversion/include/svn_pools.h (working copy)
@@ -29,8 +29,6 @@
#include <apr_errno.h> /* APR's error system */
#include <apr_pools.h>
-#include "svn_types.h"
-
#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */
Index: subversion/include/svn_utf.h
===================================================================
--- subversion/include/svn_utf.h (revision 11885)
+++ subversion/include/svn_utf.h (working copy)
@@ -26,7 +26,6 @@
#include <apr_xlate.h>
-#include "svn_error.h"
#include "svn_string.h"
#ifdef __cplusplus
Index: subversion/include/svn_time.h
===================================================================
--- subversion/include/svn_time.h (revision 11885)
+++ subversion/include/svn_time.h (working copy)
@@ -25,7 +25,7 @@
#include <apr_pools.h>
#include <apr_time.h>
-#include "svn_error.h"
+#include "svn_types.h"
#ifdef __cplusplus
extern "C" {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 7 18:31:24 2005