[PATCH] Re: verify_as_revision_before_current_plus_plus() on a production repo?

Daniel Shahaf wrote:
>> should our policy be to provide administrative choice?
>
> A knob sounds good to me [...]

And Bert said "+1" on IRC too.

The attached patch is a basic implementation. The fsfs.conf option is
spelled

   [debug]
   verify-before-commit = true

I put it under [debug] because that section name already exists and
holds a "pack after commit" option (not documented in the file template
comments) which seems conceptually similar.

What sort of testing do you think this needs? A first level could be to
test that the verification code runs when the option is explicitly
enabled and doesn't when disabled, and preferably test the default state
too. A second level could be to test that the verification actually
picks up some (injected) corruption and prevents the commit and exits
cleanly. This second level is not directly related to making the feature
optional, but it is related to providing the feature at all in a
release-mode build.

> If we enable this outside maintainer mode, we should look into the
> points raised upthread about ensuring that 'verify' is read-only.
> We don't have to fix everything, but we should at least add roadsign
> comments in the code to reduce the chance that future changes to verify
> will interact badly with the unusual callsite in verify_as_revision_before_current_plus_plus().

Ack.

> By the way, are you planning to enable verify_as_revision_before_current_plus_plus()
> in alpha3? I would suggest enabling it unconditionally before alpha3,
> then reverting it [or adding the knob under discussion] at some point
> before branching 1.10.x.

Sounds good. To keep track of the need to change the default back again,
would a bold coloured box in the release notes would be a suitable place
for such a note?

> [We should probably give that function a shorter name... ;)]

Could do. verify_before_commit?

- Julian