Re: [PATCH] t9010: svnadmin can fail even if available

Jonathan Nieder [off-list ref] writes:

If svn is built against one version of SQLite and run against
another, svnadmin (needlessly) errors out during t9010:

 <<< Started new transaction, based on original revision 1
      * adding path : branches ... done.
      * adding path : trunk ... done.
 svnadmin: Couldn't perform atomic initialization
 svnadmin: Couldn't perform atomic initialization
 svnadmin: SQLite compiled for 3.7.4, but running with 3.7.3

Work around this by putting the svn invocations into a single test
that builds a repo to compare the test-svn-fe result against.  This
test would always pass but only set the new SVNREPO test prereq if svn
succeeds; and the test using that repo gets an SVNREPO prerequisite so
it only runs with working svn installations.

This seems like the right thing to, anyway: the test script is meant
to test the version of git just built, not the installed svn.

Yes, and I understand that this will prevent the mailing list from getting
spammed by useless "bug reports" that should have been directed to distros
that packaged broken subversion, which is a plus.

But I am somewhat unhappy because I do not think we want to cater to all
the broken installations of system tools.  When tests fail because
somebody's "mkdir -p" (just a random example I picked from your patch)
does not work correctly, we would just say "Your system is broken, and
here is a nickle; get a better computer".  Why is svnadmin so special?

Also isn't the breakage not just this test, but also in all the tests that
try to run "svnadmin load"?  Shouldn't we somehow hoist this logic out of
t9010 and put it in t/lib-vcs-svn.sh or somewhere?

As far as I understand, svn interoperability bits (git-svn and vcs-svn) do
not rely on svnadmin at runtime, so a breakage in the system's svnadmin
would not be a reason to omit building and installing them.  It however
somehow feels wrong to install something we cannot even test.  I do not
have a good solution to this offhand, though.

Junio C Hamano wrote:

But I am somewhat unhappy because I do not think we want to cater to all
the broken installations of system tools.  When tests fail because
somebody's "mkdir -p" (just a random example I picked from your patch)
does not work correctly, we would just say "Your system is broken, and
here is a nickle; get a better computer".  Why is svnadmin so special?

Also isn't the breakage not just this test, but also in all the tests that
try to run "svnadmin load"?  Shouldn't we somehow hoist this logic out of
t9010 and put it in t/lib-vcs-svn.sh or somewhere?

As far as I understand, svn interoperability bits (git-svn and vcs-svn) do
not rely on svnadmin at runtime, so a breakage in the system's svnadmin
would not be a reason to omit building and installing them.

You're right, I did not do a good job of selling the patch.  I should
have said:

	Subject: t9010: svnadmin can fail even if available

	If svn is built against one version of SQLite and run against
	another, libsvn_subr needlessly errors out in operations that
	need to make a commit.

	That is clearly not a bug in git but let us consider the
	ramifications for the test suite.  git-svn uses libsvn directly
	and is probably broken by that bug; it is right for git-svn
	tests to fail.  The vcs-svn lib, on the other hand, does not use
	libsvn and the test t9010 only uses svn to check its work.  This
	points to two possible improvements:

	 - do not disable most vcs-svn tests if svn is missing.
	 - skip validation rather than failing it when svn fails.

	Bring about both by putting the svn invocations into a single
	test that builds a repo to compare the test-svn-fe result
	against.  The test will always pass but only will set the new
	SVNREPO test prereq if svn succeeds; and validation using that
	repo gets an SVNREPO prerequisite so it only runs with working
	svn installations.

	Works-around: http://bugs.debian.org/608925
	Noticed-by: A Large Angry SCM [off-list ref]
	Signed-off-by: Jonathan Nieder [off-list ref]