openssh unix dev - Nov 2015 - ssh-copy-id bugfix

On Wed, Nov 25, 2015 at 12:20 PM, Philip Hands <phil at hands.com>
wrote:
> Nico Kadel-Garcia <nkadel at gmail.com> writes:
>> What seems to be missing in the patch is a comment line, above the
>> stanza, explaining why the code uses "exec".
>
> My reading of the presence of "exec" there was:
>
>   We're assuming that the current shell may not be to our liking, so
>   there seems to be little point keeping it in memory solely so it can
>   at worst somehow get in the way of a clean exit.
>
> Does that really need a comment?  I'm not sure I can make a succinct
> explanation of what's going on for anyone that doesn't already know
what
> exec does.  Feel free to make suggestions though.
That is _precisely_ why it needs a comment. It's a selection of a
particular technology for a particular reason that someone may not
understand as important without having to dig back to a thread or bug
report like this. For example:

     # Use "exec sh -c" to ensure POSIX compliant scripting,
especially for fish and tcsh users
     [ "$DRY_RUN" ] || printf '%s\n' "$NEW_IDS" |
ssh "$@" "
     .....

Nico Kadel-Garcia wrote:
> > Does that really need a comment?
> 
> That is _precisely_ why it needs a comment. It's a selection of a
> particular technology for a particular reason that someone may not
> understand as important
Not even if they understand what the command does? (Use another shell.)

I dislike redundant comments, which this seems to be. As code change
and comments don't, those comments end up creating a lot more confusion
than they resolve.

Having clear code is far more important.

It's reasonable to require that someone reading code already knows the
tools being used.

It's not reasonable for code to educate readers on tools being used.


//Peter

On Thu, Nov 26, 2015 at 11:45 AM, Peter Stuge <peter at stuge.se>
wrote:
> Nico Kadel-Garcia wrote:
>> > Does that really need a comment?
>>
>> That is _precisely_ why it needs a comment. It's a selection of a
>> particular technology for a particular reason that someone may not
>> understand as important
>
> Not even if they understand what the command does? (Use another shell.)
>
> I dislike redundant comments, which this seems to be. As code change
> and comments don't, those comments end up creating a lot more confusion
> than they resolve.
>
> Having clear code is far more important.
>
> It's reasonable to require that someone reading code already knows the
> tools being used.
I'm familiar with the old "the code is the comments" approach to
documentation. The difficulty with it is exactly what we see here.
It's not clear, even to a reasonably intelligent bash progammer, that
the use of "exec" is to insure compatibility with fish and tcsh users.
I've used both, and in fact published the first public ports of tcsh
for SunOS, and even I'd have think carefully to deduce, from raw
principles, why my scripting in bash is written in a fairly ugly to
ensure compatibility with alternative shells. 5 years from now, I'd
probably have forgotten completely about this thread and be
hard-pressed to remember why I or someone else didn't simply quote
ordinary bash syntax there.
> It's not reasonable for code to educate readers on tools being used.
I suggest that it's very reasonable to explain particular choices in
code, especially when those choices were made for compatibility
reasons less common configurations. Failure to document such choices
leads to regression errors when someone unweaves the undocumented
compatibility code.

Been there, done that, especially had screwups with underdocumented
and subtly incompatible or inconsistent tools for editing
$HOME/.ssh/authorized_keys editing.