darcs

Issue 324 ssh with tilde (~) in path returns unrelated failure message.

Title ssh with tilde (~) in path returns unrelated failure message.
Priority bug Status given-up
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, kowey, markstos, mornfall, roumier, thorkilnaur, tommy
Assigned To
Topics FilePath, SSH

Created on 2006-10-25.16:17:51 by roumier, last changed 2017-07-30.23:21:26 by gh.

Messages
msg1146 (view) Author: roumier Date: 2006-10-25.16:17:46
Hi,

I'm not sure whether this is due to a (bad) action on my side. Anyway, Eric
(thank you Eric :) will take (good) care of it !

Cheers,
joseph

$ darcs pull
foo@bar's password:
Pulling from "foo@bar:~/redacthese"...

Tue Oct 24 16:03:09 CEST 2006  joseph.roumier@loria.fr
  * 23
Shall I pull this patch? (1/2)  [ynWvpxqadjk], or ? for help: y

Tue Oct 24 19:14:12 CEST 2006  joseph.roumier@loria.fr
  * debut de quelque chose ?
Shall I pull this patch? (2/2)  [ynWvpxqadjk], or ? for help: y

darcs failed:  user error (Error applying patch to recorded.
Running 'darcs repair' on the target repository may help.
./these.aux: openBinaryFile: does not exist (No such file or directory))
Your repository is now in an inconsistent state.
This must be fixed by running darcs repair.

----- Fin du message transféré -----
msg1148 (view) Author: kowey Date: 2006-10-25.17:26:17
On Wed, Oct 25, 2006 at 16:17:51 +0000, roumier wrote:
> darcs failed:  user error (Error applying patch to recorded.
> Running 'darcs repair' on the target repository may help.
> ./these.aux: openBinaryFile: does not exist (No such file or directory))
> Your repository is now in an inconsistent state.
> This must be fixed by running darcs repair.

I sat down with Joseph to figure out how on earth this came about, and
we have about 80% of an explanation (see below)

What next...
------------
Joseph: could you please send the sftp error message as a reply to this
ticket?

Somebody with free time: please make darcs get more robust wrt to IO
errors.  I make some suggestions below.

The story
---------
# foo@bar
cd ~/myrepo
darcs init
# make some changes
# record some patches

# local machine
darcs get foo@bar:~/myrepo
# overlook the sftp error!

# foo@bar
cd ~/myrepo
# make some more changes
# record some more patches

# local machine
darcs pull
# WHAM! (error)

Observations
------------
1) darcs get does not work with '~' in ssh paths:
   something like darcs get foo@bar:~/myStuff
   will fail with an error message from sftp

   This is because sftp does not interpret '~' in ssh paths.  Fair
   enough, but maybe darcs should disallow tildes in such paths.
   Or maybe that's too ad-hoc a solution...

2) darcs get does not detect errors from sftp.

   At the very least, the get should say something scary like
   darcs failed: (blah blah blah)

3) darcs get does not clean up after itself when something goes
   wrong, leaving the user to use an inconsistent repository.

   Perhaps it should move _darcs to something like _darcs-broken 
   so that darcs doesn't let the user do stuff in it.

   In Joseph's case, what went wrong is that darcs get managed to get
   the inventory file (via scp?), but not the actual patches (via sftp,
   which doesn't like ~)

The mystery
-----------
What bothers me about this is that it's only 80% of an explanation.
If our explanation was completely true, Joseph's working directory
would be empty, but strangely, it is not empty, and contains a
recent looking version of his files.  How on earth did those get there?
He does not use Unison, rsync or anything like that.
msg1150 (view) Author: droundy Date: 2006-10-25.18:19:58
On Wed, Oct 25, 2006 at 05:26:29PM +0000, Eric Kow wrote:
> Observations
> ------------
> 1) darcs get does not work with '~' in ssh paths:
>    something like darcs get foo@bar:~/myStuff
>    will fail with an error message from sftp
> 
>    This is because sftp does not interpret '~' in ssh paths.  Fair
>    enough, but maybe darcs should disallow tildes in such paths.
>    Or maybe that's too ad-hoc a solution...

See below. (fallback to scp?)

> 2) darcs get does not detect errors from sftp.
> 
>    At the very least, the get should say something scary like
>    darcs failed: (blah blah blah)

Yes, definitely a good idea.  If we captured the stderr of sftp, we could
throw that as an exception (perhaps with an explanatory note).  Perhaps
this is the time to start making the switch to System.Process?

> 3) darcs get does not clean up after itself when something goes
>    wrong, leaving the user to use an inconsistent repository.
> 
>    Perhaps it should move _darcs to something like _darcs-broken 
>    so that darcs doesn't let the user do stuff in it.
> 
>    In Joseph's case, what went wrong is that darcs get managed to get
>    the inventory file (via scp?), but not the actual patches (via sftp,
>    which doesn't like ~)

I think we could solve number three in an ultra-clean way by doing a
two-fold fix.  First, we should make get take a lock on the repository as
soon as possible.  Secondly, we should figure out a way to ensure that when
code exits leaving the repository in a bad state, the lock isn't just
removed, but instead the repository is marked as being in a bad state.

The latter bit is trickier, as we'd need to do it in a separate (smaller)
code section than the withRepoLock, since much of the work done while
holding the repo lock is read-only stuff that only needs the lock to ensure
that the repository isn't modified after darcs decides what to do.  I was
thinking just last night about the possibility of creating a separate type
related to Repository, which represents a mutable repository (say
MutableRepository), and then write a

withDangerLock :: (MutableRepository -> IO ()) -> IO ()

which unlike withRepoLock, will leave the repository in a broken state if
the code exits with an exception, and then we'd need to make repair fix
that broken state.

I should also note that I'm in the process of refactoring Get to use the
Repository interface.  I expect it'll be a week or so more, since I've not
much time to work on this.  It's possible that once the new Repository
stuff is in we won't need withDangerLock (which could use a better name) to
be exported from Repository itself.

> The mystery
> -----------
> What bothers me about this is that it's only 80% of an explanation.
> If our explanation was completely true, Joseph's working directory
> would be empty, but strangely, it is not empty, and contains a
> recent looking version of his files.  How on earth did those get there?
> He does not use Unison, rsync or anything like that.

scp will accept tilde in paths just fine.  So the trouble is only with
sftp.  Perhaps we should fall back to scp when sftp fails for some reason?
It's hokey and slow and would lead to unexpected slowness at times, but
might be nicer than other solutions.  Another possibility would be to munge
:~/ into : in scp paths before passing them to sftp, but of course that
only fixes "redundant" tildes, not things like server:~kowey/darcs, so it'd
make the error less commonly run into, but wouldn't remove it entirely.
-- 
David Roundy
Dept. of Physics
Oregon State University
msg3265 (view) Author: markstos Date: 2008-02-09.17:59:48
I got an unexpected error when I tried to test this. I appeared to get past the
"~" issue, but ran into a different one:

darcs get mark@nollie.summersault.com:~/perl/dfv/
darcs: failed to read patch:
Mon Sep 27 20:23:17 EST 2004  mark@summersault.com
  * Initial commit (mark@summersault.com--2004/dfv--main--0)
/home/mark/Haskell/dfv/_darcs/patches/20040928012317-ab06a-517b1dde0b6c4fa46d37921858716fd9c1f12c58.gz:
openBinaryFile: does not exist (No such file or directory)

######

I did a follow-up test with an alternate syntax that avoided the "~" and that
solved the problem. I'm changing the priority of this from a "wish" to a "bug",
because darcs does appear to be broken in this case. If it can't handle a tilde
in a path name, it should just say so and give up, rather than failing in a way
that appears unrelated.
msg3270 (view) Author: droundy Date: 2008-02-09.18:30:29
On Sat, Feb 09, 2008 at 05:59:50PM -0000, Mark Stosberg wrote:
> I did a follow-up test with an alternate syntax that avoided the "~" and that
> solved the problem. I'm changing the priority of this from a "wish" to a "bug",
> because darcs does appear to be broken in this case. If it can't handle a tilde
> in a path name, it should just say so and give up, rather than failing in a way
> that appears unrelated.

The trouble is that sftp fails to handle "~".  We could avoid using sftp,
or we could hack around this by replacing server:~/foo with server:foo
(which is identical, as far as I know), but neither of those is
particularly appealing.
-- 
David Roundy
Department of Physics
Oregon State University
msg3304 (view) Author: markstos Date: 2008-02-10.21:34:34
>
> On Sat, Feb 09, 2008 at 05:59:50PM -0000, Mark Stosberg wrote:
>> I did a follow-up test with an alternate syntax that avoided the "~" and that
>> solved the problem. I'm changing the priority of this from a "wish" to a "bug",
>> because darcs does appear to be broken in this case. If it can't handle a tilde
>> in a path name, it should just say so and give up, rather than failing in a way
>> that appears unrelated.
>
> The trouble is that sftp fails to handle "~".  We could avoid using sftp,
> or we could hack around this by replacing server:~/foo with server:foo
> (which is identical, as far as I know), but neither of those is
> particularly appealing.

The third option is to notice a "~" is being used is being used and fail
gracefully: Inform the user that these are not supported, recommend the
alternate syntax, and exit.

     Mark
msg4616 (view) Author: kowey Date: 2008-05-09.17:06:55
If anybody wants to implement this, they should note that Darcs.FilePathUtils
knows the difference between a remote path and a local path.  We should probably
take advantage of this and reject (~) in remote paths only (then again, we could
just as well reject them in all paths, which may be good).  Tough luck to the
one user who really does want a tilde in their path.
History
Date User Action Args
2006-10-25 16:17:51roumiercreate
2006-10-25 17:26:29koweysetstatus: unread -> unknown
nosy: droundy, tommy, kowey, roumier
messages: + msg1148
2006-10-25 18:20:07droundysetnosy: droundy, tommy, kowey, roumier
messages: + msg1150
2007-03-08 11:23:12koweysetnosy: + beschmi
2008-02-09 17:59:50markstossetpriority: wishlist -> bug
nosy: + markstos
messages: + msg3265
title: sftp complains about "~" in path, maybe darcs should complain first -> get over ssh with tilde (~) in path returns unrelated failure message.
2008-02-09 18:30:31droundysetnosy: droundy, tommy, beschmi, kowey, markstos, roumier
messages: + msg3270
2008-02-10 21:34:35markstossetnosy: droundy, tommy, beschmi, kowey, markstos, roumier
messages: + msg3304
2008-02-16 22:48:57markstossetstatus: unknown -> has-patch
nosy: droundy, tommy, beschmi, kowey, markstos, roumier
topic: + SSH
2008-02-16 22:49:08markstossettopic: + Confirmed
nosy: droundy, tommy, beschmi, kowey, markstos, roumier
2008-05-09 17:06:57koweysetnosy: + dagit
messages: + msg4616
2008-08-24 15:18:44koweysetstatus: has-patch -> needs-reproduction
nosy: droundy, tommy, beschmi, kowey, markstos, dagit, roumier
2008-08-24 15:19:00koweysetnosy: + mornfall
2008-08-24 15:19:37koweylinkissue1032 superseder
2008-08-24 15:20:09koweysetnosy: droundy, tommy, beschmi, kowey, markstos, dagit, roumier, mornfall
title: get over ssh with tilde (~) in path returns unrelated failure message. -> ssh with tilde (~) in path returns unrelated failure message.
2009-08-06 17:38:23adminsetnosy: + jast, Serware, dmitry.kurochkin, darcs-devel, zooko, simon, thorkilnaur, - droundy, roumier
2009-08-06 20:35:08adminsetnosy: - beschmi
2009-08-10 21:57:56adminsetnosy: + roumier, - darcs-devel, zooko, jast, Serware
2009-08-10 23:57:28adminsetnosy: - dagit
2009-08-25 17:51:59adminsetnosy: + darcs-devel, - simon
2009-08-27 14:03:20adminsetnosy: tommy, kowey, markstos, darcs-devel, roumier, thorkilnaur, dmitry.kurochkin, mornfall
2009-09-04 17:08:48koweysetstatus: needs-reproduction -> needs-implementation
nosy: tommy, kowey, markstos, darcs-devel, roumier, thorkilnaur, dmitry.kurochkin, mornfall
topic: - Confirmed
2009-09-25 09:07:51ghsettopic: + FilePath
nosy: tommy, kowey, markstos, darcs-devel, roumier, thorkilnaur, dmitry.kurochkin, mornfall
2017-07-30 23:21:26ghsetstatus: needs-implementation -> given-up