Cannot delete directory with ignored files

Bug #323111 reported by Christophe Simonis (OpenERP)
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
Medium
Vincent Ladeuil

Bug Description

bzr musn't generate a conflict when trying to delete a directory containing only ignored files (like a python module containting remaining .pyc files)

Related branches

Revision history for this message
Vincent Ladeuil (vila) wrote :

bzr doesn't make a distinction between "junk" files (as .pyc files) and "precious" but not versioned files (passwords, nuclear launch codes).

Until such a distinction is made, we can't blindly delete these files in the mentioned case.

Changed in bzr:
importance: Undecided → Wishlist
status: New → Confirmed
Martin Pool (mbp)
Changed in bzr:
importance: Wishlist → Medium
Revision history for this message
Martin Pool (mbp) wrote :

Some options for handling this:

0- Give a clear message that "directory could not be deleted because it contains unversioned files" and then make sure that just deleting the directory is enough to clear the conflict.

1- Make a trash directory somewhere, and move the to-be-deleted files into there. This could be useful if it's done every time files are to be deleted. It may use a lot of space.

2- Add a rules category of 'junk', which is like a stronger form of 'ignored'. These files are always considered safe to delete automatically, though ignored files are not.

  [name *.pyc]
     junk = true

This perhaps gives the correct behaviour (that these files should never be kept) at the expense of some more user model complexity. It doesn't totally address the bug because people who have first hit this situation may still be confused, but it would complement #0. This could be useful if we then automatically collected junk at various points, like after updating, because it would remove outdated pyc files that sometimes cause trouble.

Things like editor backups and swap files should not be removed so aggressively, but probably shouldn't block directory deletion.

Revision history for this message
Jonathan Lange (jml) wrote :

Fixing bug 344013 would make this bug less painful. Maybe that's what Martin means by the second half of option 0.

Option 3 would be to interactively prompt for deleting unversioned files.

Martin, are any of these options preferred?

summary: - can not delete directory with ignored files
+ Cannot delete directory with ignored files
Revision history for this message
Martin Pool (mbp) wrote :

Yes, that's what I meant.

I think interactively prompting would be fine.

I think any of these options are acceptable and indeed doing more than one is reasonable. If it was me I would probably do 0 first and then 2.

Vincent Ladeuil (vila)
Changed in bzr:
assignee: nobody → Vincent Ladeuil (vila)
status: Confirmed → In Progress
Revision history for this message
Vincent Ladeuil (vila) wrote :

I've started working on this.

More precisely, I'm going to implement moving the un-versioned files into a top-level bzr-orphans directory and
emit a warning in this case.

0 is not appropriate as the problem may occur during a pull or a switch (the infamous .pyc problem).
2 can come later but I'd like bzrlib.ignores to be re-factored first so we can re-use it for .bzrjunk.

Note that I punt regarding the distinction between junk and precious by *not* deleting the file
but moving it out the way. It will be up to the user to regularly inspect/purge/delete the orphans directory.

There are various ways to name the files in the .bzr-orphans directory, I'll start with something simple
<file>~n~.

Revision history for this message
Martin Pool (mbp) wrote :

I think moving them into a top-level directory is fine. I'd like the patch for this to abstract the handling of them so that we can later add the others.

> 0 is not appropriate as the problem may occur during a pull or a switch (the infamous .pyc problem).

I think it's appropriate [to give you conflicts on the deletion]: you can already get other kinds of conflicts on pull or switch. The main thing we have to do there is make sure deleting the directory is treated as implicit resolution, because it is not at present.

Revision history for this message
Vincent Ladeuil (vila) wrote : Re: [Bug 323111] Re: Cannot delete directory with ignored files

>>>>> Martin Pool <email address hidden> writes:

    > I think moving them into a top-level directory is fine. I'd like
    > the patch for this to abstract the handling of them so that we can
    > later add the others.

What do you mean by others ?

    >> 0 is not appropriate as the problem may occur during a pull or a
    >> switch (the infamous .pyc problem).

    > I think it's appropriate [to give you conflicts on the deletion]:
    > you can already get other kinds of conflicts on pull or switch.

Oh, I don't dispute that, I was talking about breaking the natural flow
by creating what could be considered as spruious conflicts, if there are
othe conflicts, they will still be detected.

    > The main thing we have to do there is make sure deleting the
    > directory is treated as implicit resolution, because it is not at
    > present.

Right, I consider that to be a different bug (see bug #344013, bug
#138803
, bug #287979, etc). If you think I've high jacked this one I can
create a new one.

Revision history for this message
Martin Pool (mbp) wrote :

On 8 September 2010 16:16, Vincent Ladeuil <email address hidden> wrote:
>>>>>> Martin Pool <email address hidden> writes:
>
>    > I think moving them into a top-level directory is fine.  I'd like
>    > the patch for this to abstract the handling of them so that we can
>    > later add the others.
>
> What do you mean by others ?

Other strategies for replacing ignored or unversioned files. istm
users may want to configure that they should be just deleted, cause an
error, be saved to a trash directory, etc.

>    >> 0 is not appropriate as the problem may occur during a pull or a
>    >> switch (the infamous .pyc problem).
>
>    > I think it's appropriate [to give you conflicts on the deletion]:
>    > you can already get other kinds of conflicts on pull or switch.
>
> Oh, I don't dispute that, I was talking about breaking the natural flow
> by creating what could be considered as spruious conflicts, if there are
> othe conflicts, they will still be detected.

+1

>
>    > The main thing we have to do there is make sure deleting the
>    > directory is treated as implicit resolution, because it is not at
>    > present.
>
> Right, I consider that to be a different bug (see bug #344013, bug
> #138803, bug #287979, etc). If you think I've high jacked this one I can
> create a new one.

No, that's fine. It would be good to fix that too.

--
Martin

Revision history for this message
Martin Pool (mbp) wrote :

There was a good suggestion in <https://bazaarvcs.wordpress.com/2010/09/15/poll-deleting-directories-containing-unversioned-files/> that we should leave the directory but mark it unversioned. I think that's quite elegant. It certainly points to it being useful to have this modularized.

Revision history for this message
Vincent Ladeuil (vila) wrote :
Download full text (3.3 KiB)

@Martin: This is the current behaviour...

There are distinct issues than can be addressed separately:
- handling junk files so we can safely delete them and *avoid* having these files block the directory deletion,
- handling precious files and generate a conflict because they can't be deleted and block the directory deletion
  (that's the actual behaviour because we consider all unversioned files as precious)

The fix I'm proposing work around the lack of precious/junk files by *not* deleting any of them but putting them aside.
If you feel that the behaviour change is too risky we can keep the actual one activated by default.

Yet, AFAIC (and I'm not alone here), the actual behaviour is a pain because I don't have "precious" files that are not versioned !

Keep in mind that the proposed fix triggers during the conflict resolution of TreeTransform, i.e. we don't know at this point
what the original request from the user was, we are handling the consequences of this request which has generated a conflict.
(Also note that we're talking about the automatic conflict resolution that is run unconditionally for every tree transform, which
is different from the automatic conflict resolution run by 'bzr resolve'. It's unfortunate that the distinction is somehow
confusing and may reveal a design problem, but that's a totally different subject :-/).

Having a proper handling of junk/precious files will *also* address the problem since we should then have enough
information to decide what to do (Only junk files? Delete the directory. Some precious. file ? Tell the user we can't
fulfill his request, generating a conflict or move it aside in a safe place to avoid the conflict and fulfill the user request).

But I've yet to hear reports about the later that can't be addressed by either:
- versioning the precious files,
- moving them to their intended place (or canceling the directory deletion, which is a variant).

So even if/when we implement such handling, the fix will be useful since it will:
- remove the junk files,
- move the precious files in a safe place (which should be an available option anyway).

 Last, but not least, why is it painful today ? Here is a scenario:

- trunk have the bzrlib/benchmark/tree_creator directory (and its associated junk .pyc files)
- branchA remove the directory
- branchA is merged into trunk, generating conflicts: Can't delete bzrlib/benchmark and bzrlib/brnchmark/tree_creator

If we leave the directory in place and for some reason have to revert trunk to a revision older than the one merging branchA,
we generate another kind of conflict leading to bzrlib/benchmark being renamed to bzrlib/benchmark.MOVED.

Rinse, repeat a few times and you end up crying.

This is even more painful if you're working in a loom and you navigate between threads... which is what people are complaining
in the first place and how we end up with the idea of this fix with Robert.

I think Andrew raised an interesting point when he asked for revert being able to re-parent an orphan, the answer to that is:
we can't. 'revert' contract is to restore the working tree into a known state, junk/precious files are not versioned, so there is no
known s...

Read more...

Revision history for this message
Martin Pool (mbp) wrote :

On 18 September 2010 19:17, Vincent Ladeuil <email address hidden> wrote:
> @Martin: This is the current behaviour...

The difference, I think, is that they were suggesting the directory
would be left behind but not conflicted. Just present and presumably
unknown (or maybe ignored.)

>  Last, but not least, why is it painful today ? Here is a scenario:
>
> - trunk have the bzrlib/benchmark/tree_creator directory (and its associated junk .pyc files)
> - branchA remove the directory
> - branchA is merged into trunk, generating conflicts: Can't delete bzrlib/benchmark and bzrlib/brnchmark/tree_creator
>

I know, I do hit this too. :)

What I typically do then is delete that directory, which in the
absence of a junk file category is a reasonable thing to do: ok,
really delete them. The annoying part is that you then need to
manually mark all the involved directories as resolved, when it ought
to be obvious that they are resolved.

I'm not convinced that generating a bzr-orphans directory that I then
need to remove is really all that much better.

> If we leave the directory in place and for some reason have to revert trunk to a revision older than the one merging branchA,
> we generate another kind of conflict leading to bzrlib/benchmark being renamed to bzrlib/benchmark.MOVED.
>
> Rinse, repeat a few times and you end up crying.

istm that's another bug, perhaps: if we want to transform to add a
directory, and the directory already exists but is unversioned,
perhaps we should just use it and then recurse down in to it

Having been through this, at the moment I think we should add the
mechanism, but perhaps leave the default on leaving them where they
are, and then make it easier to resolve the conflict by removing the
directory?

--
Martin

Vincent Ladeuil (vila)
Changed in bzr:
milestone: none → 2.3b3
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.