Date: Thu, 30 Apr 2020 10:28:23 +0200
From: Natanael Copa <ncopa@alpinelinux.org>
To: Rasmus Thomsen <oss@cogitri.dev>
Cc: ~alpine/devel@lists.alpinelinux.org
Subject: Re: Enforcing patch headers for patches
Message-ID: <20200430102823.11d1ed66@ncopa-desktop.copa.dup.pw>
In-Reply-To: <4ac4d3aad3bf0f050070166b27a2344efe32e9b7.camel@cogitri.dev>
References: <4ac4d3aad3bf0f050070166b27a2344efe32e9b7.camel@cogitri.dev>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Mon, 27 Apr 2020 12:23:43 +0200
Rasmus Thomsen <oss@cogitri.dev> wrote:

> Hello,
> 
> I'd like to propose that we start enforcing patch headers for new
> patches in aports, to make the flood of (downstream) patches more
> manageable. With a patch header a patch would look like this:
> 
> $ cat xfce-screenshooter-1.7.9-dsofix.patch
> Upstream: Yes
> Reason: Without this patch linking with bfd fails due to missing linker
> flags, as these libraries are implicitly required.
> Url: https://bugzilla.xfce.org/show_bug.cgi?id=7985
> --- xfce4-screenshooter-1.7.9.orig/Makefile.in	2010-02-07
> 14:45:15.000000000 +0100
> +++ xfce4-screenshooter-1.7.9/Makefile.in	2010-02-16
> 23:57:31.000000000 +0100
> @@ -282,7 +282,7 @@
>  INTLTOOL_PERL = @INTLTOOL_PERL@
>  INTLTOOL_UPDATE = @INTLTOOL_UPDATE@
>  LD = @LD@
> -LDFLAGS = @LDFLAGS@
> +LDFLAGS = @LDFLAGS@ -lm -lX11
>  LIBOBJS = @LIBOBJS@
>  LIBS = @LIBS@
>  LIBTOOL = @LIBTOOL@
> 
> Let me explain the purpose of the different fields:
> 
> * Upstream: This should indicate whether the patch is proposed
> upstream/already merged or if it's specific to us. Should be "Yes" if
> it has been proposed/merged already, or "No" with an explanation why it
> can't be upstreamed. This helps tracking the status of a patch
> upstream. Mustn't be omitted.

What should we set it to if the patch is suitable for upstreaming but
not yet submitted/reported upstream?

What if we are unsure if its suitable for upstreaming?

What happens if it was proposed upstream but later rejected or
reverted (eg. upstream finds ou they don't want support musl at all)

What if we lift patches from debian or fedora and they have different
syntax for similar do we need to convert it to our format?

I am afraid of needing to change the upstreaming state of the patch
when it changes or that the specified upstreaming state information
getting outdated. This info becomes useless if it cannot be trusted.

May be good to have some indication for patches that are not
upstreamable and alpine specific. Maybe this is what you mean with
"Upstream: no"

> * Reason: This describes what the patch is for. That makes it easier
> for others (or sometimes yourself at a later point) to figure out what
> a patch actually does. This field may be omitted if the patch is
> trivial and has a descriptive file name, or if the patch already has a
> commit message in it (e.g. from git format-patch).
> 
> * Url: An URL to where upstream deals with the issue, e.g. a link to
> upstream's issue report in their bugtracker or a link to a merge
> request. This may only be omitted if Upstream is "No" (meaning the
> patch is specific to Alpine Linux). This has the nice consequence that
> all patches that aren't specific to Alpine Linux have to be submitted
> to upstream, or the issue the patch tries to solve at least has to be
> reported at upstream. This lessens the maintenance burden on us (since
> we won't have to maintain as many downstream patches then) and makes
> the patches available for other developers as well.

There may be various different URLs. Where the patch comes from, URL to
bug tracker, URL to other distros bugtracker or mailing list
discussions.

> 
> While these patch header seem like a lot of boilerplate up front, it
> really does pay off in the long run. It probably takes less than a
> minute to write a patch header for the submitter of the patch since
> all of the required info is already readily available to the
> submitter: They know what the patch is for, what the upstream status
> is etc. It might take hours for other people who find out what a
> patch is for and whether it has been proposed upstream already at a
> later point.

I agree that having some reference where the patch comes from, and
where to find upstream status is useful. I try (tried?) add that to
patches whenever I can.

> 
> Regards,
> 
> Rasmus