[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] A test for "Can't get entries" error

From: Daniel Shahaf <danielsh_at_apache.org>
Date: Tue, 20 Nov 2018 07:29:05 +0000

Dmitry Pavlenko wrote on Mon, Nov 19, 2018 at 17:05:16 +0300:
> Hello Subversion community!
> I've run into an error: when performing 2 specially constructed updates one after another
> within the same session, SVN fails with
>
> $ ./ra-test 15
> svn_tests: E160016: Can't get entries of non-directory
> XFAIL: ra-test 15: check that there's no "Can't get entries" error
>

That error code is SVN_ERR_FS_NOT_DIRECTORY.

> error. I believe these updates constructed that way are valid, so the problem is
> somewhere in FSFS code. It's also interesting that if these updates are run
> separately (e.g. by adding "if (FALSE)" to one or another), they succeed.
>

Could you please clarify whether the bug reproduces under other backends (FSX and BDB)?

> +++ subversion/tests/libsvn_ra/ra-test.c (working copy)
> @@ -1784,7 +1784,113 @@ commit_locked_file(const svn_test_opts_t *opts, ap
> +cant_get_entries_of_non_directory(const svn_test_opts_t *opts, apr_pool_t *pool)
> +{
> + svn_ra_session_t *session;
> + const svn_delta_editor_t *editor;
> + void *edit_baton;
> + const svn_ra_reporter3_t *reporter;
> + void *report_baton;
>
> + SVN_ERR(make_and_open_repos(&session,
> + "cant_get_entries_of_non_directory", opts,
> + pool));
> +
> + {
> + SVN_ERR(svn_ra_get_commit_editor3(session, &editor, &edit_baton,
> + apr_hash_make(pool), NULL,
> + NULL, NULL, FALSE, pool));
> +
> + SVN_ERR(editor->open_root(edit_baton, 0, pool, &root_baton));
> + }

You make all commits using the same EDITOR. Is that allowed? Should
you make the 'editor' variable block-scoped and call svn_ra_get_commit_editor3()
anew in each block?

> + {
> + SVN_ERR(editor->open_root(edit_baton, 1, pool, &root_baton));

> + }

> + {
> + SVN_ERR(svn_ra_do_update3(session, &reporter, &report_baton,
> + 3, "", svn_depth_infinity, TRUE, FALSE,
> + svn_delta_default_editor(pool), NULL,
> + pool, pool));
> + SVN_ERR(reporter->set_path(report_baton, "", 3, svn_depth_infinity, FALSE,
> + NULL, pool));
> + SVN_ERR(reporter->set_path(report_baton, "B", 2, svn_depth_infinity, FALSE,
> + NULL, pool));
> + SVN_ERR(reporter->finish_report(report_baton, pool));
> +
> +
> + }
> + {
> + SVN_ERR(svn_ra_do_update3(session, &reporter, &report_baton,
> + 4, "", svn_depth_infinity, TRUE, FALSE,
> + svn_delta_default_editor(pool), NULL,
> + pool, pool));
> + SVN_ERR(reporter->set_path(report_baton, "", 4, svn_depth_infinity, FALSE,
> + NULL, pool));
> + SVN_ERR(reporter->set_path(report_baton, "B", 3, svn_depth_infinity, FALSE,
> + NULL, pool));
> + SVN_ERR(reporter->finish_report(report_baton, pool));
> + }

I agree that this should work, and therefore that it's a bug.

Suggestions for further debugging:

- Further minimise the test. Try to have fewer add_file calls, fewer
  set_path calls, etc.. (I realise you must have minimised it a lot
  already, from whatever the original instance was, but the more the
  better.)

- Try doing the two updates in the opposite order (exchange the order of
  the two blocks).

- See if the bug happens under FSX/BDB. That would tell us where to
  look further (in libsvn_fs_fs, or in the reporter logic in
  libsvn_repos).

> + return SVN_NO_ERROR;
> +}

Style nits:

- Per-block comments would be helpful. They don't need to be detailed,
  but something like /* r1: Create 'A' and 'A/mu' */ would help skim the
  function quickly.

- The test name isn't very descriptive. I think it would be better to
  name the test after what it expects to work: doing two updates in a
  single session after a file replacement by directory.

- One line exceeds 80 columns.

Cheers,

Daniel
Received on 2018-11-20 08:29:18 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.