darcs

Issue 826 deal with working dir errors better on darcs push

Title deal with working dir errors better on darcs push
Priority feature Status given-up
Milestone Resolved in
Superseder log failures applying to working dir as patch; and keep warning the user about them
View: 1010
Nosy List darcs-devel, dmitry.kurochkin, kowey, thorkilnaur, tommy, zooko
Assigned To
Topics UI

Created on 2008-05-01.13:23:08 by zooko, last changed 2017-07-30.23:14:29 by gh.

Messages
msg4422 (view) Author: zooko Date: 2008-05-01.13:23:01
Zandr reported this:

boreta-5:www boreta$ darcs push justin@dev:/home/darcs/www
Pushing to "justin@dev:/home/darcs/www"...
Wed Apr 30 17:58:10 PDT 2008  justin@allmydata.com
  * website - new header logo with text
Shall I push this patch? (1/1)  [ynWvpxdaqjk], or ? for help: y
Reading pristine 398/784
Reading pristine 652/784
Warning: ./.inc/brands/amd/header.inc.php-0: takeFile
./.inc/brands/amd/header.inc.php-0 in /home/darcs/www: openFd: permission denied
(Permission denied)
Warning: ./.src/amd/default.css-0: takeFile ./.src/amd/default.css-0 in
/home/darcs/www: openFd: permission denied (Permission denied)
Warning: ./.src/amd/default.css-0: takeFile ./.src/amd/default.css-0 in
/home/darcs/www: openFd: permission denied (Permission denied)
Warning: ./.src/amd/default.css-0: takeFile ./.src/amd/default.css-0 in
/home/darcs/www: openFd: permission denied (Permission denied)
Finished applying...
Push successful.
boreta-5:www boreta$

I did an experiment -- making a file unreadable and then pushing a patch that
touched that file -- and got a different problem:

 wonwin-mcbrootles-computer:~/darcsexperiment/y$ darcs push -a -v ../x
Pushing to "../x"...
We have the following patches to push:
Wed Apr 30 19:40:30 MDT 2008  z2
  * add

darcs failed:  Error applying patch to working dir:
./file: openBinaryFile: permission denied (Permission denied)
Apply failed!

In this latter case the target repository has been "corrupted" inasmuch as the
working directory differs from the pristine, which is not supposed to happen on
a push (without allow-mark-conflicts).
msg4589 (view) Author: kowey Date: 2008-05-09.08:25:01
Corruption is way too strong a word for this, I think.  Downgrading to feature
(one which needs some discussion)

Darcs manages to keep the pristine intact, which is the most important thing,
here.  

Yes, the working dir is now inconsistent with pristine, but this kind of thing
is supposed to happen anyway in normal usage.  Consider unrecorded changes, for
example. Or that, if you apply a patch that tries to remove a non-empty working
dir directory, darcs succeeds and prints a warning.  That's a feature, not a bug.

On the other hand, it is true that this kind of behaviour may be unexpected if
you're doing darcs push.  

Perhaps darcs push should be more careful than darcs apply.  For example, if it
catches a working dir error, it should maybe obliterate the patches it tried to
apply... I'm guessing we might need a --careful switch for apply (ugh, rolling
this out would be annoying), or a darcs push which did not use apply.

Could we just make this a wont-fix? :-D
msg4598 (view) Author: zooko Date: 2008-05-09.12:21:08
I'm a bit uncomfortable with marking this as wont-fix, because maybe some people
want to rely on the fact that a remote repository that only gets pushed into
will never have unrecorded changes in it.    Eh, on the other hand this
shouldn't happen unless someone has already been mucking about in that repo
(changing permissions) and it clearly announces it so that they can go much
about again and fix it.

But what about the issue that Justin and Zandr reported: "permission denied"
followed by "Push successful."?  I don't know what happened there, and at the
very least it is a user interface glitch to say permission denied and then Push
successful.
msg4602 (view) Author: kowey Date: 2008-05-09.12:29:21
> But what about the issue that Justin and Zandr reported: "permission denied"
> followed by "Push successful."?  I don't know what happened there, and at the
> very least it is a user interface glitch to say permission denied and then Push
> successful.

Well, for darcs apply, this is certainly what we want to happen (this
is robust application of patches to the working directory, see
issue284  )

The push was successful in the sense that the repository was updated
correctly; it now has the right patches and its pristine cache is
a-ok.  Whatever happens to the working directory is immaterial (famous
last words).

I could see that this could be confusing with darcs push, but I also
get the feeling that trying to be more intelligent about this could
just confuse users more!  At least in our current model, darcs being
tolerant about working dir errors is a constant.

I'm not really sure what to do about this (except to note that the
darcs push --careful could also be called darcs push --intolerant)

In any case, not a bug, but something worth discussing further.

Also, perhaps the darcs init --no-working option would be a way to
side-step all of this?
msg4607 (view) Author: zooko Date: 2008-05-09.15:28:35
On May 9, 2008, at 6:29 AM, Eric Kow wrote:

> Well, for darcs apply, this is certainly what we want to happen (this
> is robust application of patches to the working directory, see
> issue284  )
>
> The push was successful in the sense that the repository was updated
> correctly; it now has the right patches and its pristine cache is
> a-ok.  Whatever happens to the working directory is immaterial (famous
> last words).
>
> I could see that this could be confusing with darcs push, but I also
> get the feeling that trying to be more intelligent about this could
> just confuse users more!  At least in our current model, darcs being
> tolerant about working dir errors is a constant.
>
> I'm not really sure what to do about this (except to note that the
> darcs push --careful could also be called darcs push --intolerant)
>
> In any case, not a bug, but something worth discussing further.
>
> Also, perhaps the darcs init --no-working option would be a way to
> side-step all of this?

Thanks for explaining this, Eric.  Perhaps if the darcs output in  
this case indicated something about this then it would solve the user  
perception issue.  For example, could it say "The remote repository  
was successfully updated, but the working directory couldn't be  
updated to match, so now there are differences between the pristine  
version and the working directory.".
msg4609 (view) Author: kowey Date: 2008-05-09.15:40:58
> Thanks for explaining this, Eric.  Perhaps if the darcs output in this case
> indicated something about this then it would solve the user perception
> issue.  For example, could it say "The remote repository was successfully
> updated, but the working directory couldn't be updated to match, so now
> there are differences between the pristine version and the working
> directory.".

I would be happy with such a solution, with a minor quibble. In normal
darcs usage, it is expected that the working dir and pristine
directory have differences anyway, so such a message would be
misleading.

Maybe a version like this would be more accurate:

 I had some trouble all of the patches to the working directory.  Your
repository is still consistent, and the errors are likely minor, but
you should check the warnings above to be sure.  If the results are
not what you expected, you might consider doing a darcs revert to
resynchronize your working directory with the pristine cache.  Doing
so may cause you to lose whatever unrecorded changes you had in your
working directory.

Note that from a UI standpoint, this would be a useful thing to have
anyway in pull/apply.  You may not notice the Warnings scrolling by
when pull/applying a patch, so having a reminder at the end would be
good.

If somebody could boil this down into something a little more concise,
I'd be happy.

Don't know if implementation would be tricky.  We would have to keep
track of the fact that tolerant application took place.

Hmm, maybe we should ressurect issue274 to record a backup bundle?
msg7049 (view) Author: thorkilnaur Date: 2009-01-12.10:05:31
Setting status deferred, as this is a new feature.

Best regards
Thorkil
msg7075 (view) Author: thorkilnaur Date: 2009-01-13.11:11:23
Sorry, deferring was a mistake. Setting status need-info instead, asking for a 
discussion of the issue: As kowey notes (msg4609), it is not entirely clear how 
to deal with this problem.

Thanks and best regards
Thorkil
msg8708 (view) Author: kowey Date: 2009-09-05.09:37:51
I think we first need to implement issue1010 which records a leftovers patch if
ever there is a failure applying to the working directory.  If we did that, we
could just percolate the message to push.
History
Date User Action Args
2008-05-01 13:23:08zookocreate
2008-05-09 08:25:03koweysetpriority: bug -> feature
status: unread -> unknown
messages: + msg4589
nosy: + kowey
title: insufficient permissions in target repo lead to corruption -> deal with working dir errors better on darcs push
2008-05-09 12:21:09zookosetnosy: tommy, beschmi, kowey, zooko, dagit
messages: + msg4598
2008-05-09 12:29:23koweysetnosy: tommy, beschmi, kowey, zooko, dagit
messages: + msg4602
2008-05-09 15:28:37zookosetnosy: tommy, beschmi, kowey, zooko, dagit
messages: + msg4607
2008-05-09 15:41:00koweysetnosy: tommy, beschmi, kowey, zooko, dagit
messages: + msg4609
2008-05-09 15:52:28koweysettopic: + UI
nosy: tommy, beschmi, kowey, zooko, dagit
2009-01-12 10:05:34thorkilnaursetstatus: unknown -> deferred
nosy: + dmitry.kurochkin, simon, thorkilnaur
messages: + msg7049
2009-01-13 11:11:25thorkilnaursetstatus: deferred -> waiting-for
nosy: tommy, beschmi, kowey, zooko, dagit, simon, thorkilnaur, dmitry.kurochkin
messages: + msg7075
2009-08-06 21:03:43adminsetnosy: - beschmi
2009-08-11 00:12:19adminsetnosy: - dagit
2009-08-25 17:38:42adminsetnosy: + darcs-devel, - simon
2009-08-27 14:14:51adminsetnosy: tommy, kowey, darcs-devel, zooko, thorkilnaur, dmitry.kurochkin
2009-09-05 09:37:52koweysetnosy: tommy, kowey, darcs-devel, zooko, thorkilnaur, dmitry.kurochkin
superseder: + log failures applying to working dir as patch; and keep warning the user about them
messages: + msg8708
2017-07-30 23:14:29ghsetstatus: waiting-for -> given-up