You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2015/10/23 15:56:41 UTC

svn commit: r1710215 - in /subversion/trunk/subversion: bindings/javahl/native/SVNClient.cpp include/svn_client.h libsvn_client/cleanup.c

Author: philip
Date: Fri Oct 23 13:56:40 2015
New Revision: 1710215

URL: http://svn.apache.org/viewvc?rev=1710215&view=rev
Log:
Allow svn_client_vacuum and svn_client_cleanup2 to accept non-absolute
paths and convert to absolute, rather than asserting that the passed
paths are absolute.  This makes the functions behave like other client
functions.

* subversion/include/svn_client.h
  (svn_client_vacuum, svn_client_cleanup2): Rename parameter.

* subversion/libsvn_client/cleanup.c
  (svn_client_vacuum, svn_client_cleanup2): Rename parameter, convert
   to abspath.

* subversion/bindings/javahl/native/SVNClient.cpp
  (SVNClient::vacuum): Add Path checking to parameter.

Modified:
    subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp
    subversion/trunk/subversion/include/svn_client.h
    subversion/trunk/subversion/libsvn_client/cleanup.c

Modified: subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp?rev=1710215&r1=1710214&r2=1710215&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp Fri Oct 23 13:56:40 2015
@@ -1564,7 +1564,10 @@ void SVNClient::vacuum(const char *path,
     if (ctx == NULL)
         return;
 
-    SVN_JNI_ERR(svn_client_vacuum(path,
+    Path checkedPath(path, subPool);
+    SVN_JNI_ERR(checkedPath.error_occurred(),);
+
+    SVN_JNI_ERR(svn_client_vacuum(checkedPath.c_str(),
                                   remove_unversioned_items,
                                   remove_ignored_items,
                                   fix_recorded_timestamps,

Modified: subversion/trunk/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=1710215&r1=1710214&r2=1710215&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_client.h (original)
+++ subversion/trunk/subversion/include/svn_client.h Fri Oct 23 13:56:40 2015
@@ -4098,22 +4098,22 @@ svn_client_mergeinfo_log_eligible(const
  * @{
  */
 
-/** Recursively vacuum a working copy directory @a dir, removing unnecessary
+/** Recursively vacuum a working copy directory @a path, removing unnecessary
  * data.
  *
  * If @a include_externals is @c TRUE, recurse into externals and vacuum them
  * as well.
  *
  * If @a remove_unversioned_items is @c TRUE, remove unversioned items
- * in @a dir after successful working copy cleanup.
+ * in @a path after successful working copy cleanup.
  * If @a remove_ignored_items is @c TRUE, remove ignored unversioned items
- * in @a dir after successful working copy cleanup.
+ * in @a path after successful working copy cleanup.
  *
  * If @a fix_recorded_timestamps is @c TRUE, this function fixes recorded
  * timestamps for unmodified files in the working copy, reducing comparision
  * time on future checks.
  *
- * If @a vacuum_pristines is @c TRUE, and @a dir_abspath points to the working
+ * If @a vacuum_pristines is @c TRUE, and @a path points to the working
  * copy root unreferenced files in the pristine store are removed.
  *
  * When asked to remove unversioned or ignored items, and the working copy
@@ -4132,7 +4132,7 @@ svn_client_mergeinfo_log_eligible(const
  * @since New in 1.9.
  */
 svn_error_t *
-svn_client_vacuum(const char *dir_abspath,
+svn_client_vacuum(const char *path,
                   svn_boolean_t remove_unversioned_items,
                   svn_boolean_t remove_ignored_items,
                   svn_boolean_t fix_recorded_timestamps,
@@ -4142,11 +4142,11 @@ svn_client_vacuum(const char *dir_abspat
                   apr_pool_t *scratch_pool);
 
 
-/** Recursively cleanup a working copy directory @a dir_abspath, finishing any
+/** Recursively cleanup a working copy directory @a path, finishing any
  * incomplete operations, removing lockfiles, etc.
  *
  * If @a break_locks is @c TRUE, existing working copy locks at or below @a
- * dir_abspath are broken, otherwise a normal write lock is obtained.
+ * path are broken, otherwise a normal write lock is obtained.
  *
  * If @a fix_recorded_timestamps is @c TRUE, this function fixes recorded
  * timestamps for unmodified files in the working copy, reducing comparision
@@ -4156,7 +4156,7 @@ svn_client_vacuum(const char *dir_abspat
  * mod_dav served repositories is cleared. This clearing invalidates some
  * cached information used for pre-HTTPv2 repositories.
  *
- * If @a vacuum_pristines is @c TRUE, and @a dir_abspath points to the working
+ * If @a vacuum_pristines is @c TRUE, and @a path points to the working
  * copy root unreferenced files in the pristine store are removed.
  *
  * If @a include_externals is @c TRUE, recurse into externals and clean
@@ -4172,7 +4172,7 @@ svn_client_vacuum(const char *dir_abspat
  * @since New in 1.9.
  */
 svn_error_t *
-svn_client_cleanup2(const char *dir_abspath,
+svn_client_cleanup2(const char *path,
                     svn_boolean_t break_locks,
                     svn_boolean_t fix_recorded_timestamps,
                     svn_boolean_t clear_dav_cache,

Modified: subversion/trunk/subversion/libsvn_client/cleanup.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/cleanup.c?rev=1710215&r1=1710214&r2=1710215&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/cleanup.c (original)
+++ subversion/trunk/subversion/libsvn_client/cleanup.c Fri Oct 23 13:56:40 2015
@@ -215,7 +215,7 @@ cleanup_status_walk(void *baton,
 }
 
 svn_error_t *
-svn_client_cleanup2(const char *dir_abspath,
+svn_client_cleanup2(const char *path,
                     svn_boolean_t break_locks,
                     svn_boolean_t fix_recorded_timestamps,
                     svn_boolean_t clear_dav_cache,
@@ -224,9 +224,15 @@ svn_client_cleanup2(const char *dir_absp
                     svn_client_ctx_t *ctx,
                     apr_pool_t *scratch_pool)
 {
-  SVN_ERR_ASSERT(svn_dirent_is_absolute(dir_abspath));
+  const char *local_abspath;
+  
+  if (svn_path_is_url(path))
+    return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                             _("'%s' is not a local path"), path);
 
-  SVN_ERR(do_cleanup(dir_abspath,
+  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, scratch_pool));
+
+  SVN_ERR(do_cleanup(local_abspath,
                      break_locks,
                      fix_recorded_timestamps,
                      clear_dav_cache,
@@ -240,7 +246,7 @@ svn_client_cleanup2(const char *dir_absp
 }
 
 svn_error_t *
-svn_client_vacuum(const char *dir_abspath,
+svn_client_vacuum(const char *path,
                   svn_boolean_t remove_unversioned_items,
                   svn_boolean_t remove_ignored_items,
                   svn_boolean_t fix_recorded_timestamps,
@@ -249,9 +255,15 @@ svn_client_vacuum(const char *dir_abspat
                   svn_client_ctx_t *ctx,
                   apr_pool_t *scratch_pool)
 {
-  SVN_ERR_ASSERT(svn_dirent_is_absolute(dir_abspath));
+  const char *local_abspath;
+  
+  if (svn_path_is_url(path))
+    return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                             _("'%s' is not a local path"), path);
+
+  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, scratch_pool));
 
-  SVN_ERR(do_cleanup(dir_abspath,
+  SVN_ERR(do_cleanup(local_abspath,
                      FALSE /* break_locks */,
                      fix_recorded_timestamps,
                      FALSE /* clear_dav_cache */,



Re: svn commit: r1710215 - in /subversion/trunk/subversion:bindings/javahl/native/SVNClient.cpp include/svn_client.hlibsvn_client/cleanup.c

Posted by Philip Martin <ph...@wandisco.com>.
<be...@qqmail.nl> writes:

> While many libsvn client functions still take every path as argument,
> most new functions (and many revved functions) now only take absolute
> paths. I don’t think that we should reverse this step by changing the
> behavior further.

OK, I've reverted the libsvn_client part, leaving the JNI part. See
r1710290.

-- 
Philip Martin
WANdisco

RE: svn commit: r1710215 - in /subversion/trunk/subversion:bindings/javahl/native/SVNClient.cpp include/svn_client.hlibsvn_client/cleanup.c

Posted by be...@qqmail.nl.
While many libsvn client functions still take every path as argument, most new functions (and many revved functions) now only take absolute paths. I don’t think that we should reverse this step by changing the behavior further.

Changing the documentation in a backport may break future usages of the api for those who develop against new 1.9 versions and then link against an older version.

See some recent backport nominations of Julian that didn’t even want to just canonicalize input from environment variables, where we didn’t canonicalize before.


Slowly moving towards canonicalized required (which we documented before 1.0 as required) and absolute paths for working copy paths (since 1.7 on most if not all new apis) are small steps to making the api easier to depend on. (We can’t guarantee stable notifications on relative paths; especially when using multiple targets at the same time)


I’d rather see a backport of the fix in javahl as recommended in the original mail posts, while keeping the api working as promised. 

Unnecessary abspath conversions everywhere as we had in early 1.7 development are performance killers. It is better to convert as early as possible at the library boundary.

     Bert

Sent from Mail for Windows 10



From: philip@apache.org
Sent: vrijdag 23 oktober 2015 15:56
To: commits@subversion.apache.org
Subject: svn commit: r1710215 - in /subversion/trunk/subversion:bindings/javahl/native/SVNClient.cpp include/svn_client.hlibsvn_client/cleanup.c


Author: philip
Date: Fri Oct 23 13:56:40 2015
New Revision: 1710215

URL: http://svn.apache.org/viewvc?rev=1710215&view=rev
Log:
Allow svn_client_vacuum and svn_client_cleanup2 to accept non-absolute
paths and convert to absolute, rather than asserting that the passed
paths are absolute.  This makes the functions behave like other client
functions.