bugs/does not understand "yes" as boolean true value for autoenableyohhttp://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/git-annexikiwiki2020-06-17T01:18:32Zcomment 1http://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_1_2f13e023228fa4938dd314e040a8ec10/kyle2020-06-17T01:18:32Z2020-04-07T15:30:54Z
<p>As of 7.20200202.7, initremote and enableremote will refuse to accept
unknown values:</p>
<pre><code>$ git annex initremote d type=directory directory=/$PWD../dir/ autoenable=yes encryption=none
initremote d
git-annex: Bad value for autoenable (expected true or false)
failed
git-annex: initremote: 1 failed
</code></pre>
<p>Here's your <a href="https://git-annex.branchable.com/todo/assure_correct_names___40__and_values__41___for_special_remotes_parameters/">todo</a> related to those changes.</p>
<p>Rejecting invalid values isn't the more liberal casting of boolean
values that you're asking for here, but the above behavior would stop
an initremote caller from misconfiguring the remote, preventing cases
like the report you link to.</p>
<p>Assuming that taking multiple values for a boolean is a good thing in
general, it seems like it'd be problematic to switch special remote
parameters over to this behavior. The stored config is used when
enabling it in other repos, so an older version of git-annex would
see, say, autoenable=yes and not treat it as autoenable=true. To
avoid that, I suppose git-annex could map the value onto the currently
accepted form when storing the config, but I'd guess at that point
support for multiple ways to say true isn't really worth the
trade-off.</p>
comment 1http://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_2_01985a25dbdcfe487e28aaeb5823fded/joey2020-06-17T01:18:32Z2020-04-07T17:28:52Z
<p>Yes, true, git-annex has some inconsistent parsing of these.</p>
<p>It's certianly at least a bug worth fixing that the Git.Config parser only
supports "true" and not "1" and "on". (And IIRC git-config also has a
special case for true boolean settings that has only the setting in the
config file, without a value -- which git-annex also may not parse.)</p>
<p>On special remotes, Kyle's right. Same reasoning also applies to
booleans set by git-annex config. I don't immediately see any good way to
add a translation layer to either when things are set, it would be an ugly
addition in both cases.</p>
<p>I don't feel git-annex necessarily needs to mimic git here in accepting
all those things. It's well known that not all of git's UI choices are good,
and git-annex does not mimic all of them, eg git has some very nasty
positional --switch parsing.</p>
<p>But readonly and autoenable using true/false while all other special remote
configs uses yes/no is not good UI either.</p>
comment 3http://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_3_136e937bdeda3dec080b49d4755d1fba/yarikoptic2020-06-17T01:18:32Z2020-04-07T18:25:46Z
<blockquote><p>I don't feel git-annex necessarily needs to mimic git here in accepting all those things. It's well known that not all of git's UI choices are good, and git-annex does not mimic all of them, eg git has some very nasty positional --switch parsing.</p></blockquote>
<p>I would not over-generalize this issue to all possible <code>git</code> UI bad decisions... Talking about configuration boolean options, given the:</p>
<blockquote><p>But readonly and autoenable using true/false while all other special remote configs uses yes/no is not good UI either.</p></blockquote>
<p>IMHO staying inline with git would bring desired consistent (even if undesired flexibility, which I do not like either) handling.</p>
<p>FWIW, in DataLad, in reading the bool config values <a href="https://github.com/datalad/datalad/blob/master/datalad/config.py#L120">we have</a></p>
<pre><code class="python"> def anything2bool(val):
if hasattr(val, 'lower'):
val = val.lower()
if val in {"off", "no", "false", "0"} or not bool(val):
return False
elif val in {"on", "yes", "true", True} \
or (hasattr(val, 'isdigit') and val.isdigit() and int(val)) \
or isinstance(val, int) and val:
return True
else:
raise TypeError(
"Got value %s which could not be interpreted as a boolean"
% repr(val))
</code></pre>
<p>so for better or for worse, any non-0 positive integer is considered <code>True</code> as well... it seems we are ALMOST inline with git:</p>
<pre><code class="shell">$> for v in true yes 1 200 '' false no 0 -1 0.00 0.1 string; do echo -n "$v="; git -c "sec.blah=$v" config --bool sec.blah; python -c "from datalad.config import anything2bool; print(anything2bool('$v'))"; echo; done
true=true
True
yes=true
True
1=true
True
200=true
True
=false
False
false=false
False
no=false
False
0=false
False
-1=true
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 132, in anything2bool
% repr(val))
TypeError: Got value '-1' which could not be interpreted as a boolean
0.00=fatal: bad numeric config value '0.00' for 'sec.blah': invalid unit
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 132, in anything2bool
% repr(val))
TypeError: Got value '0.00' which could not be interpreted as a boolean
0.1=fatal: bad numeric config value '0.1' for 'sec.blah': invalid unit
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 132, in anything2bool
% repr(val))
TypeError: Got value '0.1' which could not be interpreted as a boolean
string=fatal: bad numeric config value 'string' for 'sec.blah': invalid unit
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 132, in anything2bool
% repr(val))
TypeError: Got value 'string' which could not be interpreted as a boolean
</code></pre>
<p>so only the negative integer is tolerated by git but not by us. I will send a quick PR</p>
comment 4http://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_4_aa4ae6db181b1261a36ad06f223ec28f/joey2020-06-17T01:18:32Z2020-04-07T18:48:18Z
<p>Surely values that git does not even document can be considered to have
undefined behavior.</p>
comment 5http://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_5_ea204e2190c5a34cfcb3dc0e0710a4e3/yarikoptic2020-06-17T01:18:32Z2020-04-08T01:08:10Z
<blockquote><p>Surely values that git does not even document can be considered to have undefined behavior.</p></blockquote>
<p>yeah. integers which aren't 0 or 1 are indeed a dark (undocumented) zone. Might be worth contributing to git and making them documented (probably better to not change behavior at this point).</p>
comment 6http://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_6_2f998fab7d88a9ade53354d1820248ea/joey2020-06-17T01:18:32Z2020-04-13T17:45:51Z
<p>I've made the git-config parser support all the ways of indicating true and
false that git documents, including the strange distinction between an empty
config setting (false) and no config setting (true). I'm not going to
chase undocumented git behavior that could vary between versions of git.</p>
<p>That also made it support values like "on" that are set by
git-annex config --set. It was always possible to set such things.
Older git-annex will display an error message if used with a repo
that has such values set. That seems acceptable.</p>
<p>I have not done anything about making special remote configs consistent.</p>
comment 7http://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_7_bfbc9a9fbae3715fea0f4a11549ec8a9/joey2020-06-17T01:18:32Z2020-04-23T16:39:16Z
<p>Looking closer at the affected special remote configs:</p>
<ul>
<li><p>readonly=true is only used by external special remotes. It used to be
stored in the remote.log, but after examining it, I have changed that.</p>
<p>So, it could be safely changed to also accept readonly=yes with no
back-compat issues.</p></li>
<li><p>When git-annex init is run, any autoenable= that is stored never results
in an error no matter what the value is; if it doesn't recognise it, it
assumes false. (And more generally, no value already stored in remote.log
causes a parse error, even if it cannot be parsed.) So autoenable=yes
would only result in older git-annex init not enabling the remote. Which
I guess is not a hard problem for a user to recover from, the remote
clearly won't be enabled when they try to use it. Two arguments in favor
of not worrying about backwards compatability for autoenable=yes:</p>
<ol>
<li><p>When autoenable=true was added originally, we obviously didn't worry
that older git-annex's would ignore that config and not autoenable.</p></li>
<li><p>If the user does anything other than git-annex init, autoenabling
doesn't happen currently, which seems like rather more foot shooting
potential for ending up w/o a remote autoenabled, but I'm not seeing
much complaint about that. Though IIRC there might be a todo about it.</p></li>
</ol>
</li>
</ul>
<p>So I'm leaning a bit toward it would be ok to accept yes/no as well as
true/false for those two, without worrying about translation to values old
git-annex will understand.</p>
<p>But.. Allowing yes/no for these two while still also allowing true/false
does not remove the special case, it just buries it a bit. All the other
initremote booleans are yes/no and not true/false. And changing them
<em>would</em> need a translation layer. I don't feel that bit of uniformity is
worth that added complexity.</p>