Dear darcs developers

Still struggling.  I'm using darcs 1.0.8 on Windows.

I'm on Windows, trying to push a patch to
	Darcs.haskell.org/ghc-fc
(a repository Simon and I have just created, with darcs get --partial
from the main HEAD).

Two problems.

1 (minor): there's an irritating (and perhaps ominous) message from ssh
(below).

2 (fatal): all goes well to begin with; then the push fails with 

darcs failed:  Patch bundle failed hash!
This probably means that the patch has been corrupted by a mailer.
The most likely culprit is CRLF newlines.

The "likely culprit" indeed seems likely.  I'm pushing from Windows to
darcs.haskell.org which runs Unix.  But the patch has never left the
repository, and has never been near a mailer.

What do I do?

Thanks

Simon

sh-2.04$ darcs push
Usage: ssh [options] host [command]
Options:
  -l user     Log in using this user name.
  -n          Redirect input from /dev/null.
  -A          Enable authentication agent forwarding.
  -a          Disable authentication agent forwarding.
  -X          Enable X11 connection forwarding.
  -x          Disable X11 connection forwarding.
  -i file     Identity for public key authentication (default:
~/.ssh/identity)
  -t          Tty; allocate a tty even if command is given.
  -T          Do not allocate a tty.
  -v          Verbose; display verbose debugging messages.
              Multiple -v increases verbosity.
  -V          Display version number only.
  -P          Don't allocate a privileged port.
  -q          Quiet; don't display any warning messages.
  -f          Fork into background after authentication.
  -e char     Set escape character; ``none'' = disable (default: ~).
  -c cipher   Select encryption algorithm: ``3des'', ``blowfish''
  -m macs     Specify MAC algorithms for protocol version 2.
  -p port     Connect to this port.  Server must be on the same port.
  -L listen-port:host:port   Forward local port to remote address
  -R listen-port:host:port   Forward remote port to local address
              These cause ssh to listen for connections on a port, and
              forward them to the other side by connecting to host:port.
  -C          Enable compression.
  -N          Do not execute a shell or command.
  -g          Allow remote hosts to connect to forwarded ports.
  -1          Force protocol version 1.
  -2          Force protocol version 2.
  -4          Use IPv4 only.
  -6          Use IPv6 only.
  -o 'option' Process the option as if it was read from a configuration
file.
  -s          Invoke command (mandatory) as SSH2 subsystem.
Pushing to "simonpj@darcs.haskell.org:/home/darcs/ghc-fc"...

Wed Jun 21 09:34:44 GMT Daylight Time 2006  simonpj@microsoft.com
  * Remove compiler from the boring list (how did it get there?)
Shall I push this patch? (1/2)  [ynWvpxqadjk], or ? for help: y

Wed Jul  5 10:44:20 GMT Daylight Time 2006  simonpj@microsoft.com
  * Massive patch for the first months work adding System FC to GHC

  This is (sadly) all done in one patch to avoid Darcs bugs.
  It's not complete work... more FC stuff to come.  A compiler
  using just this patch will fail dismally.
Shall I push this patch? (2/2)  [ynWvpxqadjk], or ? for help: y

darcs failed:  Patch bundle failed hash!
This probably means that the patch has been corrupted by a mailer.
The most likely culprit is CRLF newlines.

sh-2.04$

On Thu, Jul 06, 2006 at 01:48:34PM +0000, simonpj wrote:
> Two problems.
> 
> 1 (minor): there's an irritating (and perhaps ominous) message from ssh
> (below).

I suspect this may relate to the "ControlMaster" features Eric
implemented, which makes darcs more efficient over openssh, but
requires trickery to determine if it works.  In this case, I think the
trickery means running ssh with openssh-specific flags, and seeing if
it fails.  It shouldn't print out an error message, though.

> 2 (fatal): all goes well to begin with; then the push fails with 
> 
> darcs failed:  Patch bundle failed hash!
> This probably means that the patch has been corrupted by a mailer.
> The most likely culprit is CRLF newlines.
> 
> The "likely culprit" indeed seems likely.  I'm pushing from Windows to
> darcs.haskell.org which runs Unix.  But the patch has never left the
> repository, and has never been near a mailer.

This is not at all good.  It's worth running a darcs check --partial
on the repository you're pushing from.

Could you try running:

darcs send -o outfilename

then (somehow) copy "outfilename" to the unix server and there run
darcs apply outfilename.  This way you can manually check with md5sum
that the patch bundle wasn't modified in transit.  I suppose you could
name the patch file something like "foo.exe" to be absolutely certain
that windows (or your ftp client, or scp or whatever) doesn't try to
modify the line endings.

It's really odd that ssh would mess up your patch file, but that
sounds like what's happening.  Let me know the result of the above,
okay?
-- 
David Roundy

| This is not at all good.  It's worth running a darcs check --partial
| on the repository you're pushing from.

sh-2.04$ pwd
/c/darcs/fc-branch-2
sh-2.04$ darcs check --partial
Applying patch 237 of 237... done.
The repository is consistent!
sh-2.04$

| 
| Could you try running:
| 
| darcs send -o outfilename
| 
| then (somehow) copy "outfilename" to the unix server and there run
| darcs apply outfilename. 

Yes, I did that and it worked fine. (I called the file 'big-patch'.)  So that's pushed my local changes to the server, albeit in a clumsy way.

After doing so I tried a 'darcs pull' from my local machine (/c/darcs/fc-branch-2), and indeed it said

scp: /home/darcs/ghc-fc/.git/HEAD: No such file or directory
scp: /home/darcs/ghc-fc/_darcs/format: No such file or directory
scp: /home/darcs/ghc-fc/.git/darcs-format: No such file or directory
Pulling from "simonpj@darcs.haskell.org:/home/darcs/ghc-fc"...
No remote changes to pull in!

Which is as expected (scp messages aside)

Simon

...
> The repository is consistent!

Thanks!

> | Could you try running:
> | 
> | darcs send -o outfilename
> | 
> | then (somehow) copy "outfilename" to the unix server and there run
> | darcs apply outfilename. 
> 
> Yes, I did that and it worked fine. (I called the file 'big-patch'.)
> So that's pushed my local changes to the server, albeit in a clumsy
> way.

Okay, that's (relatively) good news.  So neither repo is corrupt, and
the trouble is just that somehow the patch got modified in transit.
Which makes it a windows/ssh bug of some sort, presumably.  Which also
makes it one that's most likely hard to debug remotely... Assuming we
still want to figure out what went wrong--we could simply be thankful
that the hash checked prevented corruption, and hope it doesn't happen
again.  If you wanted to debug this, you could push to a temporary
repo on the server without the big patch, and try to recover the patch
file that was failing the consistency check, which might give a hint
what the trouble is.  It's stored as a temporary file named something
like darcsXge5GS (or whatever).  I believe it's deleted rather
quickly, so you might need to modify darcs on the server to not delete
its temporaries (or to store this one in a non-temporary location),
which might be more work than is necesary.  It's also possible that
you could grab the file before it's deleted, but that sounds a bit
tedious.  Are you up for debugging this transport issue? Or just
hoping to go on with your ghc hacking?
-- 
David Roundy

On Thu, Jul 06, 2006 at 13:48:34 +0000, simonpj wrote:
> 1 (minor): there's an irritating (and perhaps ominous) message from ssh
> (below).
> 
> sh-2.04$ darcs push
> Usage: ssh [options] host [command]

The minor problem is a one recurring for Windows users, one which I am
investigating right now.  The error message is harmless as far as I can
tell, but I think I have an idea for improving the darcs code so you no
longer get it.

The idea consists of parsing the ssh help output to see if the control
master option exists, rather than tossing it a control master command so
that it complains.

On Thu, Jul 06, 2006 at 05:21:46PM +0000, Eric Kow wrote:
> The minor problem is a one recurring for Windows users, one which I am
> investigating right now.  The error message is harmless as far as I can
> tell, but I think I have an idea for improving the darcs code so you no
> longer get it.
> 
> The idea consists of parsing the ssh help output to see if the control
> master option exists, rather than tossing it a control master command so
> that it complains.

It seems like we really ought not to be printing stderr to the console
on windows when we call ssh, but I've no idea how to fix this.  There
were also git-relate errors where we look for nonexistent files, that
shouldn't be visible to the user, since we're explicitly looking just
to see if they exist.  But perhaps that's something we can't easily
avoid, I've no idea.  Again, perhaps it's something that'll be "fixed"
if we switch to using System.Process (which would perhaps require
configure script hacking?).
-- 
David Roundy

On Thu, Jul 06, 2006 at 17:38:11 +0000, David Roundy wrote:
> It seems like we really ought not to be printing stderr to the console
> on windows when we call ssh, but I've no idea how to fix this.  There
> were also git-relate errors where we look for nonexistent files, that
> shouldn't be visible to the user, since we're explicitly looking just
> to see if they exist.  But perhaps that's something we can't easily
> avoid, I've no idea.  Again, perhaps it's something that'll be "fixed"
> if we switch to using System.Process (which would perhaps require
> configure script hacking?).

Looking at Exec.lhs, I notice that under Windows, our exec command
detects the "/dev/null" argument and does nothing with it.  Perhaps a
better, though maybe not-so-clean, idea is to use withTemp on some junk
file.

That being said, it would be nice to use as much "official" stuff as
possible, System.Process and eventually FPS, so maybe improving Exec
shouldn't be a priority.  In general, requiring GHC 6.4.1 instead of GHC
6.2.2 would make a bunch of stuff cleaner (cf Workarounds), but I
suppose we don't do that because many distributions still ship with it?

On Thu, Jul 06, 2006 at 06:54:04PM +0000, Eric Kow wrote:
> On Thu, Jul 06, 2006 at 17:38:11 +0000, David Roundy wrote:
> > It seems like we really ought not to be printing stderr to the console
> > on windows when we call ssh, but I've no idea how to fix this.  There
> > were also git-relate errors where we look for nonexistent files, that
> > shouldn't be visible to the user, since we're explicitly looking just
> > to see if they exist.  But perhaps that's something we can't easily
> > avoid, I've no idea.  Again, perhaps it's something that'll be "fixed"
> > if we switch to using System.Process (which would perhaps require
> > configure script hacking?).
> 
> Looking at Exec.lhs, I notice that under Windows, our exec command
> detects the "/dev/null" argument and does nothing with it.  Perhaps a
> better, though maybe not-so-clean, idea is to use withTemp on some junk
> file.

Hmmm.  That's not a bad idea.

> That being said, it would be nice to use as much "official" stuff as
> possible, System.Process and eventually FPS, so maybe improving Exec
> shouldn't be a priority.  In general, requiring GHC 6.4.1 instead of GHC
> 6.2.2 would make a bunch of stuff cleaner (cf Workarounds), but I
> suppose we don't do that because many distributions still ship with it?

My traditional policy has been that a compiler isn't obsolete until
it's no longer shipped with debian stable.  Which would make ghc 6.1.1
obsolete in December (crossing fingers).  Conversely, I'd say that
when a compiler is no longer shipped by debian, it's obsolete.  Which
is to say, debian has a slow release cycle, but is also important (at
least to me).

With regard to the value of another workaround, it's quite possible
(likely even?) that System.Process will introduce its own bugs, and
that we'll want to keep the old code around for a while, even after we
start working with System.Process.
-- 
David Roundy

On 06 July 2006 16:53, David Roundy wrote:

> David Roundy <droundy@darcs.net> added the comment:
> 
> ...
>> The repository is consistent!
> 
> Thanks!
> 
>>> Could you try running:
>>> 
>>> darcs send -o outfilename
>>> 
>>> then (somehow) copy "outfilename" to the unix server and there run
>>> darcs apply outfilename.
>> 
>> Yes, I did that and it worked fine. (I called the file 'big-patch'.)
>> So that's pushed my local changes to the server, albeit in a clumsy
>> way.
> 
> Okay, that's (relatively) good news.  So neither repo is corrupt, and
> the trouble is just that somehow the patch got modified in transit.
> Which makes it a windows/ssh bug of some sort, presumably.  Which also
> makes it one that's most likely hard to debug remotely... Assuming we
> still want to figure out what went wrong--we could simply be thankful
> that the hash checked prevented corruption, and hope it doesn't happen
> again.  If you wanted to debug this, you could push to a temporary
> repo on the server without the big patch, and try to recover the patch
> file that was failing the consistency check, which might give a hint
> what the trouble is.  It's stored as a temporary file named something
> like darcsXge5GS (or whatever).  I believe it's deleted rather
> quickly, so you might need to modify darcs on the server to not delete
> its temporaries (or to store this one in a non-temporary location),
> which might be more work than is necesary.  It's also possible that
> you could grab the file before it's deleted, but that sounds a bit
> tedious.  Are you up for debugging this transport issue? Or just
> hoping to go on with your ghc hacking?

FWIW, I've also seen this same problem several times.  I always work
around it using 'darcs send --output' and then taking the patch bundle
to a Unix system to push it.

So at least this means it isn't something specific to simonpj's personal
setup, but it could conceivably be something specific to our location.

I'll see if I can work with Simon to get hold of the temporary patch
file.  It'd be nice to track this one down.

Cheers,
	Simon

On 06 July 2006 16:53, David Roundy wrote:

> David Roundy <droundy@darcs.net> added the comment:
> 
> ...
>> The repository is consistent!
> 
> Thanks!
> 
>>> Could you try running:
>>> 
>>> darcs send -o outfilename
>>> 
>>> then (somehow) copy "outfilename" to the unix server and there run
>>> darcs apply outfilename.
>> 
>> Yes, I did that and it worked fine. (I called the file 'big-patch'.)
>> So that's pushed my local changes to the server, albeit in a clumsy
>> way.
> 
> Okay, that's (relatively) good news.  So neither repo is corrupt, and
> the trouble is just that somehow the patch got modified in transit.
> Which makes it a windows/ssh bug of some sort, presumably.  Which also
> makes it one that's most likely hard to debug remotely... Assuming we
> still want to figure out what went wrong--we could simply be thankful
> that the hash checked prevented corruption, and hope it doesn't happen
> again.  If you wanted to debug this, you could push to a temporary
> repo on the server without the big patch, and try to recover the patch
> file that was failing the consistency check, which might give a hint
> what the trouble is.  It's stored as a temporary file named something
> like darcsXge5GS (or whatever).  I believe it's deleted rather
> quickly, so you might need to modify darcs on the server to not delete
> its temporaries (or to store this one in a non-temporary location),
> which might be more work than is necesary.  It's also possible that
> you could grab the file before it's deleted, but that sounds a bit
> tedious.  Are you up for debugging this transport issue? Or just
> hoping to go on with your ghc hacking?

I tried to catch the patch on the server using a shell loop before it
got deleted, but no joy.  Perhaps I'm looking in the wrong place - I
tried the root of the repository, and also $HOME.  Maybe it just gets
deleted too quickly.

Using a recompiled darcs is a bit problematic, because the server is
darcs.haskell.org :(

Cheers,
	Simon

On Fri, Jul 07, 2006 at 11:47:09AM +0100, Simon Marlow wrote:
> I tried to catch the patch on the server using a shell loop before it
> got deleted, but no joy.  Perhaps I'm looking in the wrong place - I
> tried the root of the repository, and also $HOME.  Maybe it just gets
> deleted too quickly.
> 
> Using a recompiled darcs is a bit problematic, because the server is
> darcs.haskell.org :(

How about a recompiled darcs on the client, which doesn't actually
call darcs on the server, perhaps? Or alternatively, which calls a
darcs executable with a different name.  Either of these could be done
by editing the RemoteApply.lhs file.  I'm attaching a patch to do the
former as an example.
-- 
David Roundy

Attachments

• test_patch

My former patch failed to compile due to a warning, so here's a fixed
(and now tested) version.  (And yes, I know you could easily have
fixed this yourself...)
-- 
David Roundy

Attachments

• test_patch

On 07 July 2006 15:20, David Roundy wrote:

> David Roundy <droundy@darcs.net> added the comment:
> 
> My former patch failed to compile due to a warning, so here's a fixed
> (and now tested) version.  (And yes, I know you could easily have
> fixed this yourself...)

There's only one minor problem, I've never actually managed to compile
darcs on Windows, I always just download a binary.

So I tried with this patch.  Unfortunately I don't seem to have created
a useful binary.  First of all, the SSH ControlPath stuff doesn't work
at all for me here, so I tried disabling it.  But my darcs still can't
push:

Invalid multiplex command.
Pushing to "simonmar@darcs.haskell.org:/home/darcs/ghc-fc-test"...
We have the following patches to push:
Fri Jul  7 18:07:32 ST0 2006  kevind@bu.edu
  * newtype deriving dicts, compiling at least
Fri Jul  7 15:40:40 ST0 2006  simonpj@microsoft.com
  * ..and a bit more
Fri Jul  7 15:37:55 ST0 2006  simonpj@microsoft.com
  * More on newtype deriving
Fri Jul  7 14:26:44 ST0 2006  kevind@bu.edu
  * towards newtype deriving dicts
Fri Jul  7 11:11:48 ST0 2006  kevind@bu.edu
  * newtype fixes, coercions for non-recursive newtypes now optional
Fri Jul  7 10:45:15 ST0 2006  simonpj@microsoft.com
  * Partial changes for derived newtype instances
Thu Jul  6 17:51:02 ST0 2006  simonpj@microsoft.com
  * Tiny comment change (darcs test only)
Thu Jul  6 14:55:27 ST0 2006  simonpj@microsoft.com
  * Resolve conflict in MkExternalCore
Wed Jul  5 10:44:20 ST0 2006  simonpj@microsoft.com
  * Massive patch for the first months work adding System FC to GHC

  This is (sadly) all done in one patch to avoid Darcs bugs.
  It's not complete work... more FC stuff to come.  A compiler
  using just this patch will fail dismally.

Wed Jul  5 10:44:20 ST0 2006  simonpj@microsoft.com
  * Massive patch for the first months work adding System FC to GHC

  This is (sadly) all done in one patch to avoid Darcs bugs.
  It's not complete work... more FC stuff to come.  A compiler
  using just this patch will fail dismally.
Shall I push this patch? (1/9)  [ynWvpxqadjk], or ? for help: y

Thu Jul  6 14:55:27 ST0 2006  simonpj@microsoft.com
  * Resolve conflict in MkExternalCore
Shall I push this patch? (2/9)  [ynWvpxqadjk], or ? for help: d
Pseudo-terminal will not be allocated because stdin is not a terminal.
Exit request sent.

Any ideas?

Cheers,
	Simon

On 10 July 2006 14:44, Simon Marlow wrote:

> Pseudo-terminal will not be allocated because stdin is not a terminal.
> Exit request sent.

Never mind, I thought this was an error message but in fact darcs had
done its thing and saved the patch on the remote machine.

And indeed it turns out that the remote version of the patch had the ^Ms
stripped out.

I investigated a little further, and it seems that darcs sends the patch
bundle using something like

  $ ssh darcs.haskell.org 'darcs apply ...' <patch_bundle

that is, the patch is sent in plain text over the SSH connection.
Apparently, this isn't a reliable way to send binary data on Windows:
SSH strips ^Ms on the way.  (at least, the cygwin SSH I'm using does).
I tried it by hand and verified the result.  Also, scp sends the file
reliably (as you would expect).  I don't see a flag or option to SSH to
disable this behaviour.

So maybe we should use scp instead of ssh to send the patch bundle?

Cheers,
	Simon

PS. Simon: you might want to remove the ^Ms from Coercion.lhs.

On Mon, Jul 10, 2006 at 03:46:45PM +0100, Simon Marlow wrote:
> I investigated a little further, and it seems that darcs sends the patch
> bundle using something like
> 
>   $ ssh darcs.haskell.org 'darcs apply ...' <patch_bundle
> 
> that is, the patch is sent in plain text over the SSH connection.
> Apparently, this isn't a reliable way to send binary data on Windows:
> SSH strips ^Ms on the way.  (at least, the cygwin SSH I'm using does).
> I tried it by hand and verified the result.  Also, scp sends the file
> reliably (as you would expect).  I don't see a flag or option to SSH to
> disable this behaviour.

Hmmm.  I had been under the impression that darcs fails entirely when
using the cygwin SSH, so this is news to me! We definitely would like
darcs to work with as many ssh's as possible... it's unfortunate that
they do this business (stripping out ^M).

> So maybe we should use scp instead of ssh to send the patch bundle?

Ugh.  We could do that, but then we'd have a sort of file-leaky error
condition, where if darcs doesn't exist on the server you leave a
patch bundle there.

There's also the difficulty of finding a suitable temporary filename
to create the patch bundle on the server.  I suspect we'd need to
create the file in the darcs repository itself (perhaps under
_darcs?), since we'd have no other place to try putting it.  Perhaps
if we used its sha1 hash to create the filename, then we'd be safe in
the instance of two simultaneous pushes.

I suppose we'd also have to add a flag to apply to tell it to delete
the patch bundle after running.  :(
-- 
David Roundy

On 10 July 2006 19:21, David Roundy wrote:

> David Roundy <droundy@darcs.net> added the comment:
> 
> On Mon, Jul 10, 2006 at 03:46:45PM +0100, Simon Marlow wrote:
>> I investigated a little further, and it seems that darcs sends the
>> patch bundle using something like 
>> 
>>   $ ssh darcs.haskell.org 'darcs apply ...' <patch_bundle
>> 
>> that is, the patch is sent in plain text over the SSH connection.
>> Apparently, this isn't a reliable way to send binary data on Windows:
>> SSH strips ^Ms on the way.  (at least, the cygwin SSH I'm using
>> does). I tried it by hand and verified the result.  Also, scp sends
>> the file reliably (as you would expect).  I don't see a flag or
>> option to SSH to disable this behaviour.
> 
> Hmmm.  I had been under the impression that darcs fails entirely when
> using the cygwin SSH, so this is news to me!

Indeed, we have to jump through some hoops to use cygwin or MSYS SSH.  I
have a little wrapper program that translates the command line args
(actually a Haskell program).  This was easier (for me) than trying to
use putty or the other methods on the wiki.  Maybe one day I'll write it
up.

> We definitely would like
> darcs to work with as many ssh's as possible... it's unfortunate that
> they do this business (stripping out ^M).

I have investigated some more, and it gets even weirder.  I don't
believe this is anything to do with SSH now.

If I do this:

  $ ssh darcs.haskell.org 'cat >tmp' <testfile

then the ^Ms are stripped from testfile.  However, if I do this:

  $ cat testfile | ssh darcs.haskell.org 'cat >tmp'

Then the file is transferred unchanged.

The current directory is mounted in binary mode on cygwin.

So I suspected something to do with filename redirection, which is
handled by the shell.  I therefore changed shells, from zsh to bash, and
tried again:

  $ ssh darcs.haskell.org 'cat >tmp' <testfile

This time it works!  The file is transferred intact.  However, I know
that Simon PJ had this problem, and he doesn't use zsh under cygwin, he
uses bash under MSYS.  So I tried that - and the file is again
transferred intact.  So now I'm mystified again.  I don't know 

  (a) why zsh is apparently opening the redirection in text mode,
  (b) what is going wrong in Simon PJ's environment

(Simon isn't in today, I'll investigate more when he's back).

At least we might have a possible workaround: change your shell.  Darcs
could use System.Process to bypass the shell for a more robust
workaround.

Cheers,
	Simon

On Tue, Jul 11, 2006 at 09:44:46AM +0100, Simon Marlow wrote:
> At least we might have a possible workaround: change your shell.
> Darcs could use System.Process to bypass the shell for a more robust
> workaround.

I agree.  System.Process does sound like a far less scary solution.
Running shells is just asking for trouble (except when the user
explicitly is invited to run shell code, as in the test suite or
posthooks).
-- 
David Roundy

We could base64 encode the patch bundle when streaming it via
ssh. That would protect it from about everything I can think of.
But it requires some way to turn it off if the remote darcs
doesn't support it, and an old remote darcs won't give any
helpful error message.

On Thu, Jul 13, 2006 at 08:56:57PM +0000, Zooko wrote:
> I'm willing to (continue) to do work to support Cygwin users.

Hi Zooko, I'm writing this to add you to the nosy list on bug 218,
which involves corruption of patches containing ^M when they are
pushed over openssh.  Not that I'm expecting you to fix it, but that
hopefully you'll at least be willing and/or able to test fixes.

And thanks for your work to help support Windows users!

David

P.S. I'm also including everyone on the "supporting cygwin ssh" bug,
figuring they presumably have some interest in this bug concerning
cygwin ssh.  If you're not interested, please just remove yourself,
and I apologize.

On 11 July 2006 09:45, Simon Marlow wrote:

> I have investigated some more, and it gets even weirder.  I don't
> believe this is anything to do with SSH now.
> 
> If I do this:
> 
>   $ ssh darcs.haskell.org 'cat >tmp' <testfile
> 
> then the ^Ms are stripped from testfile.  However, if I do this:
> 
>   $ cat testfile | ssh darcs.haskell.org 'cat >tmp'
> 
> Then the file is transferred unchanged.
> 
> The current directory is mounted in binary mode on cygwin.
> 
> So I suspected something to do with filename redirection, which is
> handled by the shell.  I therefore changed shells, from zsh to bash,
> and tried again:
> 
>   $ ssh darcs.haskell.org 'cat >tmp' <testfile
> 
> This time it works!  The file is transferred intact.  However, I know
> that Simon PJ had this problem, and he doesn't use zsh under cygwin,
> he uses bash under MSYS.  So I tried that - and the file is again
> transferred intact.  So now I'm mystified again.  I don't know
> 
>   (a) why zsh is apparently opening the redirection in text mode,
>   (b) what is going wrong in Simon PJ's environment
> 
> (Simon isn't in today, I'll investigate more when he's back).

Further investigations:

  - cmd.exe exhibits the same problem, ie. it strips ^M from
    a file redirected to stdin with <.

  - darcs of course uses cmd.exe when invoking ssh, which is
    why we all have this problem.  My zsh experiments
    above were a red herring.

The fix is not to use file redirection with cmd.exe (aka System.system
in Haskell).  The only alternative seems to be to use System.Process.

Cheers,
	Simon

On Thu, Jul 27, 2006 at 08:29:41AM +0000, Simon Marlow wrote:
> The fix is not to use file redirection with cmd.exe (aka System.system
> in Haskell).  The only alternative seems to be to use System.Process.

Thanks for tracking this down! Any chance you'd be willing to code up
a replacement using System.Process for Exec.exec in the darcs sources?
I don't imagine it'll be too hard for one of us to do, but (a) I
imagine you know System.Process better than anyone, and (b) you'll be
able to test if it solves your problem.

I think it's all right at this stage to simply switch to
System.Process, as it was introduced with ghc 6.4, right? I'll later
add a configure check to be sure it exists, and give a better error
message in case someone tries to compile darcs with an older ghc.

(If you've not got time, I or someone else can do this, but I imagine
it'll take much less time than you've already spent tracking this
down.  I hope that doesn't sound like I'm adding insult to injury or
vice versa...)
-- 
David Roundy

David Roundy wrote:
> David Roundy <droundy@darcs.net> added the comment:
>
> On Thu, Jul 27, 2006 at 08:29:41AM +0000, Simon Marlow wrote:
>> The fix is not to use file redirection with cmd.exe (aka
>> System.system in Haskell).  The only alternative seems to be to use
>> System.Process.
>
> Thanks for tracking this down! Any chance you'd be willing to code up
> a replacement using System.Process for Exec.exec in the darcs sources?
> I don't imagine it'll be too hard for one of us to do, but (a) I
> imagine you know System.Process better than anyone, and (b) you'll be
> able to test if it solves your problem.
>
> I think it's all right at this stage to simply switch to
> System.Process, as it was introduced with ghc 6.4, right? I'll later
> add a configure check to be sure it exists, and give a better error
> message in case someone tries to compile darcs with an older ghc.
>
> (If you've not got time, I or someone else can do this, but I imagine
> it'll take much less time than you've already spent tracking this
> down.  I hope that doesn't sound like I'm adding insult to injury or
> vice versa...)

I converted darcs' Exec.exec to use System.Process, but it doesn't fix this problem.  There's something deeper going on: the ^Ms still get stripped when invoking ssh via runProcess without an intervening cmd.exe.  I've confirmed this with a simple test program.

This is a cygwin session.  m.txt is a file containing the string "test" followed by a CR-LF sequence:

/cygdrive/c/scratch > od -x m.txt
0000000 6574 7473 0a0d
0000006
/cygdrive/c/scratch > wc -c m.txt
6 m.txt
/cygdrive/c/scratch > ssh darcs.haskell.org wc -c <m.txt
6
/cygdrive/c/scratch > cat exec.hs
import System.Process
import System.IO

main = do
  h <- openBinaryFile "m.txt" ReadMode
  p <- runProcess "ssh" ["darcs.haskell.org", "wc", "-c"]
        Nothing Nothing (Just h) Nothing Nothing
  waitForProcess p
/cygdrive/c/scratch > ghc --make exec.hs
[1 of 1] Compiling Main             ( exec.hs, exec.o )
Linking exec.exe ...
/cygdrive/c/scratch > ./exec
5

I am completely mystified now.  I'd hoped to knock this bug on the head quickly today, but sadly I've failed.

If you'd like the System.Process patch, I'll be happy to clean it up and send it anyway.

Cheers,
        Simon

> If you'd like the System.Process patch, I'll be happy to clean it up and send it anyway.

It would be great if you could send it.  One thing to note is that we're
still supporting GHC 6.2.2 until Debian etch gets released.  So it would
probably be good to add some ifdefs, keeping the old code around for old
GHC.

Is this right, David?

On Wed, Nov 29, 2006 at 02:39:32PM +0100, Eric Y. Kow wrote:
> > If you'd like the System.Process patch, I'll be happy to clean it up and send it anyway.
> 
> It would be great if you could send it.  One thing to note is that we're
> still supporting GHC 6.2.2 until Debian etch gets released.  So it would
> probably be good to add some ifdefs, keeping the old code around for old
> GHC.
> 
> Is this right, David?

Yeah, that's the plan.  Of course, etch is coming out pretty soon, so if
the patch were sufficiently orthogonal to others, it could perhaps go into
unstable without ifdefs now, and then go into a stable release after etch
is released.  Or, even without the ifdefs, a patch from Simon to use
System.Process would always be helpful whenever we do want to apply it
(even if we have to deal with merge conflicts at the time).
-- 
David Roundy
Department of Physics
Oregon State University

David Roundy wrote:
> On Wed, Nov 29, 2006 at 02:39:32PM +0100, Eric Y. Kow wrote:
>>> If you'd like the System.Process patch, I'll be happy to
> clean it up and send it anyway.
>>
>> It would be great if you could send it.  One thing to note is that
>> we're still supporting GHC 6.2.2 until Debian etch gets released.
>  So it would
>> probably be good to add some ifdefs, keeping the old code around for
>> old GHC.
>>
>> Is this right, David?
>
> Yeah, that's the plan.  Of course, etch is coming out pretty
> soon, so if
> the patch were sufficiently orthogonal to others, it could
> perhaps go into
> unstable without ifdefs now, and then go into a stable
> release after etch
> is released.  Or, even without the ifdefs, a patch from Simon to use
> System.Process would always be helpful whenever we do want to apply it
> (even if we have to deal with merge conflicts at the time).

What's the status of 'make check' on Windows?  I seem to be getting a few failures.

Regarding the System.Process patch, why don't I send a patch that moves to using System.Process on Windows only, because the Debian constraint doesn't apply there, and later you can move the #ifdefs around to use the same code on Unix.  OK?

Cheers,
        Simon

On Wed, Nov 29, 2006 at 03:29:04PM +0000, Simon Marlow wrote:
> David Roundy wrote:
> > On Wed, Nov 29, 2006 at 02:39:32PM +0100, Eric Y. Kow wrote:
> >>> If you'd like the System.Process patch, I'll be happy to
> > clean it up and send it anyway.
> >>
> >> It would be great if you could send it.  One thing to note is that
> >> we're still supporting GHC 6.2.2 until Debian etch gets released.
> >  So it would
> >> probably be good to add some ifdefs, keeping the old code around for
> >> old GHC.
> >>
> >> Is this right, David?
> >
> > Yeah, that's the plan.  Of course, etch is coming out pretty
> > soon, so if
> > the patch were sufficiently orthogonal to others, it could
> > perhaps go into
> > unstable without ifdefs now, and then go into a stable
> > release after etch
> > is released.  Or, even without the ifdefs, a patch from Simon to use
> > System.Process would always be helpful whenever we do want to apply it
> > (even if we have to deal with merge conflicts at the time).
> 
> What's the status of 'make check' on Windows?  I seem to be getting a few failures.

You'd have to ask a windows developer, or maybe Eric.  I know that some
effort was put into making test work on windows, but am unsure how thorough
it was.

> Regarding the System.Process patch, why don't I send a patch that moves
> to using System.Process on Windows only, because the Debian constraint
> doesn't apply there, and later you can move the #ifdefs around to use the
> same code on Unix.  OK?

Sounds fine.  But wouldn't it be just as easy to make the #ifdef depend on
the version of GHC?
-- 
David Roundy
Department of Physics
Oregon State University

David Roundy wrote:
> On Wed, Nov 29, 2006 at 03:29:04PM +0000, Simon Marlow wrote:
>> David Roundy wrote:
>>> On Wed, Nov 29, 2006 at 02:39:32PM +0100, Eric Y. Kow wrote:
>>>>> If you'd like the System.Process patch, I'll be happy to
>>> clean it up and send it anyway.
>>>>
>>>> It would be great if you could send it.  One thing to note is that
>>>> we're still supporting GHC 6.2.2 until Debian etch gets released.
>>>  So it would
>>>> probably be good to add some ifdefs, keeping the old code around
>>>> for old GHC.
>>>>
>>>> Is this right, David?
>>>
>>> Yeah, that's the plan.  Of course, etch is coming out pretty soon,
>>> so if the patch were sufficiently orthogonal to others, it could
>>> perhaps go into unstable without ifdefs now, and then go into a
>>> stable release after etch is released.  Or, even without the
>>> ifdefs, a patch from Simon to use System.Process would always be
>>> helpful whenever we do want to apply it (even if we have to deal
>>> with merge conflicts at the time).
>>
>> What's the status of 'make check' on Windows?  I seem to be getting
>> a few failures.
>
> You'd have to ask a windows developer, or maybe Eric.  I know that
> some effort was put into making test work on windows, but am unsure
> how thorough it was.
>
>> Regarding the System.Process patch, why don't I send a patch that
>> moves to using System.Process on Windows only, because the Debian
>> constraint doesn't apply there, and later you can move the #ifdefs
>> around to use the same code on Unix.  OK?
>
> Sounds fine.  But wouldn't it be just as easy to make the #ifdef
> depend on the version of GHC?

Sure, but I thought you didn't do that sort of thing in darcs :)

Cheers,
        Simon

I have more info on this.  Basically the problem seems to be that ssh reads its stdin in text mode, so strips out the ^Ms.  Various other programs that come with cygwin or MSYS contain code that checks to see whether stdin is attached to a non-terminal, and if so switch to binary mode, but it appears SSH doesn't do this.

Interestingly, you can convince the MSYS ssh to use binary mode by setting the environment variable CYGWIN=binmode.  This doesn't work with the Cygwin ssh, though, at least the one I have.  I'm guessing this is because MSYS is based on an old version of the Cygwin sources.

So there's a fragile workaround: use MSYS ssh and set CYGWIN=binmode.

Cheers,
        Simon

On Thu, Nov 30, 2006 at 09:13:54AM +0000, Simon Marlow wrote:
> I have more info on this.  Basically the problem seems to be that ssh
> reads its stdin in text mode, so strips out the ^Ms.  Various other
> programs that come with cygwin or MSYS contain code that checks to see
> whether stdin is attached to a non-terminal, and if so switch to binary
> mode, but it appears SSH doesn't do this.

I hate Windows.

> Interestingly, you can convince the MSYS ssh to use binary mode by
> setting the environment variable CYGWIN=binmode.  This doesn't work with
> the Cygwin ssh, though, at least the one I have.  I'm guessing this is
> because MSYS is based on an old version of the Cygwin sources.
> 
> So there's a fragile workaround: use MSYS ssh and set CYGWIN=binmode.

Another workaround would be to scp the patch bundle to a temporary location
and then tell darcs where to find it.  I seem to think we may have done
this in the past, but that it has issues with the location of that
temporary patch bundle (I suspect we could just stick it in the repository,
but with what name?), and cleanup would be a nightmare.  What if our
internet connection is flaky?

Another idea would be to have darcs on windows explicitely escape the ^M
characters somehow, and teach darcs when receiving patches to try
unescaping ^M characters if a patch bundle fails to match the hash.  It's
seriously ugly, but might be extra-robust.  I'm not sure what sort of
escaping I'd choose.  I guess MIME-ish escaping would probably be most
reasonable, and we could have it as an option in send.  Wait, we already
MIME-escape when using darcs send.  And darcs apply already has the
unescaping logic.  So perhaps we can just add a feature that on windows
darcs mime-escapes the patch bundle?

It looks like we could do this with the make_email function in the Email
module.  We'd have to supply some extra information, and it'd be formatted
as an email with some extra data in it, but won't take up much extra space,
and the only penalty is some extra parsing time.  Because of that penalty,
I think I'd prefer either a flag, or to do this always on Windows, and
never on other platforms.  But in a pinch we could just do it always
always, and figure that it doesn't hurt much--which it doesn't, unless the
patches are huge.
-- 
David Roundy
Department of Physics
Oregon State University

Hopefully fixed by Message-ID <E1Gw5tu-0007N7-Ti@lanthane.pps.jussieu.fr>,
dated 18 December 2006, on darcs-devel.  Perhaps one of the Simons could
test it?

                                                          Juliusz

Juliusz Chroboczek wrote:
> Juliusz Chroboczek <jch@pps.jussieu.fr> added the comment:
>
> Hopefully fixed by Message-ID
> <E1Gw5tu-0007N7-Ti@lanthane.pps.jussieu.fr>,
> dated 18 December 2006, on darcs-devel.  Perhaps one of the Simons
> could test it?

Thanks Juliusz, I'll try to test it out.

Cheers,
        Simon