Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 15 Apr 2015 09:19:05 +0000
From:      Alexey Dokuchaev <danfe@FreeBSD.org>
To:        "Mikhail T." <mi+thun@aldan.algebra.com>
Cc:        ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org
Subject:   Re: On the "makepatch" target (Re: svn commit: r384004 - head/security/pecl-crack/files)
Message-ID:  <20150415091905.GB50397@FreeBSD.org>
In-Reply-To: <552D539D.4040800@aldan.algebra.com>
References:  <201504141624.t3EGO1xY065515@svn.freebsd.org> <20150414162951.GA10928@FreeBSD.org> <552D47CF.6090604@aldan.algebra.com> <20150414170911.GA25041@FreeBSD.org> <552D539D.4040800@aldan.algebra.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Apr 14, 2015 at 01:51:25PM -0400, Mikhail T. wrote:
> On 14.04.2015 13:09, Alexey Dokuchaev wrote:
> > The point is to avoid useless noise in the diff.  "Statistics" you're
> > talking about is pretty meaningless indeed, and since each next careless
> > submitter will probably overwrite it with their own timezone makes it
> > even more meaningless.
> 
> I just tried using the target and found, that I dislike it for the
> following reasons:
> 
>   * It retains the .orig suffix in the diff -- a completely useless 5 bytes
>     carrying no information;

There are lots of "useless" bytes all over the place.  You can get any text
file and compress it with not a single bit lost.  Seriously Mikhail, this
suffix is traditionally used, and as long as it stays the same, there is no
problem to really talk about.  In fact, most people prefer suffixed form,
and it helps to navigate large patches quickly (eye is trained to recognize
common substring), search for it, etc.

>   * It discards the timestamp of the new version of the patched file,
>     throwing out not only the timezone, but the actual time the work was
>     done;

Correct.  Keeping timestamp of the new file makes no sense -- not due to the
simple fact that every time you touch the file, mtime will be different --
but also because in the vast majority of cases "the actual time the work was
done" == time of commit with more than good enough precision.

Keeping mtime of the source file can, in fact, be helpful -- it can give you
an idea of that compilers, operating systems, C++ standards, etc. existed at
the time of work; it helps to "link" the patch to the particular version of
the code it was generated against, for example.

>   * It creates, mechanically, one patch per file -- whereas I, for one,
>     often prefer to group related changes to multiple files into a single
>     patch (makes it easier to refer upstream maintainers to the patches);

Right, this is legitimate case when makepatch does not work.  Well, it was
not designed for cumulative patches; those should be cooked manually.

>   * The new patchs' names retain extensions of the patched files, such as,
>     for example patch-crack.c. This confuses various tools, which treat
>     files based on extensions like cscope or mantis bug-tracker, which,
>     in this example, treat it like C-source, rather than a patch.

I don't see a problem here.  Mantis (and others) should be fixed to either
detect the contents or allow to specify that "files/patch-*" is a patch,
(ir)relardless of the extension.

./danfe



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150415091905.GB50397>