On Fri, 2016-01-15 at 13:51 +0100, Thomas Gummerer wrote:
> On 01/14, David Turner wrote:
> > + if (requested_ref_storage_backend)
> > + ref_storage_backend =
> > requested_ref_storage_backend;
> > + if (strcmp(ref_storage_backend, "files")) {
> > + git_config_set("extensions.refStorage",
> > ref_storage_backend);
> > + git_config_set("core.repositoryformatversion",
> > ref_storage_backend);
> > +#ifdef USE_LIBLMDB
> > + register_ref_storage_backend(&refs_be_lmdb);
>
> refs_be_lmdb is not yet defined at this point in the patch series.
> It
> doesn't break anything, because USE_LIBLMDB doesn't leak through the
> makefile yet, but I still think it would make more sense to have the
> ifdef in 19/20.
Thanks, fixed.
> > +#endif
> > + set_ref_storage_backend(ref_storage_backend);
>
> More importantly, there is no check whether setting the ref storage
> backend succeeds. If a user accidentally sets an invalid value for
> the ref backend, a broken repository will be created without even
> warning the user.
>
> While the repository will still work fine at this point in the
> series,
> git will die with an invalid ref backend in the config after 19/20.
> It can be fixed by setting extensions.refStorage to files in the
> config, because the backend is initialized to the default files
> backend below when the ref backend cannot be set.
>
> I think it would be best to die() here, if setting the ref backend
> doesn't succeed, and clean up the files that were already written,
> instead of leaving the user with a broken repository.
I've rearranged this code.
> > +test_expect_success 'init with bogus storage backend fails' '
> > +
> > + (
> > + mkdir again2 &&
> > + cd again2 &&
> > + git init --ref-storage=test >out2 2>err2
> > + )
> > +'
>
> I noticed the above mainly because of this test, which doesn't seem
> to
> test what it claims to test. It only seems to test that git init
> succeeds, and writes something to out2 and err2, it's missing the
> checks that the contents are actually what they're supposed to be.
Fixed, thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html