Re: [lmi] [lmi-commits] master 802eb23 6/6: Refactor for simplicity

[Vadim Zeitlin]

On Tue, 6 Dec 2016 22:25:12 +0000 Greg Chicares <address@hidden> wrote:

GC> ...so I took the "BOOST" comment as suggesting that a general library
GC> facility would be preferable, and figured that you weren't aware that
GC> lmi already had a function to compare files.

 You're right about this, if I knew about files_are_identical() I would
have tried to extend it instead of writing a whole new function -- but I
indeed didn't.

GC> I guess invoking cmp here would break your flow (so the issue is
GC> not how many seconds it takes to invoke it, because that's tiny).

 Yes, it's just a separate manual step to perform. It's not a big deal, of
course, but as I said, I was surprised that you decided to spend time on
changing something that diminished, rather than enhanced, the usability,
even if slightly.

GC> I certainly wasn't trying to make your work more cumbersome--I only
GC> intended to merge duplicate code. I'd welcome a patch that restores
GC> the lost functionality while avoiding duplication (because all code,
GC> even in system tests, must be maintained, at a nonzero cost). I see
GC> three ways that might be done:
GC> 
GC> (1) Add code to files_are_identical() to report the first byte
GC> that differs iff they aren't identical. Or, probably better...

 I'd prefer this approach, it seems simple to me and this function is only
used in the unit tests, so why not give more information about the failure
in it?

GC> (2) Report the same thing with a new function, called cmp() perhaps.

 This could be done too, but why bother with another function when having
just one would seem to work just fine?

GC> Or, best of all...
GC> 
GC> (3) Do something like this:
GC>   if(!files_are_identical(file0, file1))
GC>     {
GC>     std::ostringstream oss;
GC>     oss << "cmp " << file0 << ' ' << file1;
GC>     system_command(oss.str());
GC>     }
GC> That seems ideal.

 Sorry for being contrarian once again but it doesn't seem so to me. This
could result in a huge amount of output if there was an extra offset in one
of the files, for example.

GC> The only problem is that it won't work for me in a cross-development
GC> chroot where system_command() uses CreateProcess() to invoke an
GC> msw-native 'cmp' that I don't have. I'm sure there's a good solution to
GC> that, which would also fix all the remaining unit- test problems in the
GC> chroot:
GC> 
GC> Running authenticity_test:
GC>   Result of 'md5sum --version':
GC> 
GC> ** uncaught exception: std::runtime_error: Exit code 12345 from command 
'md5sum  --version'.
GC> 
GC> Running system_command_test:
GC> 
GC> ** uncaught exception: std::runtime_error: Exit code 12345 from command 
'grep --quiet abc eraseme'.
GC> 
GC> If you could figure out a good solution for that,

 If I understand the problem correctly, i.e. these errors happen when
running these programs under Wine, then I think the answer to this is in
the Wine FAQ:

https://wiki.winehq.org/FAQ#How_do_I_launch_native_applications_from_a_Windows_application.3F

AFAIR, I also could just symlink native programs into a directory in the
PATH under Wine "drive_c" and this worked too, at least for running them
from cmd.exe running under Wine -- but I don't see why running them from
lmi would be any different.

 Of course, there is always the trivial solution consisting in installing
the missing programs into Wine, either as native ones (preferred) or even
as parts of Cygwin -- which is getting somewhat metacircular, but should
still work, AFAICS.

 But all this seems suspiciously simple to me, so I wonder if I missed the
point entirely?
VZ