revised patch for gmime init, with test.

[Pieter Praet]

On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  
> wrote:
> > On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  
> > wrote:
> > > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  
> > > wrote:
> > 
> > > with differing hashes), this has the potential of causing confusion
> > > and/or quite some extra work when debugging using git-bisect(1), so
> > > I'd like to propose that bugfixes for (to-be-)released code are only
> > > applied on the 'maint' branch ('release' in the case of Notmuch),
> > > and then immediately merged back into 'master'.  In fact, this would
> > > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > > branch, to prevent issues like [1].
> > 
> > There is some merit it to this. On the other hand, it makes the history
> > messier.  [1] would have also been prevented by making the patch against
> > the right branch.
> 
> I thought about this a bit more, and I agree that at least the release
> candidates (basically anything tagged on branch release) ought to be
> merged back to master. Since any series of bugfix patches seems to be
> cause for a new release candidate, this should avoid the need to have
> doubly applied patches.
> 

Thanks!

> I'm less convinced about the need to merge every little doc change and
> debian packaging change back to master right away. This might be a
> purely aesthetic objection; [...]

See my previous reply [1].

> [...] I'm not sure if the extra merge commits
> cause any problems for e.g. bisection.
> 

Infrequent merging increases the possibility of bugs due to unforeseen
interactions between commits on different branches, which is likely to
require one to do a multitude of fake merges (`git merge --no-commit')
in order to properly track down the offending commit, so... frequent
merging would actually *prevent* issues when bisecting.


> d


Peace

-- 
Pieter

[1] id:"878vlar7ka.fsf at praet.org"

revised patch for gmime init, with test.

[Pieter Praet]

On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  
> > wrote:
> 
> > with differing hashes), this has the potential of causing confusion
> > and/or quite some extra work when debugging using git-bisect(1), so
> > I'd like to propose that bugfixes for (to-be-)released code are only
> > applied on the 'maint' branch ('release' in the case of Notmuch),
> > and then immediately merged back into 'master'.  In fact, this would
> > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > branch, to prevent issues like [1].
> 
> There is some merit it to this. On the other hand, it makes the history
> messier.  [...]

This *can* get rather messy when interlacing the 'master'/'maint' merges
with non-rebased changesets pulled from other repos (most often aren't
rebased in advance since they've already been published), which is a
common occurence in the magit [1] repo.

But take Org-mode [2] for example (which is an *extremely* active project),
where non-rebased changesets are rare(ish), and where the 'maint' branch
is constantly being merged back into 'master', resulting in a very clean
ladder-like commit log, eg. :

  --******---**---*---*--- master
 \ / / \
  *---*---*-*---*--- maint
 0.11   bugfix   NEWSbugfix   0.12~rc1


> [...] [1] would have also been prevented by making the patch against
> the right branch.
> 

True, but if we can obviate the need to check if we're on the right
branch altogether, without causing any unwanted side-effects, wouldn't
it be counterproductive not to?


Peace

-- 
Pieter

[1] git://github.com/magit/magit.git
[2] git://orgmode.org/org-mode.git

Re: revised patch for gmime init, with test.

[Pieter Praet]

On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner da...@tethera.net wrote:
 On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet pie...@praet.org wrote:
  On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner da...@tethera.net wrote:
 
  with differing hashes), this has the potential of causing confusion
  and/or quite some extra work when debugging using git-bisect(1), so
  I'd like to propose that bugfixes for (to-be-)released code are only
  applied on the 'maint' branch ('release' in the case of Notmuch),
  and then immediately merged back into 'master'.  In fact, this would
  preferrably happen after *every* (series of) commit(s) on the 'maint'
  branch, to prevent issues like [1].
 
 There is some merit it to this. On the other hand, it makes the history
 messier.  [...]

This *can* get rather messy when interlacing the 'master'/'maint' merges
with non-rebased changesets pulled from other repos (most often aren't
rebased in advance since they've already been published), which is a
common occurence in the magit [1] repo.

But take Org-mode [2] for example (which is an *extremely* active project),
where non-rebased changesets are rare(ish), and where the 'maint' branch
is constantly being merged back into 'master', resulting in a very clean
ladder-like commit log, eg. :

  --******---**---*---*--- master
 \ / / \
  *---*---*-*---*--- maint
 0.11   bugfix   NEWSbugfix   0.12~rc1


 [...] [1] would have also been prevented by making the patch against
 the right branch.
 

True, but if we can obviate the need to check if we're on the right
branch altogether, without causing any unwanted side-effects, wouldn't
it be counterproductive not to?


Peace

-- 
Pieter

[1] git://github.com/magit/magit.git
[2] git://orgmode.org/org-mode.git
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: revised patch for gmime init, with test.

[Pieter Praet]

On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner da...@tethera.net wrote:
 On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner da...@tethera.net wrote:
  On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet pie...@praet.org wrote:
   On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner da...@tethera.net 
   wrote:
  
   with differing hashes), this has the potential of causing confusion
   and/or quite some extra work when debugging using git-bisect(1), so
   I'd like to propose that bugfixes for (to-be-)released code are only
   applied on the 'maint' branch ('release' in the case of Notmuch),
   and then immediately merged back into 'master'.  In fact, this would
   preferrably happen after *every* (series of) commit(s) on the 'maint'
   branch, to prevent issues like [1].
  
  There is some merit it to this. On the other hand, it makes the history
  messier.  [1] would have also been prevented by making the patch against
  the right branch.
 
 I thought about this a bit more, and I agree that at least the release
 candidates (basically anything tagged on branch release) ought to be
 merged back to master. Since any series of bugfix patches seems to be
 cause for a new release candidate, this should avoid the need to have
 doubly applied patches.
 

Thanks!

 I'm less convinced about the need to merge every little doc change and
 debian packaging change back to master right away. This might be a
 purely aesthetic objection; [...]

See my previous reply [1].

 [...] I'm not sure if the extra merge commits
 cause any problems for e.g. bisection.
 

Infrequent merging increases the possibility of bugs due to unforeseen
interactions between commits on different branches, which is likely to
require one to do a multitude of fake merges (`git merge --no-commit')
in order to properly track down the offending commit, so... frequent
merging would actually *prevent* issues when bisecting.


 d


Peace

-- 
Pieter

[1] id:878vlar7ka@praet.org
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

revised patch for gmime init, with test.

[David Bremner]

On Fri, 13 Jan 2012 12:52:48 -0800, Jameson Graef Rollins  wrote:
> 
> Doesn't everything need to be merged into master eventually anyway?  It
> seems to me that unless it's a change that very narrowly targeting an
> issue in a release branch that is not an issue in master, every patch
> will ultimately need to be applied to both.  It doesn't really make
> sense to me to apply a change to one branch and not the other, if they
> will eventually need to be applied to both anyway.

The following two sequences of commands apply the same changes, but
result in a different history graph.

1) notmuch checkout release && git am patch  && \
   notmuch checkout master && git cherry-pick release

2) notmuch checkout release && git am patch  && \
   notmuch checkout master && git merge release

well, they apply the same changes if release was an ancestor of master
when the they both began.

revised patch for gmime init, with test.

[Jameson Graef Rollins]

On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner  wrote:
> I thought about this a bit more, and I agree that at least the release
> candidates (basically anything tagged on branch release) ought to be
> merged back to master. Since any series of bugfix patches seems to be
> cause for a new release candidate, this should avoid the need to have
> doubly applied patches.
> 
> I'm less convinced about the need to merge every little doc change and
> debian packaging change back to master right away. This might be a
> purely aesthetic objection; I'm not sure if the extra merge commits
> cause any problems for e.g. bisection.

Doesn't everything need to be merged into master eventually anyway?  It
seems to me that unless it's a change that very narrowly targeting an
issue in a release branch that is not an issue in master, every patch
will ultimately need to be applied to both.  It doesn't really make
sense to me to apply a change to one branch and not the other, if they
will eventually need to be applied to both anyway.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

revised patch for gmime init, with test.

[David Bremner]

On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  
> > wrote:
> 
> > with differing hashes), this has the potential of causing confusion
> > and/or quite some extra work when debugging using git-bisect(1), so
> > I'd like to propose that bugfixes for (to-be-)released code are only
> > applied on the 'maint' branch ('release' in the case of Notmuch),
> > and then immediately merged back into 'master'.  In fact, this would
> > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > branch, to prevent issues like [1].
> 
> There is some merit it to this. On the other hand, it makes the history
> messier.  [1] would have also been prevented by making the patch against
> the right branch.

I thought about this a bit more, and I agree that at least the release
candidates (basically anything tagged on branch release) ought to be
merged back to master. Since any series of bugfix patches seems to be
cause for a new release candidate, this should avoid the need to have
doubly applied patches.

I'm less convinced about the need to merge every little doc change and
debian packaging change back to master right away. This might be a
purely aesthetic objection; I'm not sure if the extra merge commits
cause any problems for e.g. bisection.

d

Re: revised patch for gmime init, with test.

[David Bremner]

On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner da...@tethera.net wrote:
 On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet pie...@praet.org wrote:
  On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner da...@tethera.net wrote:
 
  with differing hashes), this has the potential of causing confusion
  and/or quite some extra work when debugging using git-bisect(1), so
  I'd like to propose that bugfixes for (to-be-)released code are only
  applied on the 'maint' branch ('release' in the case of Notmuch),
  and then immediately merged back into 'master'.  In fact, this would
  preferrably happen after *every* (series of) commit(s) on the 'maint'
  branch, to prevent issues like [1].
 
 There is some merit it to this. On the other hand, it makes the history
 messier.  [1] would have also been prevented by making the patch against
 the right branch.

I thought about this a bit more, and I agree that at least the release
candidates (basically anything tagged on branch release) ought to be
merged back to master. Since any series of bugfix patches seems to be
cause for a new release candidate, this should avoid the need to have
doubly applied patches.

I'm less convinced about the need to merge every little doc change and
debian packaging change back to master right away. This might be a
purely aesthetic objection; I'm not sure if the extra merge commits
cause any problems for e.g. bisection.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: revised patch for gmime init, with test.

[Jameson Graef Rollins]

On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner da...@tethera.net wrote:
 I thought about this a bit more, and I agree that at least the release
 candidates (basically anything tagged on branch release) ought to be
 merged back to master. Since any series of bugfix patches seems to be
 cause for a new release candidate, this should avoid the need to have
 doubly applied patches.
 
 I'm less convinced about the need to merge every little doc change and
 debian packaging change back to master right away. This might be a
 purely aesthetic objection; I'm not sure if the extra merge commits
 cause any problems for e.g. bisection.

Doesn't everything need to be merged into master eventually anyway?  It
seems to me that unless it's a change that very narrowly targeting an
issue in a release branch that is not an issue in master, every patch
will ultimately need to be applied to both.  It doesn't really make
sense to me to apply a change to one branch and not the other, if they
will eventually need to be applied to both anyway.

jamie.


pgpzHyGS32K3U.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: revised patch for gmime init, with test.

[David Bremner]

On Fri, 13 Jan 2012 12:52:48 -0800, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 
 Doesn't everything need to be merged into master eventually anyway?  It
 seems to me that unless it's a change that very narrowly targeting an
 issue in a release branch that is not an issue in master, every patch
 will ultimately need to be applied to both.  It doesn't really make
 sense to me to apply a change to one branch and not the other, if they
 will eventually need to be applied to both anyway.

The following two sequences of commands apply the same changes, but
result in a different history graph.

1) notmuch checkout release  git am patch   \
   notmuch checkout master  git cherry-pick release

2) notmuch checkout release  git am patch   \
   notmuch checkout master  git merge release

well, they apply the same changes if release was an ancestor of master
when the they both began.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

revised patch for gmime init, with test.

[David Bremner]

On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  
> wrote:

> with differing hashes), this has the potential of causing confusion
> and/or quite some extra work when debugging using git-bisect(1), so
> I'd like to propose that bugfixes for (to-be-)released code are only
> applied on the 'maint' branch ('release' in the case of Notmuch),
> and then immediately merged back into 'master'.  In fact, this would
> preferrably happen after *every* (series of) commit(s) on the 'maint'
> branch, to prevent issues like [1].

There is some merit it to this. On the other hand, it makes the history
messier.  [1] would have also been prevented by making the patch against
the right branch.

revised patch for gmime init, with test.

[Pieter Praet]

On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  wrote:
> It turns out that our existing (trivial) python test is enough to
> catch this bug, but the corpus needs to be augmented.  This
> augmentation is a bit intrusive so I'm thinking of cherry-picking only
> the actual fix to the release branch.
> 

Due to the ~same change being applied in multiple places (and thus
with differing hashes), this has the potential of causing confusion
and/or quite some extra work when debugging using git-bisect(1), so
I'd like to propose that bugfixes for (to-be-)released code are only
applied on the 'maint' branch ('release' in the case of Notmuch),
and then immediately merged back into 'master'.  In fact, this would
preferrably happen after *every* (series of) commit(s) on the 'maint'
branch, to prevent issues like [1].


In this specific case (g_mime_init fix), it would have sufficed to
apply the test suite augmentation patches on 'master' and the bugfix
only on 'maint', merging 'maint' into 'master', and then removing
the "test_subtest_known_broken" line in a followup commit on 'master'.

Thus, maintainers would have fewer merge conflicts to resolve,
developers would at all times benefit from release-specific fixes,
and (pre-)release users would have clear indication of what was fixed
due to the test suite reporting "FIXED" instead of "PASS".


Thanks!

> Unfortunately the test message is 8 bit, so it may be encoded in some
> inconvenient way for patch application. The message is attached to an
> earlier message in the thread if you want to double check.
> 
> I also wondered about putting g_type_init inside the (!initialized)
> test, but decided against it on the grounds of minimality.
> 
> I think we want to in the medium term factor out all of the
> initialization code into one (probably private) function; we can clean
> things up a bit more then.
> 
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

[1] id:"87mxa95u3d.fsf at zancas.localnet"

Re: revised patch for gmime init, with test.

[David Bremner]

On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet pie...@praet.org wrote:
 On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner da...@tethera.net wrote:

 with differing hashes), this has the potential of causing confusion
 and/or quite some extra work when debugging using git-bisect(1), so
 I'd like to propose that bugfixes for (to-be-)released code are only
 applied on the 'maint' branch ('release' in the case of Notmuch),
 and then immediately merged back into 'master'.  In fact, this would
 preferrably happen after *every* (series of) commit(s) on the 'maint'
 branch, to prevent issues like [1].

There is some merit it to this. On the other hand, it makes the history
messier.  [1] would have also been prevented by making the patch against
the right branch.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

revised patch for gmime init, with test.

[David Bremner]

It turns out that our existing (trivial) python test is enough to
catch this bug, but the corpus needs to be augmented.  This
augmentation is a bit intrusive so I'm thinking of cherry-picking only
the actual fix to the release branch.

Unfortunately the test message is 8 bit, so it may be encoded in some
inconvenient way for patch application. The message is attached to an
earlier message in the thread if you want to double check.

I also wondered about putting g_type_init inside the (!initialized)
test, but decided against it on the grounds of minimality.

I think we want to in the medium term factor out all of the
initialization code into one (probably private) function; we can clean
things up a bit more then.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

revised patch for gmime init, with test.

[David Bremner]

It turns out that our existing (trivial) python test is enough to
catch this bug, but the corpus needs to be augmented.  This
augmentation is a bit intrusive so I'm thinking of cherry-picking only
the actual fix to the release branch.

Unfortunately the test message is 8 bit, so it may be encoded in some
inconvenient way for patch application. The message is attached to an
earlier message in the thread if you want to double check.

I also wondered about putting g_type_init inside the (!initialized)
test, but decided against it on the grounds of minimality.

I think we want to in the medium term factor out all of the
initialization code into one (probably private) function; we can clean
things up a bit more then.