You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/01/02 14:46:57 UTC

svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Author: stefan2
Date: Fri Jan  2 13:46:57 2015
New Revision: 1649029

URL: http://svn.apache.org/r1649029
Log:
Sync FSX with FSFS: Merge DAG-related sub-pool introduction
patches r1647905, r1648243, r1648253 and r1648272 from FSFS
and resolve the usual text conflicts due to naming differences.
Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.

Modified:
    subversion/trunk/subversion/libsvn_fs_x/   (props changed)
    subversion/trunk/subversion/libsvn_fs_x/dag.c
    subversion/trunk/subversion/libsvn_fs_x/tree.c

Propchange: subversion/trunk/subversion/libsvn_fs_x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jan  2 13:46:57 2015
@@ -86,5 +86,5 @@
 /subversion/branches/verify-at-commit/subversion/libsvn_fs_x:1462039-1462408
 /subversion/branches/verify-keep-going/subversion/libsvn_fs_x:1439280-1492639,1546002-1546110
 /subversion/branches/wc-collate-path/subversion/libsvn_fs_x:1402685-1480384
-/subversion/trunk/subversion/libsvn_fs_fs:1415133-1596500,1596567,1597414,1597989,1598273,1599140,1600872,1601633,1603485-1603487,1603499,1603605,1604128,1604188,1604413-1604414,1604416-1604417,1604421,1604442,1604700,1604717,1604720,1604726,1604755,1604794,1604802,1604824,1604836,1604844,1604902-1604903,1604911,1604925,1604933,1604947,1605059-1605060,1605064-1605065,1605068,1605071-1605073,1605075,1605123,1605188-1605189,1605191,1605197,1605444,1605633,1606132,1606142,1606144,1606514,1606526,1606528,1606551,1606554,1606564,1606598-1606599,1606656,1606658,1606662,1606744,1606840,1607085,1607572,1612407,1612810,1613339,1613872,1614611,1615348,1615351-1615352,1615356,1616338-1616339,1616613,1617586,1617688,1618138,1618151,1618153,1618226,1618641,1618653,1618662,1619068,1619358,1619413,1619769,1619774,1620602,1620909,1620912,1620928,1620930,1621275,1621635,1622931,1622937,1622942,1622946,1622959-1622960,1622963,1622987,1623007,1623368,1623373,1623377,1623379,1623381,1623398,1623402,162
 4011,1624265,1624512,1626246,1626871,1626873,1626886,1627497-1627498,1627502,1627947-1627949,1627966,1628083,1628093,1628158-1628159,1628161,1628392-1628393,1628415,1628427,1628676,1628738,1628762,1628764,1629854-1629855,1629857,1629865,1629873,1629875,1629879,1630067,1630070,1631049-1631051,1631075,1631115,1631171,1631180,1631185-1631186,1631196-1631197,1631239-1631240,1631548,1631550,1631563,1631567,1631588,1631598,1632646,1632776,1632849,1632851-1632853,1632856-1632857,1632868,1632908,1632926,1633232,1633617-1633618,1634872,1634875,1634879-1634880,1634920,1636478,1636483,1636629,1636644,1637184,1637186,1637330,1637358,1637363,1637393,1639319,1639322,1639335,1639348,1639352,1639355,1639358,1639414,1639419,1639426,1639430,1639436,1639440,1639549,1640061-1640062,1640197,1640915,1640966,1641013,1643139,1643233,1645567,1646021,1646712,1646716,1647537,1647540-1647541,1647820
+/subversion/trunk/subversion/libsvn_fs_fs:1415133-1596500,1596567,1597414,1597989,1598273,1599140,1600872,1601633,1603485-1603487,1603499,1603605,1604128,1604188,1604413-1604414,1604416-1604417,1604421,1604442,1604700,1604717,1604720,1604726,1604755,1604794,1604802,1604824,1604836,1604844,1604902-1604903,1604911,1604925,1604933,1604947,1605059-1605060,1605064-1605065,1605068,1605071-1605073,1605075,1605123,1605188-1605189,1605191,1605197,1605444,1605633,1606132,1606142,1606144,1606514,1606526,1606528,1606551,1606554,1606564,1606598-1606599,1606656,1606658,1606662,1606744,1606840,1607085,1607572,1612407,1612810,1613339,1613872,1614611,1615348,1615351-1615352,1615356,1616338-1616339,1616613,1617586,1617688,1618138,1618151,1618153,1618226,1618641,1618653,1618662,1619068,1619358,1619413,1619769,1619774,1620602,1620909,1620912,1620928,1620930,1621275,1621635,1622931,1622937,1622942,1622946,1622959-1622960,1622963,1622987,1623007,1623368,1623373,1623377,1623379,1623381,1623398,1623402,162
 4011,1624265,1624512,1626246,1626871,1626873,1626886,1627497-1627498,1627502,1627947-1627949,1627966,1628083,1628093,1628158-1628159,1628161,1628392-1628393,1628415,1628427,1628676,1628738,1628762,1628764,1629854-1629855,1629857,1629865,1629873,1629875,1629879,1630067,1630070,1631049-1631051,1631075,1631115,1631171,1631180,1631185-1631186,1631196-1631197,1631239-1631240,1631548,1631550,1631563,1631567,1631588,1631598,1632646,1632776,1632849,1632851-1632853,1632856-1632857,1632868,1632908,1632926,1633232,1633617-1633618,1634872,1634875,1634879-1634880,1634920,1636478,1636483,1636629,1636644,1637184,1637186,1637330,1637358,1637363,1637393,1639319,1639322,1639335,1639348,1639352,1639355,1639358,1639414,1639419,1639426,1639430,1639436,1639440,1639549,1640061-1640062,1640197,1640915,1640966,1641013,1643139,1643233,1645567,1646021,1646712,1646716,1647537,1647540-1647541,1647820,1647905,1648230,1648238,1648241-1648243,1648253,1648272,1648532
 /subversion/trunk/subversion/libsvn_fs_x:1414756-1509914

Modified: subversion/trunk/subversion/libsvn_fs_x/dag.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/dag.c?rev=1649029&r1=1649028&r2=1649029&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/dag.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/dag.c Fri Jan  2 13:46:57 2015
@@ -1105,6 +1105,8 @@ svn_fs_x__dag_dup(const dag_node_t *node
   /* Only copy cached node_revision_t for immutable nodes. */
   if (node->node_revision && !svn_fs_x__dag_check_mutable(node))
     new_node->node_revision = copy_node_revision(node->node_revision, pool);
+  else
+    new_node->node_revision = NULL;
 
   new_node->node_pool = pool;
 

Modified: subversion/trunk/subversion/libsvn_fs_x/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/tree.c?rev=1649029&r1=1649028&r2=1649029&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/tree.c Fri Jan  2 13:46:57 2015
@@ -1640,22 +1640,23 @@ x_change_node_prop(svn_fs_root_t *root,
   apr_hash_t *proplist;
   svn_fs_x__txn_id_t txn_id;
   svn_boolean_t mergeinfo_mod = FALSE;
+  apr_pool_t *subpool = svn_pool_create(pool);
 
   if (! root->is_txn_root)
     return SVN_FS__NOT_TXN(root);
   txn_id = root_txn_id(root);
 
-  path = svn_fs__canonicalize_abspath(path, pool);
-  SVN_ERR(open_path(&parent_path, root, path, 0, TRUE, pool));
+  path = svn_fs__canonicalize_abspath(path, subpool);
+  SVN_ERR(open_path(&parent_path, root, path, 0, TRUE, subpool));
 
   /* Check (non-recursively) to see if path is locked; if so, check
      that we can use it. */
   if (root->txn_flags & SVN_FS_TXN_CHECK_LOCKS)
     SVN_ERR(svn_fs_x__allow_locked_operation(path, root->fs, FALSE, FALSE,
-                                             pool));
+                                             subpool));
 
-  SVN_ERR(make_path_mutable(root, parent_path, path, pool));
-  SVN_ERR(svn_fs_x__dag_get_proplist(&proplist, parent_path->node, pool));
+  SVN_ERR(make_path_mutable(root, parent_path, path, subpool));
+  SVN_ERR(svn_fs_x__dag_get_proplist(&proplist, parent_path->node, subpool));
 
   /* If there's no proplist, but we're just deleting a property, exit now. */
   if ((! proplist) && (! value))
@@ -1663,7 +1664,7 @@ x_change_node_prop(svn_fs_root_t *root,
 
   /* Now, if there's no proplist, we know we need to make one. */
   if (! proplist)
-    proplist = apr_hash_make(pool);
+    proplist = apr_hash_make(subpool);
 
   if (strcmp(name, SVN_PROP_MERGEINFO) == 0)
     {
@@ -1678,9 +1679,9 @@ x_change_node_prop(svn_fs_root_t *root,
 
       if (increment != 0)
         {
-          SVN_ERR(increment_mergeinfo_up_tree(parent_path, increment, pool));
+          SVN_ERR(increment_mergeinfo_up_tree(parent_path, increment, subpool));
           SVN_ERR(svn_fs_x__dag_set_has_mergeinfo(parent_path->node,
-                                                  (value != NULL), pool));
+                                                  (value != NULL), subpool));
         }
 
       mergeinfo_mod = TRUE;
@@ -1691,14 +1692,17 @@ x_change_node_prop(svn_fs_root_t *root,
 
   /* Overwrite the node's proplist. */
   SVN_ERR(svn_fs_x__dag_set_proplist(parent_path->node, proplist,
-                                     pool));
+                                     subpool));
 
   /* Make a record of this modification in the changes table. */
-  return add_change(root->fs, txn_id, path,
-                    svn_fs_x__dag_get_id(parent_path->node),
-                    svn_fs_path_change_modify, FALSE, TRUE, mergeinfo_mod,
-                    svn_fs_x__dag_node_kind(parent_path->node),
-                    SVN_INVALID_REVNUM, NULL, pool);
+  SVN_ERR(add_change(root->fs, txn_id, path,
+                     svn_fs_x__dag_get_id(parent_path->node),
+                     svn_fs_path_change_modify, FALSE, TRUE, mergeinfo_mod,
+                     svn_fs_x__dag_node_kind(parent_path->node),
+                     SVN_INVALID_REVNUM, NULL, subpool));
+
+  svn_pool_destroy(subpool);
+  return SVN_NO_ERROR;
 }
 
 
@@ -1716,6 +1720,7 @@ x_props_changed(svn_boolean_t *changed_p
                 apr_pool_t *pool)
 {
   dag_node_t *node1, *node2;
+  apr_pool_t *subpool = svn_pool_create(pool);
 
   /* Check that roots are in the same fs. */
   if (root1->fs != root2->fs)
@@ -1723,10 +1728,13 @@ x_props_changed(svn_boolean_t *changed_p
       (SVN_ERR_FS_GENERAL, NULL,
        _("Cannot compare property value between two different filesystems"));
 
-  SVN_ERR(get_dag(&node1, root1, path1, TRUE, pool));
-  SVN_ERR(get_dag(&node2, root2, path2, TRUE, pool));
-  return svn_fs_x__dag_things_different(changed_p, NULL, node1, node2,
-                                        strict, pool);
+  SVN_ERR(get_dag(&node1, root1, path1, TRUE, subpool));
+  SVN_ERR(get_dag(&node2, root2, path2, TRUE, subpool));
+  SVN_ERR(svn_fs_x__dag_things_different(changed_p, NULL, node1, node2,
+                                         strict, subpool));
+  svn_pool_destroy(subpool);
+
+  return SVN_NO_ERROR;
 }
 
 
@@ -2512,17 +2520,18 @@ x_make_dir(svn_fs_root_t *root,
   parent_path_t *parent_path;
   dag_node_t *sub_dir;
   svn_fs_x__txn_id_t txn_id = root_txn_id(root);
+  apr_pool_t *subpool = svn_pool_create(pool);
 
-  path = svn_fs__canonicalize_abspath(path, pool);
+  path = svn_fs__canonicalize_abspath(path, subpool);
   SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
-                    TRUE, pool));
+                    TRUE, subpool));
 
   /* Check (recursively) to see if some lock is 'reserving' a path at
      that location, or even some child-path; if so, check that we can
      use it. */
   if (root->txn_flags & SVN_FS_TXN_CHECK_LOCKS)
     SVN_ERR(svn_fs_x__allow_locked_operation(path, root->fs, TRUE, FALSE,
-                                             pool));
+                                             subpool));
 
   /* If there's already a sub-directory by that name, complain.  This
      also catches the case of trying to make a subdirectory named `/'.  */
@@ -2530,23 +2539,26 @@ x_make_dir(svn_fs_root_t *root,
     return SVN_FS__ALREADY_EXISTS(root, path);
 
   /* Create the subdirectory.  */
-  SVN_ERR(make_path_mutable(root, parent_path->parent, path, pool));
+  SVN_ERR(make_path_mutable(root, parent_path->parent, path, subpool));
   SVN_ERR(svn_fs_x__dag_make_dir(&sub_dir,
                                  parent_path->parent->node,
                                  parent_path_path(parent_path->parent,
-                                                  pool),
+                                                  subpool),
                                  parent_path->entry,
                                  txn_id,
-                                 pool));
+                                 subpool));
 
   /* Add this directory to the path cache. */
-  SVN_ERR(dag_node_cache_set(root, parent_path_path(parent_path, pool),
-                             sub_dir, pool));
+  SVN_ERR(dag_node_cache_set(root, parent_path_path(parent_path, subpool),
+                             sub_dir, subpool));
 
   /* Make a record of this modification in the changes table. */
-  return add_change(root->fs, txn_id, path, svn_fs_x__dag_get_id(sub_dir),
-                    svn_fs_path_change_add, FALSE, FALSE, FALSE,
-                    svn_node_dir, SVN_INVALID_REVNUM, NULL, pool);
+  SVN_ERR(add_change(root->fs, txn_id, path, svn_fs_x__dag_get_id(sub_dir),
+                     svn_fs_path_change_add, FALSE, FALSE, FALSE,
+                     svn_node_dir, SVN_INVALID_REVNUM, NULL, subpool));
+
+  svn_pool_destroy(subpool);
+  return SVN_NO_ERROR;
 }
 
 
@@ -2561,13 +2573,14 @@ x_delete_node(svn_fs_root_t *root,
   svn_fs_x__txn_id_t txn_id;
   apr_int64_t mergeinfo_count = 0;
   svn_node_kind_t kind;
+  apr_pool_t *subpool = svn_pool_create(pool);
 
   if (! root->is_txn_root)
     return SVN_FS__NOT_TXN(root);
 
   txn_id = root_txn_id(root);
-  path = svn_fs__canonicalize_abspath(path, pool);
-  SVN_ERR(open_path(&parent_path, root, path, 0, TRUE, pool));
+  path = svn_fs__canonicalize_abspath(path, subpool);
+  SVN_ERR(open_path(&parent_path, root, path, 0, TRUE, subpool));
   kind = svn_fs_x__dag_node_kind(parent_path->node);
 
   /* We can't remove the root of the filesystem.  */
@@ -2579,31 +2592,35 @@ x_delete_node(svn_fs_root_t *root,
      check that we can use the existing lock(s). */
   if (root->txn_flags & SVN_FS_TXN_CHECK_LOCKS)
     SVN_ERR(svn_fs_x__allow_locked_operation(path, root->fs, TRUE, FALSE,
-                                             pool));
+                                             subpool));
 
   /* Make the parent directory mutable, and do the deletion.  */
-  SVN_ERR(make_path_mutable(root, parent_path->parent, path, pool));
+  SVN_ERR(make_path_mutable(root, parent_path->parent, path, subpool));
   SVN_ERR(svn_fs_x__dag_get_mergeinfo_count(&mergeinfo_count,
                                             parent_path->node));
   SVN_ERR(svn_fs_x__dag_delete(parent_path->parent->node,
                                parent_path->entry,
-                               txn_id, pool));
+                               txn_id, subpool));
 
   /* Remove this node and any children from the path cache. */
-  SVN_ERR(dag_node_cache_invalidate(root, parent_path_path(parent_path, pool),
-                                    pool));
+  SVN_ERR(dag_node_cache_invalidate(root, parent_path_path(parent_path,
+                                                           subpool),
+                                    subpool));
 
   /* Update mergeinfo counts for parents */
   if (mergeinfo_count > 0)
     SVN_ERR(increment_mergeinfo_up_tree(parent_path->parent,
                                         -mergeinfo_count,
-                                        pool));
+                                        subpool));
 
   /* Make a record of this modification in the changes table. */
-  return add_change(root->fs, txn_id, path,
-                    svn_fs_x__dag_get_id(parent_path->node),
-                    svn_fs_path_change_delete, FALSE, FALSE, FALSE, kind,
-                    SVN_INVALID_REVNUM, NULL, pool);
+  SVN_ERR(add_change(root->fs, txn_id, path,
+                     svn_fs_x__dag_get_id(parent_path->node),
+                     svn_fs_path_change_delete, FALSE, FALSE, FALSE, kind,
+                     SVN_INVALID_REVNUM, NULL, subpool));
+
+  svn_pool_destroy(subpool);
+  return SVN_NO_ERROR;
 }
 
 
@@ -2767,13 +2784,17 @@ x_copy(svn_fs_root_t *from_root,
        const char *to_path,
        apr_pool_t *pool)
 {
-  return svn_error_trace(copy_helper(from_root,
-                                     svn_fs__canonicalize_abspath(from_path,
-                                                                  pool),
-                                     to_root,
-                                     svn_fs__canonicalize_abspath(to_path,
-                                                                  pool),
-                                     TRUE, pool));
+  apr_pool_t *subpool = svn_pool_create(pool);
+
+  SVN_ERR(copy_helper(from_root,
+                      svn_fs__canonicalize_abspath(from_path, subpool),
+                      to_root, 
+                      svn_fs__canonicalize_abspath(to_path, subpool),
+                      TRUE, subpool));
+
+  svn_pool_destroy(subpool);
+
+  return SVN_NO_ERROR;
 }
 
 
@@ -2786,12 +2807,19 @@ x_revision_link(svn_fs_root_t *from_root
                 const char *path,
                 apr_pool_t *pool)
 {
+  apr_pool_t *subpool;
+
   if (! to_root->is_txn_root)
     return SVN_FS__NOT_TXN(to_root);
 
-  path = svn_fs__canonicalize_abspath(path, pool);
-  return svn_error_trace(copy_helper(from_root, path, to_root, path,
-                                     FALSE, pool));
+  subpool = svn_pool_create(pool);
+
+  path = svn_fs__canonicalize_abspath(path, subpool);
+  SVN_ERR(copy_helper(from_root, path, to_root, path, FALSE, subpool));
+
+  svn_pool_destroy(subpool);
+
+  return SVN_NO_ERROR;
 }
 
 
@@ -2830,10 +2858,11 @@ x_make_file(svn_fs_root_t *root,
   parent_path_t *parent_path;
   dag_node_t *child;
   svn_fs_x__txn_id_t txn_id = root_txn_id(root);
+  apr_pool_t *subpool = svn_pool_create(pool);
 
-  path = svn_fs__canonicalize_abspath(path, pool);
+  path = svn_fs__canonicalize_abspath(path, subpool);
   SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
-                    TRUE, pool));
+                    TRUE, subpool));
 
   /* If there's already a file by that name, complain.
      This also catches the case of trying to make a file named `/'.  */
@@ -2844,26 +2873,29 @@ x_make_file(svn_fs_root_t *root,
      that we can use it. */
   if (root->txn_flags & SVN_FS_TXN_CHECK_LOCKS)
     SVN_ERR(svn_fs_x__allow_locked_operation(path, root->fs, FALSE, FALSE,
-                                             pool));
+                                             subpool));
 
   /* Create the file.  */
-  SVN_ERR(make_path_mutable(root, parent_path->parent, path, pool));
+  SVN_ERR(make_path_mutable(root, parent_path->parent, path, subpool));
   SVN_ERR(svn_fs_x__dag_make_file(&child,
                                   parent_path->parent->node,
                                   parent_path_path(parent_path->parent,
-                                                   pool),
+                                                   subpool),
                                   parent_path->entry,
                                   txn_id,
-                                  pool));
+                                  subpool));
 
   /* Add this file to the path cache. */
-  SVN_ERR(dag_node_cache_set(root, parent_path_path(parent_path, pool), child,
-                             pool));
+  SVN_ERR(dag_node_cache_set(root, parent_path_path(parent_path, subpool),
+                             child, subpool));
 
   /* Make a record of this modification in the changes table. */
-  return add_change(root->fs, txn_id, path, svn_fs_x__dag_get_id(child),
-                    svn_fs_path_change_add, TRUE, FALSE, FALSE,
-                    svn_node_file, SVN_INVALID_REVNUM, NULL, pool);
+  SVN_ERR(add_change(root->fs, txn_id, path, svn_fs_x__dag_get_id(child),
+                     svn_fs_path_change_add, TRUE, FALSE, FALSE,
+                     svn_node_file, SVN_INVALID_REVNUM, NULL, subpool));
+
+  svn_pool_destroy(subpool);
+  return SVN_NO_ERROR;
 }
 
 
@@ -3024,7 +3056,7 @@ apply_textdelta(void *baton, apr_pool_t
 
   /* Now, make sure this path is mutable. */
   SVN_ERR(make_path_mutable(tb->root, parent_path, tb->path, pool));
-  tb->node = parent_path->node;
+  tb->node = svn_fs_x__dag_dup(parent_path->node, tb->pool);
 
   if (tb->base_checksum)
     {
@@ -3079,6 +3111,7 @@ x_apply_textdelta(svn_txdelta_window_han
                   svn_checksum_t *result_checksum,
                   apr_pool_t *pool)
 {
+  apr_pool_t *subpool = svn_pool_create(pool);
   txdelta_baton_t *tb = apr_pcalloc(pool, sizeof(*tb));
 
   tb->root = root;
@@ -3087,10 +3120,12 @@ x_apply_textdelta(svn_txdelta_window_han
   tb->base_checksum = svn_checksum_dup(base_checksum, pool);
   tb->result_checksum = svn_checksum_dup(result_checksum, pool);
 
-  SVN_ERR(apply_textdelta(tb, pool));
+  SVN_ERR(apply_textdelta(tb, subpool));
 
   *contents_p = window_consumer;
   *contents_baton_p = tb;
+
+  svn_pool_destroy(subpool);
   return SVN_NO_ERROR;
 }
 
@@ -3183,11 +3218,11 @@ apply_text(void *baton, apr_pool_t *pool
 
   /* Now, make sure this path is mutable. */
   SVN_ERR(make_path_mutable(tb->root, parent_path, tb->path, pool));
-  tb->node = parent_path->node;
+  tb->node = svn_fs_x__dag_dup(parent_path->node, tb->pool);
 
   /* Make a writable stream for replacing the file's text. */
   SVN_ERR(svn_fs_x__dag_get_edit_stream(&(tb->file_stream), tb->node,
-                                         tb->pool));
+                                        tb->pool));
 
   /* Create a 'returnable' stream which writes to the file_stream. */
   tb->stream = svn_stream_create(tb, tb->pool);
@@ -3212,6 +3247,7 @@ x_apply_text(svn_stream_t **contents_p,
              svn_checksum_t *result_checksum,
              apr_pool_t *pool)
 {
+  apr_pool_t *subpool = svn_pool_create(pool);
   struct text_baton_t *tb = apr_pcalloc(pool, sizeof(*tb));
 
   tb->root = root;
@@ -3219,9 +3255,11 @@ x_apply_text(svn_stream_t **contents_p,
   tb->pool = pool;
   tb->result_checksum = svn_checksum_dup(result_checksum, pool);
 
-  SVN_ERR(apply_text(tb, pool));
+  SVN_ERR(apply_text(tb, subpool));
 
   *contents_p = tb->stream;
+
+  svn_pool_destroy(subpool);
   return SVN_NO_ERROR;
 }
 
@@ -3241,6 +3279,7 @@ x_contents_changed(svn_boolean_t *change
                    apr_pool_t *pool)
 {
   dag_node_t *node1, *node2;
+  apr_pool_t *subpool = svn_pool_create(pool);
 
   /* Check that roots are in the same fs. */
   if (root1->fs != root2->fs)
@@ -3252,21 +3291,24 @@ x_contents_changed(svn_boolean_t *change
   {
     svn_node_kind_t kind;
 
-    SVN_ERR(svn_fs_x__check_path(&kind, root1, path1, pool));
+    SVN_ERR(svn_fs_x__check_path(&kind, root1, path1, subpool));
     if (kind != svn_node_file)
       return svn_error_createf
         (SVN_ERR_FS_GENERAL, NULL, _("'%s' is not a file"), path1);
 
-    SVN_ERR(svn_fs_x__check_path(&kind, root2, path2, pool));
+    SVN_ERR(svn_fs_x__check_path(&kind, root2, path2, subpool));
     if (kind != svn_node_file)
       return svn_error_createf
         (SVN_ERR_FS_GENERAL, NULL, _("'%s' is not a file"), path2);
   }
 
-  SVN_ERR(get_dag(&node1, root1, path1, TRUE, pool));
-  SVN_ERR(get_dag(&node2, root2, path2, TRUE, pool));
-  return svn_fs_x__dag_things_different(NULL, changed_p, node1, node2,
-                                        strict, pool);
+  SVN_ERR(get_dag(&node1, root1, path1, TRUE, subpool));
+  SVN_ERR(get_dag(&node2, root2, path2, TRUE, subpool));
+  SVN_ERR(svn_fs_x__dag_things_different(NULL, changed_p, node1, node2,
+                                         strict, subpool));
+
+  svn_pool_destroy(subpool);
+  return SVN_NO_ERROR;
 }
 
 
@@ -3282,16 +3324,20 @@ x_get_file_delta_stream(svn_txdelta_stre
                         apr_pool_t *pool)
 {
   dag_node_t *source_node, *target_node;
+  apr_pool_t *subpool = svn_pool_create(pool);
 
   if (source_root && source_path)
-    SVN_ERR(get_dag(&source_node, source_root, source_path, TRUE, pool));
+    SVN_ERR(get_dag(&source_node, source_root, source_path, TRUE, subpool));
   else
     source_node = NULL;
-  SVN_ERR(get_dag(&target_node, target_root, target_path, TRUE, pool));
+  SVN_ERR(get_dag(&target_node, target_root, target_path, TRUE, subpool));
 
   /* Create a delta stream that turns the source into the target.  */
-  return svn_fs_x__dag_get_file_delta_stream(stream_p, source_node,
-                                             target_node, pool);
+  SVN_ERR(svn_fs_x__dag_get_file_delta_stream(stream_p, source_node,
+                                              target_node, pool));
+
+  svn_pool_destroy(subpool);
+  return SVN_NO_ERROR;
 }
 
 
@@ -3440,7 +3486,10 @@ x_node_history(svn_fs_history_t **histor
 
 /* Find the youngest copyroot for path PARENT_PATH or its parents in
    filesystem FS, and store the copyroot in *REV_P and *PATH_P.
-   Perform all allocations in POOL. */
+   Perform all allocations in POOL.
+
+   Note that *PATH_P will not be allocated in POOL but will be taken from
+   a DAG node in PARENT_PATH. */
 static svn_error_t *
 find_youngest_copyroot(svn_revnum_t *rev_p,
                        const char **path_p,
@@ -3494,36 +3543,47 @@ svn_error_t *x_closest_copy(svn_fs_root_
   svn_fs_root_t *copy_dst_root;
   dag_node_t *copy_dst_node;
   svn_boolean_t related;
+  apr_pool_t *subpool = svn_pool_create(pool);
 
   /* Initialize return values. */
   *root_p = NULL;
   *path_p = NULL;
 
-  path = svn_fs__canonicalize_abspath(path, pool);
-  SVN_ERR(open_path(&parent_path, root, path, 0, FALSE, pool));
+  path = svn_fs__canonicalize_abspath(path, subpool);
+  SVN_ERR(open_path(&parent_path, root, path, 0, FALSE, subpool));
 
   /* Find the youngest copyroot in the path of this node-rev, which
      will indicate the target of the innermost copy affecting the
      node-rev. */
   SVN_ERR(find_youngest_copyroot(&copy_dst_rev, &copy_dst_path,
-                                 fs, parent_path, pool));
+                                 fs, parent_path, subpool));
   if (copy_dst_rev == 0)  /* There are no copies affecting this node-rev. */
-    return SVN_NO_ERROR;
+    {
+      svn_pool_destroy(subpool);
+      return SVN_NO_ERROR;
+    }
 
   /* It is possible that this node was created from scratch at some
      revision between COPY_DST_REV and REV.  Make sure that PATH
      exists as of COPY_DST_REV and is related to this node-rev. */
   SVN_ERR(svn_fs_x__revision_root(&copy_dst_root, fs, copy_dst_rev, pool));
   SVN_ERR(open_path(&copy_dst_parent_path, copy_dst_root, path,
-                    open_path_node_only | open_path_allow_null, FALSE, pool));
+                    open_path_node_only | open_path_allow_null, FALSE,
+                    subpool));
   if (copy_dst_parent_path == NULL)
-    return SVN_NO_ERROR;
+    {
+      svn_pool_destroy(subpool);
+      return SVN_NO_ERROR;
+    }
 
   copy_dst_node = copy_dst_parent_path->node;
   SVN_ERR(svn_fs_x__dag_related_node(&related, copy_dst_node,
                                      parent_path->node));
   if (!related)
-    return SVN_NO_ERROR;
+    {
+      svn_pool_destroy(subpool);
+      return SVN_NO_ERROR;
+    }
 
   /* One final check must be done here.  If you copy a directory and
      create a new entity somewhere beneath that directory in the same
@@ -3545,12 +3605,17 @@ svn_error_t *x_closest_copy(svn_fs_root_
       svn_fs_x__id_t pred;
       SVN_ERR(svn_fs_x__dag_get_predecessor_id(&pred, copy_dst_node));
       if (!svn_fs_x__id_used(&pred))
-        return SVN_NO_ERROR;
+        {
+          svn_pool_destroy(subpool);
+          return SVN_NO_ERROR;
+        }
     }
 
   /* The copy destination checks out.  Return it. */
   *root_p = copy_dst_root;
-  *path_p = copy_dst_path;
+  *path_p = apr_pstrdup(pool, copy_dst_path);
+
+  svn_pool_destroy(subpool);
   return SVN_NO_ERROR;
 }
 
@@ -3759,12 +3824,14 @@ fs_history_prev(svn_fs_history_t **prev_
     }
   else
     {
+      apr_pool_t *iterpool = svn_pool_create(scratch_pool);
       prev_history = history;
 
       while (1)
         {
+          svn_pool_clear(iterpool);
           SVN_ERR(history_prev(&prev_history, prev_history, cross_copies,
-                               result_pool, scratch_pool));
+                               result_pool, iterpool));
 
           if (! prev_history)
             break;
@@ -3772,6 +3839,8 @@ fs_history_prev(svn_fs_history_t **prev_
           if (fhd->is_interesting)
             break;
         }
+
+      svn_pool_destroy(iterpool);
     }
 
   *prev_history_p = prev_history;



Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Julian Foad <ju...@btopenworld.com>.
Ivan Zhakov wrote:
> On 12 January 2015 at 18:39, Julian Foad wrote:
>> Ivan Zhakov wrote:
>>>>>> URL: http://svn.apache.org/r1649029
>>>>>> Log:
>>>>>> Sync FSX with FSFS: Merge DAG-related sub-pool introduction
>>>>>> patches r1647905, r1648243, r1648253 and r1648272 from FSFS
>>>>>> and resolve the usual text conflicts due to naming differences.
>>>>>> Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.
>> 
>>> Also, it seems that this commit has another problems:
>>> 1. It doesn't have proper log message [1].
>>> 2. It mixes different unrelated changes in one commit, while it will
>>> be much easier to review them separately.
>> 
>> There is nothing wrong here. The log message says what's merged, and 
>> that is four revisions all described as "DAG-related sub-pool 
>> introduction", which means they are all parts of the same change - not 
>> unrelated changes.
> 
> Subversion community guide is very specific about proper log messages
> and I don't understand why this commit (and other previous FSX "merge"
> commits") does not follow it.

The Community Guide doesn't document what we do for merges. For merges, we nearly always just say what was merged. In my opinion, giving a detailed log message for what's changed by a merge should be encouraged but not required. I would suggest that for a fairly simple, mechanical change like this one, it is not usually very useful.

> Quoting Subversion community guide [1] again:
> [[[
> The log message should name every affected function, variable, macro,
> makefile target, grammar rule, etc, including the names of symbols
> that are being removed in this commit. This helps people searching
> through the logs later. Don't hide names in wildcards, because the
> globbed portion may be what someone searches for later.
> ]]]

Some exceptions are listed after the quoted section. Merges are another exception, which isn't (yet) documented.

> There is very good reason that every affected identifier should be
> mentioned in log message.

Sure, but not for every kind of change. 

- Julian


> [1] 
> https://subversion.apache.org/docs/community-guide/conventions.html#log-messages


Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Branko Čibej <br...@wandisco.com>.
On 12.01.2015 16:54, Ivan Zhakov wrote:
> On 12 January 2015 at 18:39, Julian Foad <ju...@btopenworld.com> wrote:
>> Ivan Zhakov wrote:
>>>>>> URL: http://svn.apache.org/r1649029
>>>>>> Log:
>>>>>> Sync FSX with FSFS: Merge DAG-related sub-pool introduction
>>>>>> patches r1647905, r1648243, r1648253 and r1648272 from FSFS
>>>>>> and resolve the usual text conflicts due to naming differences.
>>>>>> Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.
>>> Also, it seems that this commit has another problems:
>>> 1. It doesn't have proper log message [1].
>>> 2. It mixes different unrelated changes in one commit, while it will
>>> be much easier to review them separately.
>> There is nothing wrong here. The log message says what's merged, and that is four revisions all described as "DAG-related sub-pool introduction", which means they are all parts of the same change - not unrelated changes.
>>
> Subversion community guide is very specific about proper log messages
> and I don't understand why this commit (and other previous FSX "merge"
> commits") does not follow it.
>
> Quoting Subversion community guide [1] again:
> [[[
> The log message should name every affected function, variable, macro,
> makefile target, grammar rule, etc, including the names of symbols
> that are being removed in this commit. This helps people searching
> through the logs later. Don't hide names in wildcards, because the
> globbed portion may be what someone searches for later.
> ]]]
>
> There is very good reason that every affected identifier should be
> mentioned in log message.

The point of that text is that anyone should be able to easily find the
changes just by reading the logs. When we merge stuff between branches,
we always just write which revisions were merged from where and why.
That's a clear pointer to the original logs. You're splitting hairs that
aren't even there.

Furthermore, you're trying to apply a resolved veto to code that was not
covered by it; we all agree that FSX is experimental and who's to know
what further changes will happen to those bits of code in the next
several years? Not to mention the last several days, if you want to be
pedantic about pool usage; it's all in the commit logs.

I'm really getting awfully tired of this constant sniping. Can you
please calm down a bit before you randomly accuse people for doing stuff
that's perfectly in line with our policies? Correction: ONE person;
which is, in fact, quite a bit worse.

-- Brane


Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jan 12, 2015 at 06:54:22PM +0300, Ivan Zhakov wrote:
> Subversion community guide is very specific about proper log messages
> and I don't understand why this commit (and other previous FSX "merge"
> commits") does not follow it.
> 
> Quoting Subversion community guide [1] again:
> [[[
> The log message should name every affected function, variable, macro,
> makefile target, grammar rule, etc, including the names of symbols
> that are being removed in this commit. This helps people searching
> through the logs later. Don't hide names in wildcards, because the
> globbed portion may be what someone searches for later.
> ]]]
> 
> There is very good reason that every affected identifier should be
> mentioned in log message.

We don't usually do this in merge commits.

Would you really expect this in a 'sync with trunk' merge on a feature
branch that hasn't been updated in a while?
It would be a lot of duplicated work.

The same can be said for merges between subfolders.

Of course, merge commits should name the revisions merged so their log
messages cound be found easily.

Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 12 January 2015 at 18:39, Julian Foad <ju...@btopenworld.com> wrote:
> Ivan Zhakov wrote:
>>>>> URL: http://svn.apache.org/r1649029
>>>>> Log:
>>>>> Sync FSX with FSFS: Merge DAG-related sub-pool introduction
>>>>> patches r1647905, r1648243, r1648253 and r1648272 from FSFS
>>>>> and resolve the usual text conflicts due to naming differences.
>>>>> Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.
>
>> Also, it seems that this commit has another problems:
>> 1. It doesn't have proper log message [1].
>> 2. It mixes different unrelated changes in one commit, while it will
>> be much easier to review them separately.
>
> There is nothing wrong here. The log message says what's merged, and that is four revisions all described as "DAG-related sub-pool introduction", which means they are all parts of the same change - not unrelated changes.
>

Subversion community guide is very specific about proper log messages
and I don't understand why this commit (and other previous FSX "merge"
commits") does not follow it.

Quoting Subversion community guide [1] again:
[[[
The log message should name every affected function, variable, macro,
makefile target, grammar rule, etc, including the names of symbols
that are being removed in this commit. This helps people searching
through the logs later. Don't hide names in wildcards, because the
globbed portion may be what someone searches for later.
]]]

There is very good reason that every affected identifier should be
mentioned in log message.

[1] https://subversion.apache.org/docs/community-guide/conventions.html#log-messages

-- 
Ivan Zhakov

Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Julian Foad <ju...@btopenworld.com>.
Ivan Zhakov wrote:
>>>> URL: http://svn.apache.org/r1649029
>>>> Log:
>>>> Sync FSX with FSFS: Merge DAG-related sub-pool introduction
>>>> patches r1647905, r1648243, r1648253 and r1648272 from FSFS
>>>> and resolve the usual text conflicts due to naming differences.
>>>> Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.

> Also, it seems that this commit has another problems:
> 1. It doesn't have proper log message [1].
> 2. It mixes different unrelated changes in one commit, while it will
> be much easier to review them separately.

There is nothing wrong here. The log message says what's merged, and that is four revisions all described as "DAG-related sub-pool introduction", which means they are all parts of the same change - not unrelated changes.

- Julian


Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
 5 January 2015 at 15:49, Stefan Fuhrmann <st...@wandisco.com> wrote:
> On Fri, Jan 2, 2015 at 3:21 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On 2 January 2015 at 16:46,  <st...@apache.org> wrote:
>> > Author: stefan2
>> > Date: Fri Jan  2 13:46:57 2015
>> > New Revision: 1649029
>> >
>> > URL: http://svn.apache.org/r1649029
>> > Log:
>> > Sync FSX with FSFS: Merge DAG-related sub-pool introduction
>> > patches r1647905, r1648243, r1648253 and r1648272 from FSFS
>> > and resolve the usual text conflicts due to naming differences.
>> > Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.
>> >
>> Stefan,
>>
>> You've already got veto for these revisions (r1648243 etc) when you
>> have committed them to FSFS and reverted on trunk. What is the point
>> to merge them to FSX?
>
> The veto was based upon the crash Evgeny found.
> That crash was caused by r1648238.
>
As far as I understand there were two veto reasons against r1648253:
crash and APR pools usage violation.

Also, it seems that this commit has another problems:
1. It doesn't have proper log message [1].
2. It mixes different unrelated changes in one commit, while it will
be much easier to review them separately.

Mentioning all changed identifiers in log message is important.
Quoting Subversion community guide [1]
[[[
The log message should name every affected function, variable, macro,
makefile target, grammar rule, etc, including the names of symbols
that are being removed in this commit. This helps people searching
through the logs later. Don't hide names in wildcards, because the
globbed portion may be what someone searches for later.
]]]

We use the shortened log messages for backports to release branches,
but FSX is not a branch. This code is in trunk.

[1] https://subversion.apache.org/docs/community-guide/conventions.html#log-messages

---
Ivan Zhakov

Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Jan 2, 2015 at 3:21 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:

> On 2 January 2015 at 16:46,  <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Fri Jan  2 13:46:57 2015
> > New Revision: 1649029
> >
> > URL: http://svn.apache.org/r1649029
> > Log:
> > Sync FSX with FSFS: Merge DAG-related sub-pool introduction
> > patches r1647905, r1648243, r1648253 and r1648272 from FSFS
> > and resolve the usual text conflicts due to naming differences.
> > Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.
> >
> Stefan,
>
> You've already got veto for these revisions (r1648243 etc) when you
> have committed them to FSFS and reverted on trunk. What is the point
> to merge them to FSX?
>

The veto was based upon the crash Evgeny found.
That crash was caused by r1648238.

I'm now beginning to move all FSX code towards
the two-pool paradigm. Those changes merged here
are a step into that direction.

-- Stefan^2.

Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 2 January 2015 at 16:46,  <st...@apache.org> wrote:
> Author: stefan2
> Date: Fri Jan  2 13:46:57 2015
> New Revision: 1649029
>
> URL: http://svn.apache.org/r1649029
> Log:
> Sync FSX with FSFS: Merge DAG-related sub-pool introduction
> patches r1647905, r1648243, r1648253 and r1648272 from FSFS
> and resolve the usual text conflicts due to naming differences.
> Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.
>
Stefan,

You've already got veto for these revisions (r1648243 etc) when you
have committed them to FSFS and reverted on trunk. What is the point
to merge them to FSX?

-- 
Ivan Zhakov