Re: [gentoo-pms] Re: [PATCH] Explain eapply behavior for EAPI 6

[Michał Górny]

Dnia 2015-10-14, o godz. 20:24:22
Ulrich Mueller <ulm@g.o> napisał(a):

> >>>>> 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.

Sure, updated.

> > +	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.

Corrected.

>
> >      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:

Not sure about this. But maybe it makes sense if we can get the right
wording...

>     IF first parameter begins with a hyphen AND any parameter is equal to --

Hmm, that's not exactly correct. All parameters preceding -- would have
to begin with a hyphen. So maybe...

      IF any parameter is equal to -- AND all parameters preceding it
        being with a hyphen

>         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

Not sure if we should really specify error handling here. The idea is
that this is incorrect syntax, so it is unconditional fatal 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

Ok.

>
> > +			\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)?

Where should I say it? Put in the algorithm?

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>