public inbox for gentoo-pms@lists.gentoo.org
 help / color / mirror / Atom feed
From: Ulrich Mueller <ulm@gentoo.org>
To: "Michał Górny" <mgorny@gentoo.org>
Cc: gentoo-pms@lists.gentoo.org
Subject: [gentoo-pms] Re: [PATCH] Explain eapply behavior for EAPI 6
Date: Wed, 14 Oct 2015 20:24:22 +0200	[thread overview]
Message-ID: <22046.40406.63642.760729@a1i15.kph.uni-mainz.de> (raw)
In-Reply-To: <1444841121-2541-1-git-send-email-mgorny@gentoo.org>

[-- Attachment #1: Type: text/plain, Size: 3969 bytes --]

>>>>> On Wed, 14 Oct 2015, Michał Górny wrote:

> ---
>  pkg-mgr-commands.tex | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)

> // I'm aware this isn't perfect but I tried to keep the algorithm
> // readable and reasonably concise. Hope it's good enough for a start
> // though.

Thanks, this will indeed serve well as a start.

> diff --git a/pkg-mgr-commands.tex b/pkg-mgr-commands.tex
> index dbdbdfd..1d57d63 100644
> --- a/pkg-mgr-commands.tex
> +++ b/pkg-mgr-commands.tex
> @@ -148,10 +148,12 @@ These commands are used during the \t{src\_prepare} phase to apply patches to th
>  Ebuilds must not run any of these commands once the current phase function has returned.
>   
>  \begin{description}
> -\item[eapply] \featurelabel{eapply}
> -    Only available in EAPIs listed in table~\ref{tab:patch-commands} as supporting \t{eapply}.
> +\item[eapply] \featurelabel{eapply} Takes zero or more patch(1) options, followed by one or more

Should we say "GNU patch" here? We require it in ebuild-env-commands.tex.

> +	file or directory paths. Processes options and applies all patches found in specified locations
> +	according to Algorithm~\ref{alg:eapply}. Only available in EAPIs listed in
> +	table~\ref{tab:patch-commands} as supporting \t{eapply}.
>  

> -\item[eapply\_user] \featurelabel{eapply-user} Takes no arguments. Package managers supporting it
> +\item[eapply\_user] \featurelabel{eapply-user} Takes no arguments. Package managers supporting i

Slip of fingers.

>      apply user-provided patches to the source tree in the current working directory. Exact behaviour
>      is implementation defined and beyond the scope of this specification. Package managers not
>      supporting it must implement the function as a no-op. Only available in EAPIs listed in
> @@ -161,7 +163,29 @@ Ebuilds must not run any of these commands once the current phase function has r
>  \begin{algorithm}
>  \caption{eapply logic} \label{alg:eapply}
>  \begin{algorithmic}[1]
> -\STATE \COMMENT{WORK IN PROGRESS}
> +\STATE let options=()
> +\STATE let infiles=FALSE
> +\FOR{\$x in parameters}
> +	\IF{infiles is FALSE}
> +		\IF{\$x is "\t{-{}-}"}
> +			\STATE let infiles=TRUE
> +		\ELSIF{\$x starts with "\t{-}"}
> +			\STATE options+=( "\$x" )
> +		\ELSE
> +			\STATE let infiles=TRUE
> +		\ENDIF
> +	\ENDIF
> +
> +	\IF{infiles is TRUE}

I wonder if it wouldn't be cleaner to do the whole option parsing
first. Something along the lines of this:

    IF first parameter begins with a hyphen AND any parameter is equal to --
        collect all parameters before the first -- in the options array
        collect all parameters after the first -- in the files array
    ELSIF any parameter beginning with a hyphen follows one without a hyphen
        return an error
    ELSE
        collect all parameters starting with a hyphen in the options array
        collect all parameters without a hyphen in the files array
    ENDIF

    FOR x in files
        ...

> +		\IF{\$x is a directory}
> +			\FOR{\$f in all files matching \$x/*.diff and \$x/*.patch}
> +				\STATE call \t{patch -p1 -f -s "\$\{options[@]\}" "\$f"}

Options "-p1 -g0 -E --no-backup-if-mismatch" are field-proven in
eutils.eclass. This doesn't mean that we have to adhere to it
slavishly, but I would suggest that we keep both the -g0 and the
--no-backup-if-mismatch options (but drop -E). Also I would omit
the -s; it is up to the PM if it wants to suppress or filter output.
So in summary, that is:

patch -p1 -f -g0 --no-backup-if-mismatch

> +			\ENDFOR
> +		\ELSE
> +			\STATE call \t{patch -p1 -f -s "\$\{options[@]\}" "\$x"}
> +		\ENDIF
> +	\ENDIF
> +\ENDFOR
>  \end{algorithmic}
>  \end{algorithm}

We should say something about error handling. For example, does the
function return when it encounters the first error of a called
patch(1)?

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

  reply	other threads:[~2015-10-14 18:24 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-14 16:45 [gentoo-pms] [PATCH] Explain eapply behavior for EAPI 6 Michał Górny
2015-10-14 18:24 ` Ulrich Mueller [this message]
2015-10-14 18:35   ` [gentoo-pms] " Michał Górny
2015-10-14 18:52     ` Ulrich Mueller
2015-10-14 19:05       ` Michał Górny
2015-10-14 19:23 ` [gentoo-pms] [PATCH v2] " Michał Górny
2015-10-14 19:40   ` [gentoo-pms] " Ulrich Mueller
2015-10-14 19:55     ` Michał Górny
2015-10-14 20:10       ` Ulrich Mueller
2015-10-14 20:19         ` Michał Górny
2015-10-14 20:19 ` [gentoo-pms] [PATCH v3] " Michał Górny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22046.40406.63642.760729@a1i15.kph.uni-mainz.de \
    --to=ulm@gentoo.org \
    --cc=gentoo-pms@lists.gentoo.org \
    --cc=mgorny@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox