[GUILT 00/28] Teach guilt import-commit how create legal patch names, and more

2014-03-21 Thread Per Cederqvist
I recently found myself sitting on a train with a computer in front of
me.  I tried to use guilt import-commit, which seemed to work, but
when I tried to guilt push the commits I had just imported I got
some errors.  It turned out that guilt import-commit had generated
invalid patch names.

I decided to fix the issue, and write a test case that demonstrated
the problem.

One thing led to another, and here I am, a few late nights at a hotel
and a return trip on the train later, submitting a patch series in 28
parts.  Sorry about the number of patches, but this is what happens
when you uncover a bug while writing a test case for the bug you
uncovered while writing a test case for your original problem.

The patch series consists of:

 - A number of fixes to the test suite.

 - A number of bug fixes which I hope are non-controversial.  Most of
   the fixes have test cases.

 - Changed behavior: guilt push when there is nothing more to push
   now uses exit status 1.  This makes it possible to write shell
   loops such as while guilt push; do make test||break; done.  Also,
   guilt pop when no patches are applied also uses exit status 1.
   (This aligns guilt push and guilt pop with how hg qpush and
   hg qpop has worked for several years.)

 - Changed behavior: by default, guilt no longer changes branch when
   you push a patch.  You need to do git config guilt.reusebranch
   false to re-enable that.  This patch sets the default value of
   guilt.reusebranch to true; it should in my opinion change to false
   a year or two after the next release.

A more detailed overview of the patches:

1. Some tests fail if git config guilt.diffstat true is in effect.

2-4. Some commands fail if run from a directory with spaces in its
 name.

5. guilt new had an overly restrictive argument parser.

6-8. guilt.diffstat could break guilt fold and guilt new.

9-10. The test suite did not test exit status properly.

11. Remove pointless redirections in the test suite.

12-13. guilt header tried to check if a patch existed, but the check
was broken.

14-16. Various parts of guilt tried to ensure that patch names were
   legal git branch names, but failed.

17-20. guilt graph failed when no patch was applied, and when a
   branch name contained a comma or a quote.

21-23. git config log.decorate short caused guilt import-commit,
   guilt patchbomb and guilt rebase to fail in various ways.

24. Patches may contain backslashes, but various informative messages
from guilt did backslash processing.

25-26. guilt push and guilt pop should fail when there is nothing
   to do.

27-28. These two commits were things I intended to contribute a while
   back, when contributing the Change git branch when patches are
   applied change (commit 67d3af63f422).  These commits makes
   that behavior optional, and it defaults to being disabled, so
   that you can use both Guilt v0.35 (and earlier) and the current
   Guilt code against the same repo.  These commits add some code
   complexity, and you might want to skip them if you think the
   current behavior is better.

This patch series is also available on
http://repo.or.cz/w/guilt/ceder.git in the oslo-2014 branch.  If you
already have a copy of guilt, you should be able to fetch that branch
with something like:

git remote add ceder http://repo.or.cz/r/guilt/ceder.git
git fetch ceder refs/heads/oslo-2014:refs/remotes/ceder/oslo-2014

A few of the regression/t-*.out files contain non-ASCII characters.  I
hope they survive the mail transfer; if not, please use the repo above
to fetch the commits.


Per Cederqvist (28):
  The tests should not fail if guilt.diffstat is set.
  Allow guilt delete -f to run from a dir which contains spaces.
  Added test case for guilt delete -f.
  Allow guilt import-commit to run from a dir which contains spaces.
  guilt new: Accept more than 4 arguments.
  Fix and simplify the do_get_patch function.
  Added test cases for guilt fold.
  Added more test cases for guilt new: empty patches.
  Test suite: properly check the exit status of commands.
  Run test_failed if the exit status of a test script is bad.
  test suite: remove pointless redirection.
  guilt header: more robust header selection.
  Check that guilt header '.*' fails.
  Use git check-ref-format to validate patch names.
  Produce legal patch names in guilt-import-commit.
  Fix backslash handling when creating names of imported patches.
  guilt graph no longer loops when no patches are applied.
  guilt-graph: Handle commas in branch names.
  Check that guilt graph works when working on a branch with a comma.
  guilt graph: Handle patch names containing quotes.
  The log.decorate setting should not influence import-commit.
  The log.decorate setting should not influence patchbomb.
  The log.decorate setting should not influence guilt rebase.
  disp no longer processes backslashes.
  guilt push now fails when there are no more patches to 

Re: [GUILT 00/28] Teach guilt import-commit how create legal patch names, and more

2014-03-21 Thread Jeff Sipek
On Fri, Mar 21, 2014 at 08:31:38AM +0100, Per Cederqvist wrote:
 I recently found myself sitting on a train with a computer in front of
 me.  I tried to use guilt import-commit, which seemed to work, but
 when I tried to guilt push the commits I had just imported I got
 some errors.  It turned out that guilt import-commit had generated
 invalid patch names.
 
 I decided to fix the issue, and write a test case that demonstrated
 the problem.
 
 One thing led to another, and here I am, a few late nights at a hotel
 and a return trip on the train later, submitting a patch series in 28
 parts.  Sorry about the number of patches, but this is what happens
 when you uncover a bug while writing a test case for the bug you
 uncovered while writing a test case for your original problem.

No problem.  I prefer large number of patches instead of a big wad that's
much harder to follow.

 The patch series consists of:
...
  - Changed behavior: guilt push when there is nothing more to push
now uses exit status 1.  This makes it possible to write shell
loops such as while guilt push; do make test||break; done.  Also,
guilt pop when no patches are applied also uses exit status 1.
(This aligns guilt push and guilt pop with how hg qpush and
hg qpop has worked for several years.)

Sounds fine.

  - Changed behavior: by default, guilt no longer changes branch when
you push a patch.  You need to do git config guilt.reusebranch
false to re-enable that.  This patch sets the default value of
guilt.reusebranch to true; it should in my opinion change to false
a year or two after the next release.

Probably a fair thing to do.  I should really make a release soon :/

I'm sending this off before I go through all the patches so you know that
I've seen this and plan to comment/pull.  It'll probably take a bit to go
through all 28 :)

Thanks,

Jeff.

-- 
Ready; T=0.01/0.01 08:47:23
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html