svn commit: r1382304 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

2012-09-08 Thread stefan2
Author: stefan2
Date: Sat Sep  8 13:51:39 2012
New Revision: 1382304

URL: http://svn.apache.org/viewvc?rev=1382304view=rev
Log:
Fix a minor issue with props deltification: Deltify against a props
representation instead of some text rep.

* subversion/libsvn_fs_fs/fs_fs.c
  (choose_delta_base, write_hash_delta_rep): add props flag to parameters
  (rep_write_get_baton, write_final_rev): update callers

Modified:
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1382304r1=1382303r2=1382304view=diff
==
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Sep  8 13:51:39 2012
@@ -6782,11 +6782,14 @@ rep_write_contents(void *baton,
 
 /* Given a node-revision NODEREV in filesystem FS, return the
representation in *REP to use as the base for a text representation
-   delta.  Perform temporary allocations in *POOL. */
+   delta if PROPS is FALSE.  If PROPS has been set, a suitable props
+   base representation will be returned.  Perform temporary allocations
+   in *POOL. */
 static svn_error_t *
 choose_delta_base(representation_t **rep,
   svn_fs_t *fs,
   node_revision_t *noderev,
+  svn_boolean_t props,
   apr_pool_t *pool)
 {
   int count;
@@ -6835,7 +6838,8 @@ choose_delta_base(representation_t **rep
 SVN_ERR(svn_fs_fs__get_node_revision(base, fs,
  base-predecessor_id, pool));
 
-  *rep = base-data_rep;
+  /* return a suitable base representation */
+  *rep = props ? base-prop_rep : base-data_rep;
 
   return SVN_NO_ERROR;
 }
@@ -6882,7 +6886,7 @@ rep_write_get_baton(struct rep_write_bat
   SVN_ERR(get_file_offset(b-rep_offset, file, b-pool));
 
   /* Get the base for this delta. */
-  SVN_ERR(choose_delta_base(base_rep, fs, noderev, b-pool));
+  SVN_ERR(choose_delta_base(base_rep, fs, noderev, FALSE, b-pool));
   SVN_ERR(read_representation(source, fs, base_rep, b-pool));
 
   /* Write out the rep header. */
@@ -7301,7 +7305,8 @@ write_hash_rep(representation_t *rep,
is not NULL, it will be used in addition to the on-disk cache to find
earlier reps with the same content.  When such existing reps can be found,
we will truncate the one just written from the file and return the existing
-   rep.  Perform temporary allocations in POOL. */
+   rep.  If PROPS is set, assume that we want to a props representation as
+   the base for our delta.  Perform temporary allocations in POOL. */
 static svn_error_t *
 write_hash_delta_rep(representation_t *rep,
  apr_file_t *file,
@@ -7309,6 +7314,7 @@ write_hash_delta_rep(representation_t *r
  svn_fs_t *fs,
  node_revision_t *noderev,
  apr_hash_t *reps_hash,
+ svn_boolean_t props,
  apr_pool_t *pool)
 {
   svn_txdelta_window_handler_t diff_wh;
@@ -7329,7 +7335,7 @@ write_hash_delta_rep(representation_t *r
   int diff_version = ffd-format = SVN_FS_FS__MIN_SVNDIFF1_FORMAT ? 1 : 0;
 
   /* Get the base for this delta. */
-  SVN_ERR(choose_delta_base(base_rep, fs, noderev, pool));
+  SVN_ERR(choose_delta_base(base_rep, fs, noderev, props, pool));
   SVN_ERR(read_representation(source, fs, base_rep, pool));
 
   SVN_ERR(get_file_offset(rep-offset, file, pool));
@@ -7564,7 +7570,7 @@ write_final_rev(const svn_fs_id_t **new_
   if (ffd-deltify_directories)
 SVN_ERR(write_hash_delta_rep(noderev-data_rep, file,
  str_entries, fs, noderev, NULL,
- pool));
+ FALSE, pool));
   else
 SVN_ERR(write_hash_rep(noderev-data_rep, file, str_entries,
fs, NULL, pool));
@@ -7603,7 +7609,7 @@ write_final_rev(const svn_fs_id_t **new_
   if (ffd-deltify_properties)
 SVN_ERR(write_hash_delta_rep(noderev-prop_rep, file,
  proplist, fs, noderev, reps_hash,
- pool));
+ TRUE, pool));
   else
 SVN_ERR(write_hash_rep(noderev-prop_rep, file, proplist,
fs, reps_hash, pool));




svn commit: r1382333 - in /subversion/trunk/subversion/libsvn_fs_fs: fs_fs.c fs_fs.h

2012-09-08 Thread danielsh
Author: danielsh
Date: Sat Sep  8 17:33:07 2012
New Revision: 1382333

URL: http://svn.apache.org/viewvc?rev=1382333view=rev
Log:
Follow-up to r1381808: move logic around.  No externally-visible change.

* subversion/libsvn_fs_fs/fs_fs.h
  (svn_fs_fs__read_noderev): Remove ALLOW_FOR_TXN_ROOTS parameter.

* subversion/libsvn_fs_fs/fs_fs.c
  (svn_fs_fs__read_noderev): Implement signature change.
  (get_node_revision_body): Track signature change, move workaround here.

Modified:
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1382333r1=1382332r2=1382333view=diff
==
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Sep  8 17:33:07 2012
@@ -2284,8 +2284,11 @@ get_node_revision_body(node_revision_t *
   SVN_ERR(svn_fs_fs__read_noderev(noderev_p,
   svn_stream_from_aprfile2(revision_file, 
FALSE,
pool),
-  svn_fs_fs__id_txn_id(id) != NULL,
   pool));
+  /* Workaround issue #4031: is-fresh-txn-root in revision files. */
+  if (svn_fs_fs__id_txn_id(id) != NULL)
+(*noderev_p)-is_fresh_txn_root = FALSE;
+
 
   /* The noderev is not in cache, yet. Add it, if caching has been enabled. */
   return set_cached_node_revision_body(*noderev_p, fs, id, pool);
@@ -2294,7 +2297,6 @@ get_node_revision_body(node_revision_t *
 svn_error_t *
 svn_fs_fs__read_noderev(node_revision_t **noderev_p,
 svn_stream_t *stream,
-svn_boolean_t allow_for_txn_roots,
 apr_pool_t *pool)
 {
   apr_hash_t *headers;
@@ -2425,9 +2427,7 @@ svn_fs_fs__read_noderev(node_revision_t 
 }
 
   /* Get whether this is a fresh txn root. */
-  value = allow_for_txn_roots
-? apr_hash_get(headers, HEADER_FRESHTXNRT, APR_HASH_KEY_STRING)
-: NULL;
+  value = apr_hash_get(headers, HEADER_FRESHTXNRT, APR_HASH_KEY_STRING);
   noderev-is_fresh_txn_root = (value != NULL);
 
   /* Get the mergeinfo count. */

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h?rev=1382333r1=1382332r2=1382333view=diff
==
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h Sat Sep  8 17:33:07 2012
@@ -94,13 +94,11 @@ svn_fs_fs__write_noderev(svn_stream_t *o
  apr_pool_t *pool);
 
 /* Read a node-revision from STREAM. Set *NODEREV to the new structure,
-   allocated in POOL. If ALLOW_FOR_TXN_ROOTS is FALSE, the is-fresh-txn-root
-   flag will be ignored. */
+   allocated in POOL. */
 /* ### Currently used only by fs_fs.c */
 svn_error_t *
 svn_fs_fs__read_noderev(node_revision_t **noderev,
 svn_stream_t *stream,
-svn_boolean_t allow_for_txn_roots,
 apr_pool_t *pool);
 
 




Re: svn commit: r1381808 - in /subversion/trunk/subversion/libsvn_fs_fs: fs_fs.c fs_fs.h

2012-09-08 Thread Daniel Shahaf
Stefan Fuhrmann wrote on Sat, Sep 08, 2012 at 01:32:54 +0200:
 On Fri, Sep 7, 2012 at 9:36 PM, Daniel Shahaf danie...@elego.de wrote:
 
  I think this fix is wrong.  svn_fs_fs__read_noderev() should return the
  parsed noderev as is --- if it's a revision file and the noderev
  contains an is-fresh-txn-root field, then it needs to reflect that.
 
 
 The problem is that the whole situation is very messy:
 
 * the flag basically says this is still under construction,
   redirect data lookup as required. For committed data,
   this is clearly an invalid state or at least useless info.
 
 * there is some bug that will leave this flag in empty
   revisions. This bug needs to be found an fixed (not
   a big problem, just needs to be done).
 * _existing_ repositories have cases of this otherwise
   benign corruption.These cases need to be handled
   gracefully. 1.8 checks for issue #4129 will report all
   those repositories as corrupted.
 
 * the fix for issue #2608 makes svn_fs_fs__dag_get_revision
   report the wrong revision.
 
 The only other way to make that work would be an
 additional flag in noderev structure that allows for the
 is-fresh-txn-root flag to be set but ignoring it in all
 other code.
 
 Would you be more happy with that solution?
 

Well, I see two things here:

- It would be cleaner to have svn_fs_fs__read_noderev() a simple
  unserializer --- done in r1382333.

- The users of is_fresh_txn_root should be refactored --- I wonder if
  we couldn't just remove the need for the field altogether without much
  work; see my last comment on issue #4031 for details.

Sorry for the brevity yesterday, I think that commit review would have
benefited from being written today rather than yesterday.

 -- Stefan^2.
 
 -- 
 *
 
 Join us this October at Subversion Live
 2012http://www.wandisco.com/svn-live-2012
  for two days of best practice SVN training, networking, live demos,
 committer meet and greet, and more! Space is limited, so get signed up
 todayhttp://www.wandisco.com/svn-live-2012
 !
 *