Re: "stgit clean" has problems with removed generated files

I recently encountered a problem with "stgit clean", but it really is a generic issue, which can also impact other subcommands.

In a kernel tree, the precise problem I had is due to generated files committed by error in an upstream branch (a BSP from a well-known vendor, indeed ;). The first patch in my stgir stack does some cleanup by removing them from git control (so that "make dep" does not cause them to change every so often).

Now when I want to run "stg clean" for applied patches, stgit first wants to pop the whole stack, including that patch, which triggers the following error:

fatal: Untracked working tree file 'include/asm-arm/constants.h' would be                                                                                                              overwritten by merge.                                                                                                                                                                  stg clean: git-read-tree failed (local changes maybe?)                                                                                                                                 

Obviously, removing all such files by hand allows to run "stg clean", as does (floating and) popping that patch and deleting it, or running "stg clean --unappplied". The 1st approach is intrusive, and forces the user to rerun "make dep", which is not very friendy; the 2nd one is too error-prone for an operation that ought to be risk-less (delete has no --undo). The 3rd option is probably the best, but is surely not so intuitive.

The root issue seems to be that stgit has problem with such generated files, ie., files that were removed from version-control, but can still legitimately exist in the tree. Dealing with them could possibly be done (eg. allowing to back them up, and restore them when pushing the annoying patch), but is not a trivial issue (eg. we still need to guard the user against real conflicts).

At least we could put a notice about this case in the doc, so users don't get too surprised.

I could think of several (non-exclusive) possibilities to deal with the precise problem I got, which should allow.

First, when cleaning patches, we could first look up which patches are to be removed, and only pop the necessary ones.

Second, we could generalize the "clean" subcommand to accept arbitrary ranges, not only the "applied" and "unapplied" ones. A special case would be "stg clean that-patch", which would be a secure version of "stg delete".

BTW, maybe it would those make sense to allowthose special ranges in most places a range is valid.

Best regards,

On 23/11/06, Yann Dirson <ydirson@altern.org> wrote:

That's a git error and I think it is the correct behaviour. It is safer to notify that a local file is overridden by a merge/switch operation rather than just losing its content.

Maybe 'stg clean' should only pop to the first empty applied patch rather than popping all of them as it is also more efficient.

That's a GIT issue more than an StGIT one, unless GIT already has a way to deal with this and StGIT doesn't pass the right options.

OK, I mentioned it above as well. This should really be done for efficiency but it might not solve the problem if the empty patch is deeper into the stack.

Easy to fix as well.

Is there any other place where ranges could be used but aren't?

On Thu, Nov 23, 2006 at 04:33:42PM +0000, Catalin Marinas wrote:

Right, I do not discuss the behaviour of git here. But when I first encountered this issue, I was really wondering about what was happenning. It would be really helpful in such a case, if stgit was able to pinpoint the precise patch which could not be popped. It could also be helpful to tell when popping patches - currently it's done "behind my back", and I could only understand what was happenning by reading the code.

I'm not sure if git itself should add support for this. IMHO, such an issue is more related to the nature of patch management - git itself does not have a need to repetedly pop/push paches, with related merge hassles like this one.

Right, it could be advertised as good practice to burry those near the bottom of the stack, and anyway to keep actively-changing ones near the top.

Hm, let's see... I'd say "export" (I have missed it already), "files" and maybe "commit" and "pick", although the latter would require a syntax for ranges in other branch.

While reviewing the various commands for this, I realized that "stg pop <patch>" semantics is significantly different from "stg push <patch>" - ie. it is an equivalent of "goto". What about turning it into a float+pop, to better match the "push" behaviour ?

I also realized that "stg help <command>" output does not go through to the pager, while eg. the help for "mail" is quite long.

Best regards,

Yann Dirson wrote:

I think it was corrected in git, or is being corrected (meaning it is in 'next' or in 'pu'); it being relaxing "would be overwritten by merge" check for files which were, but are not, under version control.

On 23/11/06, Yann Dirson <ydirson@altern.org> wrote:

I fixed this.

I changed the export command to take a range of patches as arguments (and a --dir option for the export directory).

Thanks for the suggestion, I fixed pop to take patch ranges now.

Added this feature as well. I'll try to push the changes tonight.