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 2016/04/18 11:02:07 UTC
svn commit: r1739709 - in /subversion/branches/1.9.x: ./ STATUS
subversion/libsvn_fs_fs/fs_fs.c
subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
Author: stsp
Date: Mon Apr 18 09:02:07 2016
New Revision: 1739709
URL: http://svn.apache.org/viewvc?rev=1739709&view=rev
Log:
On the 1.9.x branch, merge ^/subversion/branches/1.9.x-fsfs-rep-comparison.
* r1717876, r1717877
Fix svn_fs_contents_different on FSFS producing false negatives.
Justification:
Under rare circumstances, see issue #4554, the API may not recognize
two file contents as being different. This may result in incomplete
reports and dump files.
Notes:
r1717877 provides a test case that verifies everything still works
even with the EXPANDED_SIZE check removed.
Branch:
^/subversion/branches/1.9.x-fsfs-rep-comparison
Votes:
+1: stefan2, rhuijben, stsp
Modified:
subversion/branches/1.9.x/ (props changed)
subversion/branches/1.9.x/STATUS
subversion/branches/1.9.x/subversion/libsvn_fs_fs/fs_fs.c
subversion/branches/1.9.x/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
Propchange: subversion/branches/1.9.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Apr 18 09:02:07 2016
@@ -2,6 +2,7 @@
/subversion/branches/1.5.x-r30215:870312
/subversion/branches/1.7.x-fs-verify:1146708,1161180
/subversion/branches/1.9-cache-improvements:1678948-1679863
+/subversion/branches/1.9.x-fsfs-rep-comparison:1717869-1739708
/subversion/branches/1.9.x-r1664664:1674265-1674433
/subversion/branches/1.9.x-r1667233:1673207-1673638
/subversion/branches/1.9.x-r1700215:1701365-1703825
@@ -97,4 +98,4 @@
/subversion/branches/verify-at-commit:1462039-1462408
/subversion/branches/verify-keep-going:1439280-1546110
/subversion/branches/wc-collate-path:1402685-1480384
-/subversion/trunk:1660545-1660547,1660549-1662901,1663003,1663183-1663184,1663338,1663347,1663355,1663374,1663450,1663500,1663530,1663671,1663697,1663706,1663738,1663749,1663791,1663991,1664035,1664078,1664080,1664084-1664085,1664187,1664191,1664193,1664200,1664344,1664476,1664480-1664481,1664483,1664489-1664490,1664507,1664520-1664521,1664523,1664526-1664527,1664531-1664532,1664588,1664593-1664594,1664596,1664653,1664664,1664672,1664674,1664684,1664927,1664938-1664940,1664978,1664984,1664997,1665164,1665195,1665213,1665259,1665318,1665437-1665438,1665609,1665611-1665612,1665845,1665850,1665852,1665873,1665886,1665894,1665896,1666096,1666258,1666270,1666272,1666379,1666449,1666690,1666832,1666851,1666965,1667101,1667106-1667107,1667120,1667228,1667233-1667235,1667249-1667250,1667258,1667290,1667301,1667471,1667691-1667693,1667699-1667700,1667715,1667941,1667976,1668320,1668598-1668600,1668602-1668603,1668607-1668608,1668618,1668625,1669743,1669746,1669749,1669945,1670139,1670149,167
0152,1670329,1670337,1670347,1670353,1671164,1671388,1672295,1672311,1672372,1672404,1672511-1672512,1672578,1672728,1673044,1673062-1673063,1673065,1673153,1673170,1673172,1673197,1673202,1673204,1673228,1673282,1673445,1673691-1673692,1673746,1673785,1673803,1674015,1674032,1674170,1674301,1674305,1674308,1674339-1674340,1674406,1674415,1674455-1674456,1674475,1674487,1674522,1674580,1674626-1674627,1674785,1674891,1675771,1675774,1676526,1676535,1676538,1676555,1676564,1676570,1676665,1676667,1676769,1677003,1677191,1677267,1677440,1678147,1678149,1678494,1678571,1678734,1678742,1678745-1678746,1678755,1678839,1678846,1678894,1678950,1678963,1679166,1679169,1679228,1679230,1679240,1679287,1679864,1679866,1679909,1680242,1680264,1680495,1680705,1680819,1681317,1682714,1682854,1683071,1683126,1683135,1683290,1683303,1683311,1683378,1683387,1684034,1684077,1684322,1684325,1684344,1684412,1684940,1685034,1685085,1686175,1686239,1686478,1686541,1686543,1686554,1686557,1686802,1686888,
1686984,1687029,1687304,1687389,1687769,1687776,1687812,1688258,1688273,1688395,1689214,1689216,1689721,1689729,1691712-1691713,1691924,1691928,1692091,1692093,1692098,1692448,1692469-1692470,1692798-1692799,1693135,1693138,1693159,1693886,1694023,1694194,1694481,1694929,1695022,1695600,1695606,1695681,1696222,1696225,1696387,1696695,1697381,1697384,1697387,1697664,1697824,1697835,1697845,1697914,1697967,1698106,1698312,1700215,1700219-1700220,1700740,1700951,1701064,1701206,1701270,1701298,1701598,1701603,1701611,1701633,1701638,1701646,1701736,1701792,1701797,1701838,1701997,1702198,1702200,1702203,1702218,1702231,1702237-1702239,1702247,1702288,1702299-1702300,1702310,1702397,1702407,1702467,1702472,1702474,1702478,1702533,1702549,1702553,1702565,1702891,1702974,1702991,1703470,1703475-1703477,1703544,1703581,1703675,1703688-1703689,1703740,1704292,1704573,1704847,1705060,1705062,1705064,1705088,1705328,1705843,1706241,1706323-1706324,1706375,1706428,1706437,1706783,1706983,17069
99,1708699,1709388-1709389,1709553,1709562,1710104,1710167,1710215,1710290,1710558,1711250,1711346,1711507,1711510,1714314,1714358,1714790,1715224,1715232,1715262,1715793,1716808,1717154,1717874-1717875,1717878,1718167,1718267,1718269,1718484,1720643,1721174-1721175,1722860-1722861,1722879,1722887,1725180,1728308,1728387,1729060,1729519,1730856,1734926,1735179,1735826,1736432,1737122,1738259,1739278,1739280
+/subversion/trunk:1660545-1660547,1660549-1662901,1663003,1663183-1663184,1663338,1663347,1663355,1663374,1663450,1663500,1663530,1663671,1663697,1663706,1663738,1663749,1663791,1663991,1664035,1664078,1664080,1664084-1664085,1664187,1664191,1664193,1664200,1664344,1664476,1664480-1664481,1664483,1664489-1664490,1664507,1664520-1664521,1664523,1664526-1664527,1664531-1664532,1664588,1664593-1664594,1664596,1664653,1664664,1664672,1664674,1664684,1664927,1664938-1664940,1664978,1664984,1664997,1665164,1665195,1665213,1665259,1665318,1665437-1665438,1665609,1665611-1665612,1665845,1665850,1665852,1665873,1665886,1665894,1665896,1666096,1666258,1666270,1666272,1666379,1666449,1666690,1666832,1666851,1666965,1667101,1667106-1667107,1667120,1667228,1667233-1667235,1667249-1667250,1667258,1667290,1667301,1667471,1667691-1667693,1667699-1667700,1667715,1667941,1667976,1668320,1668598-1668600,1668602-1668603,1668607-1668608,1668618,1668625,1669743,1669746,1669749,1669945,1670139,1670149,167
0152,1670329,1670337,1670347,1670353,1671164,1671388,1672295,1672311,1672372,1672404,1672511-1672512,1672578,1672728,1673044,1673062-1673063,1673065,1673153,1673170,1673172,1673197,1673202,1673204,1673228,1673282,1673445,1673691-1673692,1673746,1673785,1673803,1674015,1674032,1674170,1674301,1674305,1674308,1674339-1674340,1674406,1674415,1674455-1674456,1674475,1674487,1674522,1674580,1674626-1674627,1674785,1674891,1675771,1675774,1676526,1676535,1676538,1676555,1676564,1676570,1676665,1676667,1676769,1677003,1677191,1677267,1677440,1678147,1678149,1678494,1678571,1678734,1678742,1678745-1678746,1678755,1678839,1678846,1678894,1678950,1678963,1679166,1679169,1679228,1679230,1679240,1679287,1679864,1679866,1679909,1680242,1680264,1680495,1680705,1680819,1681317,1682714,1682854,1683071,1683126,1683135,1683290,1683303,1683311,1683378,1683387,1684034,1684077,1684322,1684325,1684344,1684412,1684940,1685034,1685085,1686175,1686239,1686478,1686541,1686543,1686554,1686557,1686802,1686888,
1686984,1687029,1687304,1687389,1687769,1687776,1687812,1688258,1688273,1688395,1689214,1689216,1689721,1689729,1691712-1691713,1691924,1691928,1692091,1692093,1692098,1692448,1692469-1692470,1692798-1692799,1693135,1693138,1693159,1693886,1694023,1694194,1694481,1694929,1695022,1695600,1695606,1695681,1696222,1696225,1696387,1696695,1697381,1697384,1697387,1697664,1697824,1697835,1697845,1697914,1697967,1698106,1698312,1700215,1700219-1700220,1700740,1700951,1701064,1701206,1701270,1701298,1701598,1701603,1701611,1701633,1701638,1701646,1701736,1701792,1701797,1701838,1701997,1702198,1702200,1702203,1702218,1702231,1702237-1702239,1702247,1702288,1702299-1702300,1702310,1702397,1702407,1702467,1702472,1702474,1702478,1702533,1702549,1702553,1702565,1702891,1702974,1702991,1703470,1703475-1703477,1703544,1703581,1703675,1703688-1703689,1703740,1704292,1704573,1704847,1705060,1705062,1705064,1705088,1705328,1705843,1706241,1706323-1706324,1706375,1706428,1706437,1706783,1706983,17069
99,1708699,1709388-1709389,1709553,1709562,1710104,1710167,1710215,1710290,1710558,1711250,1711346,1711507,1711510,1714314,1714358,1714790,1715224,1715232,1715262,1715793,1716808,1717154,1717869,1717871,1717873-1717875,1717878,1718167,1718267,1718269,1718484,1720643,1721174-1721175,1722860-1722861,1722879,1722887,1725180,1728308,1728387,1729060,1729519,1730856,1734926,1735179,1735826,1736432,1737122,1738259,1739278,1739280
Modified: subversion/branches/1.9.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/STATUS?rev=1739709&r1=1739708&r2=1739709&view=diff
==============================================================================
--- subversion/branches/1.9.x/STATUS (original)
+++ subversion/branches/1.9.x/STATUS Mon Apr 18 09:02:07 2016
@@ -92,17 +92,3 @@ Veto-blocked changes:
Approved changes:
=================
-
- * r1717876, r1717877
- Fix svn_fs_contents_different on FSFS producing false negatives.
- Justification:
- Under rare circumstances, see issue #4554, the API may not recognize
- two file contents as being different. This may result in incomplete
- reports and dump files.
- Notes:
- r1717877 provides a test case that verifies everything still works
- even with the EXPANDED_SIZE check removed.
- Branch:
- ^/subversion/branches/1.9.x-fsfs-rep-comparison
- Votes:
- +1: stefan2, rhuijben, stsp
Modified: subversion/branches/1.9.x/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/subversion/libsvn_fs_fs/fs_fs.c?rev=1739709&r1=1739708&r2=1739709&view=diff
==============================================================================
--- subversion/branches/1.9.x/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/1.9.x/subversion/libsvn_fs_fs/fs_fs.c Mon Apr 18 09:02:07 2016
@@ -1443,8 +1443,8 @@ svn_fs_fs__file_text_rep_equal(svn_boole
svn_stream_t *contents_a, *contents_b;
representation_t *rep_a = a->data_rep;
representation_t *rep_b = b->data_rep;
- svn_boolean_t a_empty = !rep_a || rep_a->expanded_size == 0;
- svn_boolean_t b_empty = !rep_b || rep_b->expanded_size == 0;
+ svn_boolean_t a_empty = !rep_a;
+ svn_boolean_t b_empty = !rep_b;
/* This makes sure that neither rep will be NULL later on */
if (a_empty && b_empty)
@@ -1453,28 +1453,6 @@ svn_fs_fs__file_text_rep_equal(svn_boole
return SVN_NO_ERROR;
}
- if (a_empty != b_empty)
- {
- *equal = FALSE;
- return SVN_NO_ERROR;
- }
-
- /* File text representations always know their checksums - even in a txn. */
- if (memcmp(rep_a->md5_digest, rep_b->md5_digest, sizeof(rep_a->md5_digest)))
- {
- *equal = FALSE;
- return SVN_NO_ERROR;
- }
-
- /* Paranoia. Compare SHA1 checksums because that's the level of
- confidence we require for e.g. the working copy. */
- if (rep_a->has_sha1 && rep_b->has_sha1)
- {
- *equal = memcmp(rep_a->sha1_digest, rep_b->sha1_digest,
- sizeof(rep_a->sha1_digest)) == 0;
- return SVN_NO_ERROR;
- }
-
/* Same path in same rev or txn? */
if (svn_fs_fs__id_eq(a->id, b->id))
{
@@ -1482,6 +1460,30 @@ svn_fs_fs__file_text_rep_equal(svn_boole
return SVN_NO_ERROR;
}
+ /* Beware of the combination NULL rep and possibly empty rep.
+ * Due to EXPANDED_SIZE not being reliable, we can't easily detect empty
+ * reps. So, we can only take further shortcuts if both reps are given. */
+ if (!a_empty && !b_empty)
+ {
+ /* File text representations always know their checksums -
+ * even in a txn. */
+ if (memcmp(rep_a->md5_digest, rep_b->md5_digest,
+ sizeof(rep_a->md5_digest)))
+ {
+ *equal = FALSE;
+ return SVN_NO_ERROR;
+ }
+
+ /* Paranoia. Compare SHA1 checksums because that's the level of
+ confidence we require for e.g. the working copy. */
+ if (rep_a->has_sha1 && rep_b->has_sha1)
+ {
+ *equal = memcmp(rep_a->sha1_digest, rep_b->sha1_digest,
+ sizeof(rep_a->sha1_digest)) == 0;
+ return SVN_NO_ERROR;
+ }
+ }
+
SVN_ERR(svn_fs_fs__get_contents(&contents_a, fs, rep_a, TRUE,
scratch_pool));
SVN_ERR(svn_fs_fs__get_contents(&contents_b, fs, rep_b, TRUE,
Modified: subversion/branches/1.9.x/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c?rev=1739709&r1=1739708&r2=1739709&view=diff
==============================================================================
--- subversion/branches/1.9.x/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c (original)
+++ subversion/branches/1.9.x/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c Mon Apr 18 09:02:07 2016
@@ -1647,6 +1647,98 @@ delta_chain_with_plain(const svn_test_op
#undef REPO_NAME
+/* ------------------------------------------------------------------------ */
+
+#define REPO_NAME "test-repo-compare_0_length_rep"
+
+static svn_error_t *
+compare_0_length_rep(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_fs_t *fs;
+ svn_fs_txn_t *txn;
+ svn_fs_root_t *root;
+ svn_revnum_t rev;
+ int i, k;
+ apr_hash_t *fs_config;
+
+ /* Test expectations. */
+#define no_rep_file "no-rep"
+#define empty_plain_file "empty-plain"
+#define plain_file "plain"
+#define empty_delta_file "empty-delta"
+#define delta_file "delta"
+
+ enum { COUNT = 5 };
+ const char *file_names[COUNT] = { no_rep_file,
+ empty_delta_file,
+ plain_file,
+ empty_delta_file,
+ delta_file };
+
+ int equal[COUNT][COUNT] = { { 1, 1, 0, 1, 0 },
+ { 1, 1, 0, 1, 0 },
+ { 0, 0, 1, 0, 1 },
+ { 1, 1, 0, 1, 0 },
+ { 0, 0, 1, 0, 1 } };
+
+ /* Well, this club is FSFS only ... */
+ if (strcmp(opts->fs_type, "fsfs") != 0)
+ return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL, NULL);
+
+ /* We want to check that whether NULL reps, empty PLAIN reps and empty
+ * DELTA reps are all considered equal, yet different from non-empty reps.
+ *
+ * Because we can't create empty PLAIN reps with recent formats anymore,
+ * some format selection & upgrade gymnastics is needed. */
+
+ /* Create a format 1 repository.
+ * This one does not support DELTA reps, so all is PLAIN. */
+ fs_config = apr_hash_make(pool);
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_PRE_1_4_COMPATIBLE, "x");
+ SVN_ERR(svn_test__create_fs2(&fs, REPO_NAME, opts, fs_config, pool));
+
+ /* Revision 1, create 3 files:
+ * One with no rep, one with an empty rep and a non-empty one. */
+ SVN_ERR(svn_fs_begin_txn(&txn, fs, 0, pool));
+ SVN_ERR(svn_fs_txn_root(&root, txn, pool));
+ SVN_ERR(svn_fs_make_file(root, no_rep_file, pool));
+ SVN_ERR(svn_fs_make_file(root, empty_plain_file, pool));
+ SVN_ERR(svn_test__set_file_contents(root, empty_plain_file, "", pool));
+ SVN_ERR(svn_fs_make_file(root, plain_file, pool));
+ SVN_ERR(svn_test__set_file_contents(root, plain_file, "x", pool));
+ SVN_ERR(svn_fs_commit_txn(NULL, &rev, txn, pool));
+
+ /* Upgrade the file system format. */
+ SVN_ERR(svn_fs_upgrade2(REPO_NAME, NULL, NULL, NULL, NULL, pool));
+ SVN_ERR(svn_fs_open2(&fs, REPO_NAME, NULL, pool, pool));
+
+ /* Revision 2, create two more files:
+ * a file with an empty DELTA rep and a non-empty one. */
+ SVN_ERR(svn_fs_begin_txn(&txn, fs, rev, pool));
+ SVN_ERR(svn_fs_txn_root(&root, txn, pool));
+ SVN_ERR(svn_fs_make_file(root, empty_delta_file, pool));
+ SVN_ERR(svn_test__set_file_contents(root, empty_delta_file, "", pool));
+ SVN_ERR(svn_fs_make_file(root, delta_file, pool));
+ SVN_ERR(svn_test__set_file_contents(root, delta_file, "x", pool));
+ SVN_ERR(svn_fs_commit_txn(NULL, &rev, txn, pool));
+
+ /* Now compare. */
+ SVN_ERR(svn_fs_revision_root(&root, fs, rev, pool));
+ for (i = 0; i < COUNT; ++i)
+ for (k = 0; k < COUNT; ++k)
+ {
+ svn_boolean_t different;
+ SVN_ERR(svn_fs_contents_different(&different, root, file_names[i],
+ root, file_names[k], pool));
+ SVN_TEST_ASSERT(different != equal[i][k]);
+ }
+
+ return SVN_NO_ERROR;
+}
+
+#undef REPO_NAME
+
/* The test table. */
@@ -1695,6 +1787,8 @@ static struct svn_test_descriptor_t test
"rep-sharing effectiveness"),
SVN_TEST_OPTS_PASS(delta_chain_with_plain,
"delta chains starting with PLAIN, issue #4577"),
+ SVN_TEST_OPTS_PASS(compare_0_length_rep,
+ "compare empty PLAIN and non-existent reps"),
SVN_TEST_NULL
};