Re: [PATCH 16/18] fsck: support demoting errors to warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Tue, 23 Dec 2014, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> 
> >> >> > Of course you can say that! ;-) The problem these ugly messages
> >> >> > try to solve is to give the user a hint which setting to change
> >> >> > if they want to override the default behavior, though...
> >> >> 
> >> >> Ahh, OK, then dashed form would not work as a configuration
> >> >> variable names, so missingTaggerEntry would be the only usable
> >> >> option.
> >> >
> >> > Except that cunning me has made it so that both
> >> > missing-tagger-entry *and* missingTaggerEntry work...
> >> 
> >> Then the missing-tagger-entry side needs to be dropped.  The naming
> >> does not conform to the way how we name our officially supported
> >> configuration variables.
> >
> > But it does conform with the way we do our command-line parameters,
> 
> Hmmm....  What is the expected user interaction?  The way I read the
> series was ($MISSING_TAGGER stands for the "token" we choose to show):
> 
>     (1) The user runs fsck without customization, and may see a
> 	warning (or error):
> 
>         $ git fsck
>         error in tag d6602ec5194c87b0fc87103ca4d67251c76f233a: $MISSING_TAGGER
> 
>     (2) The user demotes it to warning and runs fsck again:
> 
> 	$ git config fsck.$MISSING_TAGGER warn
>         $ git fsck
>         warning: tag d6602ec5194c87b0fc87103ca4d67251c76f233a: $MISSING_TAGGER

The intended use case is actually when receive.fsckObjects = true and you
call `git push`, seeing 'remote: error: $MULTIPLE_AUTHORS: ...'.

Now, the $MULTIPLE_AUTHORS *config* setting is parsed by `git
receive-pack`, but that is not the command that needs to customize the
fsck call: it is either `git index-pack` or `git unpack-objects`. So what
`git receive-pack` does is to pass the config options as command-line
options to the called command. For consistency with the rest of Git, the
command-line options were *not* camel-cased, but lower-case,
dash-separated.

The parser I wrote actually accepts both versions, allowing me to skip the
tedious step to convert the camelCased config setting into a
lower-case-dashed version to pass to `index-pack` or `unpack-objects`,
only to be parsed by the same parser as `fsck` would use directly.

So I am rather happy with the fact that the parser handles both camelCased
and lower-case-dashed versions.

> I suspect that it would be much better if the configuration variables
> were organized the other way around, e.g.
> 
> 	$ git config fsck.warn missingTagger,someOtherKindOfError

I had something similar in an earlier version of my patch series, but it
was shot down rightfully: if you want to allow inheriting defaults from
$HOME/.gitconfig, you have to configure the severity levels individually.

(The current solution also sidesteps the problematic situation when both
fsck.warn *and* fsck.error contain, say, missingTagger.)

> Then a one-shot override would make sense and easier to give as
> command line option, e.g.
> 
> 	$ git fsck --warn=missingTagger,someOtherKindOfError

Yep, my first implementation actually used
`--strict=missing-tagger,-some-demoted-error`. But as I mentioned above,
that approach is not as flexible as the current one.

> But the proposed organization to use one variable per questionable
> event type (as opposed to one variable per severity level) would
> lead to a one-shot override of this form, e.g.
> 
> 	$ git fsck --missing-tagger=warn --some-other-kind-of-error=warn
> 
> which I think is insane to require us to support unbound number of
> dashed options.

The intended use case is actually *not* the command-line, but the config
file, in particular allowing /etc/gitconfig, $HOME/.gitconfig *and*
.git/config to customize the settings.

> Or are you saying that we allow "git config core.file-mode true"
> from the command line to set core.fileMode configuration?

I do not understand this reference. I did not suggest to change `git
config`, did I? If I did, I apologize; it was definitely *not* my
intention to change long-standing customs.

Ciao,
Dscho
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]