Re: [PATCH 00/16] Improvements, mainly to user name handling and scp.

[Michael Witten]

In-Reply-To<20151229150334.gb27...@ucc.gu.uwa.edu.au>
References: 
<20151229150334.gb27...@ucc.gu.uwa.edu.au>

On Tue, 29 Dec 2015 23:03:34 +0800, Matt Johnston wrote:

> On Mon, Dec 07, 2015 at 10:42:59PM -, Michael Witten wrote:
>> User names have hitherto been handled neither consistently nor well; this
>> series alleviates at least some of the issues.
>> 
>> Fear not the long patch series!
>> 
>> Most commits involve a fairly small number of changes; while I could have
>> consolidated these changes into fewer commits, I think the series as a whole
>> provides a better narration for what's going.
>> 
>> Besides a few small improvements along the way, the main thrust is:
>> 
>>   * Removing user-name handling from `scp' (in favor of using the
>> handling that is already present in `dropbear'/`dbclient').
>> 
>>   * Lazily looking up the current user's name.
>>   
>>   * Removing unused code.
>> 
>> Overall, 7 files were changed, with 37 insertions(+) and 158 deletions(-):
>> 
>>   cli-main.c   |   2 +-
>>   cli-runopts.c|  32 ++
>>   common-session.c |   1 +
>>   runopts.h|   2 +-
>>   scp.c| 125 
>> ++-
>>   scpmisc.c|  31 +-
>>   scpmisc.h|   2 -
>> 
>> This is the series:
>> 
>>   [01] scp: Insert comma into stderr message
>>   [02] scp: Have `fatal()' append a newline to the message
>>   [03] scp: only pass `-v' when DEBUG_TRACE is set
>>   [04] scp: `-l%s' -> `-l %s'
>>   [05] scp: const/static correctness improvements
>>   [06] scp: Introduce `get_user_name()'
>>   [07] scp: Use "unknown" when the user name cannot be retrieved
>>   [08] scp: style: simplify code by using a tertiary operator
>>   [09] scp: Use `get_user_name()' more often
>>   [10] scp: Simplify code now that the user name is never `NULL'
>>   [11] scp: Remove parsing of `[user@]host'
>>   [12] scp: Remove unused functions
>>   [13] scp: Remove `replacearg()'
>>   [14] runopts: Mark `*cli_runopts.own_user' as `const'
>>   [15] runopts: There's no reason to make a duplicate of "unknown"
>>   [16] runopts: Re-introduce the `get_user_name()' function from `scp' 
>> development
>> 
>> Lastly, for convenience in reviewing the changes, here is the overall patch:
>> 
>> [ ... ]
>
> Hi Michael,
>
> I think the general change of these patches makes sense
> (avoiding failure when a local user doesn't exist) but it
> needs to be more minimal. scp comes straight from OpenSSH
> with some small changes for uClinux etc. I've tried to
> avoid additional changes since it really needs updating to a
> more recent OpenSSH - the more changes, the larger that work
> is. Upstreaming it might be an option.
>
> Cheers,
> Matt

Well, even if I go through the trouble of interfacing with this
other community (OpenSSH), and thereby successfully introduce
these changes 'upstream', it sounds like there's still going to
be a lot of work to get the result merged 'downstream' as
part of the project that I actually [and, yet, only marginally]
care about (Dropbear SSH).

This seems like it's going to be a waste of my time, and I've
already spent a not-insignificant amount of time on this.

My instinct is that we should view Dropbear's copies as being
what they, in fact,  are: forks with their own history and
purpose.

Yet, if that view is undesirable, then let's get this set
straight now and for the rest of time:

  * Let's identify all the files that are viewed as being merely
patched copies of 'upstream' files.

  * Let's segregate them obviously, and make clear notes (for the
benefit of others) that all substantive changes should be made
upstream.
  
  + Most portably, these files could be refactored into a
subdirectory named 'upstream' or 'third-party'.

It would be possible to account for DropBear-specific patches
either explicitly (by applying them at build-time), or implicitly
(by placing them in a specific branch that is merged with the main
line of history via a non-fastforward merge commit).

  + Alternatively, these files could be included in at least one git
submodule (or the mercurial equivalent); this submodule
could then track the patches explicitly, and provide a focus for
discussing Dropbear-specific patches and upstream synchronization.
  
  * Let's bring those 'upstream' copies up-to-date.

Only after this effort, will it make sense to me to try to introduce
my patches upstream in order to have them merged downstream.

Sincerely,
Michael Witten

Re: [PATCH 00/16] Improvements, mainly to user name handling and scp.

[Michael Witten]

On Tue, 29 Dec 2015 23:03:34 +0800, Matt Johnston wrote:

> On Mon, Dec 07, 2015 at 10:42:59PM -, Michael Witten wrote:
>> User names have hitherto been handled neither consistently nor well; this
>> series alleviates at least some of the issues.
>> 
>> Fear not the long patch series!
>> 
>> Most commits involve a fairly small number of changes; while I could have
>> consolidated these changes into fewer commits, I think the series as a whole
>> provides a better narration for what's going.
>> 
>> Besides a few small improvements along the way, the main thrust is:
>> 
>>   * Removing user-name handling from `scp' (in favor of using the
>> handling that is already present in `dropbear'/`dbclient').
>> 
>>   * Lazily looking up the current user's name.
>>   
>>   * Removing unused code.
>> 
>> Overall, 7 files were changed, with 37 insertions(+) and 158 deletions(-):
>> 
>>   cli-main.c   |   2 +-
>>   cli-runopts.c|  32 ++
>>   common-session.c |   1 +
>>   runopts.h|   2 +-
>>   scp.c| 125 
>> ++-
>>   scpmisc.c|  31 +-
>>   scpmisc.h|   2 -
>> 
>> This is the series:
>> 
>>   [01] scp: Insert comma into stderr message
>>   [02] scp: Have `fatal()' append a newline to the message
>>   [03] scp: only pass `-v' when DEBUG_TRACE is set
>>   [04] scp: `-l%s' -> `-l %s'
>>   [05] scp: const/static correctness improvements
>>   [06] scp: Introduce `get_user_name()'
>>   [07] scp: Use "unknown" when the user name cannot be retrieved
>>   [08] scp: style: simplify code by using a tertiary operator
>>   [09] scp: Use `get_user_name()' more often
>>   [10] scp: Simplify code now that the user name is never `NULL'
>>   [11] scp: Remove parsing of `[user@]host'
>>   [12] scp: Remove unused functions
>>   [13] scp: Remove `replacearg()'
>>   [14] runopts: Mark `*cli_runopts.own_user' as `const'
>>   [15] runopts: There's no reason to make a duplicate of "unknown"
>>   [16] runopts: Re-introduce the `get_user_name()' function from `scp' 
>> development
>> 
>> Lastly, for convenience in reviewing the changes, here is the overall patch:
>> 
>> [ ... ]
>
> Hi Michael,
>
> I think the general change of these patches makes sense
> (avoiding failure when a local user doesn't exist) but it
> needs to be more minimal. scp comes straight from OpenSSH
> with some small changes for uClinux etc. I've tried to
> avoid additional changes since it really needs updating to a
> more recent OpenSSH - the more changes, the larger that work
> is. Upstreaming it might be an option.
>
> Cheers,
> Matt

Well, even if I go through the trouble of interfacing with this
other community (OpenSSH), and thereby successfully introduce
these changes 'upstream', it sounds like there's still going to
be a lot of work to get the result merged 'downstream' as
part of the project that I actually [and, yet, only marginally]
care about (Dropbear SSH).

This seems like it's going to be a waste of my time, and I've
already spent a not-insignificant amount of time on this.

My instinct is that we should view Dropbear's copies as being
what they, in fact,  are: forks with their own history and
purpose.

Yet, if that view is undesirable, then let's get this set
straight now and for the rest of time:

  * Let's identify all the files that are viewed as being merely
patched copies of 'upstream' files.

  * Let's segregate them obviously, and make clear notes (for the
benefit of others) that all substantive changes should be made
upstream.
  
  + Most portably, these files could be refactored into a
subdirectory named 'upstream' or 'third-party'.

It would be possible to account for DropBear-specific patches
either explicitly (by applying them at build-time), or implicitly
(by placing them in a specific branch that is merged with the main
line of history via a non-fastforward merge commit).

  + Alternatively, these files could be included in at least one git
submodule (or the mercurial equivalent); this submodule
could then track the patches explicitly, and provide a focus for
discussing Dropbear-specific patches and upstream synchronization.
  
  * Let's bring those 'upstream' copies up-to-date.

Only after this effort, will it make sense to me to try to introduce
my patches upstream in order to have them merged downstream.

Sincerely,
Michael Witten

Re: [PATCH 00/16] Improvements, mainly to user name handling and scp.

[Michael Witten]

On Mon, 11 Jan 2016 16:37:09 -, Michael Witten wrote:

> Message-ID: <2666123da20b47a589b819f37bd4cc54-mfwit...@gmail.com>
> In-Reply-To<20151229150334.gb27...@ucc.gu.uwa.edu.au>
> References: 
> <20151229150334.gb27...@ucc.gu.uwa.edu.au>

Woops!

I apologize for having sent a malformed email; you'll notice that the
header 'In-Reply-To' is missing the colon. Please ignore that email,
the one with the following Message-ID:

  <2666123da20b47a589b819f37bd4cc54-mfwit...@gmail.com>

Instead, make replies to the email that has the following Message-ID:

  

Terribly sorry,
Michael Witten