Re: stgit: bunch of bugreports/wishes

From: Catalin Marinas <catalin.marinas@gmail.com>
Date: Jun 26, 2006, 21:04 UTC
Message-ID: <b0943d9e0606261404s38d4e316ja1af671f2d0d6e73@mail.gmail.com>
In-Reply-To: <20060622221425.GA7851@nowhere.earth>

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

> Here are a number of problems I encountered while playing with
> uncommit with 0.10:

"uncommit" was really intended as generating some simple patches from a linear list of commits (maybe for undoing a "stg commit" or after a git-am to modify some patches before pushint upstream). History re-writing is somehow outside StGIT's goals.

> - uncommit ignores grafts.  This causes "uncommit -n" to through
> "graft merges" without asking, and surely gives unexpected result
> when a graft is used to change an ancestor rather than adding one.

[...]

I could fix "uncommit" to fail at this point but, as I said above, I wouldn't add extra features to this command.

Maybe you can explain your workflow a bit as I don't see the need for mass uncommitting.

Show 11 quoted lines

> - uncommit could be more flexible to help with mass-uncommitting,
> eg. with something like "--to <commit>" (to avoid counting manually),
> or "--to-merge" to cleanly stop on first merge instead of failing
> there.  This may have an impact on how uncommits are numbered.
>
> - uncommit synopsis is incomplete (lacks " | -n <n> <basename>")
>
> - after mass-uncommitting, more help to look at the stack would be
> needed.  Eg. a "stg series" flag to print more commit info (author,
> files), or to limit the listing to a given author (like "stg patches"
> limits for a file).

These would be good indeed. I also had a plan to generate the patch name from the subject line (i.e. replacing the spaces with a dash) to be more meaningful. But got really busy with my job recently and didn't have time.

> - when a push is not committed because of a conflict, looking at the
> previous diff for the patch would help.  Maybe something like "stg
> show --old" ?

"stg show <patch>//top.old" should show it (well, with a bit more typing than --old).

> - the help string for push should say "patches", and possibly document
> more precisely the syntax, something like:

I plan to change the syntax of push a bit to allow things like patch1..patch2 without the --to option (the latter would still be there but taking a single patch).

> -help = 'push a patch on top of the series'
> -usage = """%prog [options] [<patch1> [<patch2>...]]
> +help = 'push patches on top of the series'
> +usage = """%prog [options] [<patch1> [<patch2>...] | -n <n> <patchroot>]

Does the <patchroot> syntax work?

Show 7 quoted lines

> - "push --undo" is not robust.  On the occasion reproduced below, I
> had to rollback the push myself by hand-modifying the stgit data,
> which took me some effort.  I'll have to gather precise info, but the
> issue occurs on patch reordering, on a genuine conflict, and seems to
> be involve a change to a non-existent file, when that file would have
> been added by a non-applied patch originally below the one I attempted
> to apply.

[...]

> ydirson$ stg push --undo
> Undoing the "patch10" push...stg push: ['git-diff-index', 'HEAD', 'path/to/the/file.java'] failed (fatal: ambiguous argument
> 'path/to/the/file.java': unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions)

I got this problem as well. StGIT needs fixing but I think a quick workaround is to create an empty file (touch patch/to/the/file.java) before the undo and git-diff-index will be happy.

Thanks for the bug reports/suggestions.

-- 
Catalin