Re: pkgtools/pkgdiff: mkpatches, ignore time stamp for unchanged patches

[Jonathan Perkin <jperkin%joyent.com@localhost>]

* On 2015-06-10 at 01:26 BST, Greg Troxel wrote:

> Jonathan Perkin <jperkin%joyent.com@localhost> writes:
> 
> >> I think it would be good if mkpatches declined to update patchfiles if
> >> they weren't actually meaningfully different, at least by default.  I
> >> find it annoying when I run mkpatches and e.g. see 12 .orig files, when
> >> examination shows that really there's just the new patch I added and 3
> >> that were defuzzed, with 8 unchanged.
> >> 
> >> I guess this comes down to suggesting that mkpatches be "mkpatches;
> >> patchdiff".  Does anyone ever want to not have this behavior?
> >
> > Me.  In my opinion patches should always be appropriate to the version
> > currently in pkgsrc, and it should be up to the person who updates the
> > package version to ensure that they are, rather than relying on fuzz.
> >
> > This helps a great deal when fixing packages without bumping the
> > version.  It means that the diffs resulting from mkpatches are only
> > those which are caused by your change, and that makes it much easier
> > to verify that you aren't accidentally changing something else.
> 
> I think there are multiple cases, and we may be talking about different
> things (due to me being unclear):
> 
>   a: patch is actually different
> 
>   b: defuzz only (with possible datestamp change)
> 
>   c: change of datestamps only
> 
>   d: no actual change (patch-foo and patch-foo.orig are identical)
> 
> 
> I don't want mkpatches to ever change patch files for category d.  Right
> now I get the original patchfile moved to patch-foo.orig and a
> byte-for-byte-identical patch-foo witha new mod date.  That's just
> noise/mess.

I agree here, it would be good for mkpatches to restore the original
mtime in this case, and this should be a trivial change.

> For category c, I also don't want a change.  I don't know why anyone
> would.

Actually I would.  Why?  Because it's correct.  I want our patches to
reflect the current package version, not some version in the past.

When updating foo from 1.1 to 1.2 the patches should all be updated to
be correct for 1.2, not left in a state where some are accurate for
1.2, and some are accurate for 1.1 but still apply to 1.2.

I guess we're basically disagreeing on whether this is something we
should fix technically rather than socially.  I'd rather the latter,
primarily because I prefer correctness and dislike magic workarounds
that encourage pretending we're still on foo-1.1, but also because if
we promote a standard way of performing updates, we can help avoid
other mistakes such as patches which are generated after 'bmake subst'
and expand @PREFIX@ -> /usr/pkg, something I've seen a bit recently.

> For category a, a change is necessary.
> 
> So the only interesting case is b, and I can see reasons to do it either
> way sometimes, as you say.
> 
> So if
> 
>   mkpatches --no-defuzz
> 
> changed category a only, and
> 
>   mkpatches
> 
> changed category a and b, and
> 
>   mkpatches --regenerate-timestamps
> 
> did a, b and c, would we all be happy?

I dislike adding more options, it's more things that new pkgsrc
contributors have to learn, and it encourages incorrect patches.  I'd
rather that the single

  $ bmake patch; mkpatches; bmake mps; mkpatches -c

workflow worked correctly in all cases and that we advocate its
universal use (and/or a wrapper script or make target which does it
all for us).  I don't want to have to start using extra options
depending on whether I'm in package update (--regenerate-timestamps)
or build fix (--no-defuzz) mode.

I think we're mostly there already, except for fixing case 'd' above,
but would like to see additional checks in mkpatches too, e.g.
increasing modularity of pkglint and pulling in the patch checks when
running 'mkpatches' which would catch the aforementioned @PREFIX@ ->
/usr/pkg mistakes.

-- 
Jonathan Perkin  -  Joyent, Inc.  -  www.joyent.com