Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Kent Fredric]

On Wed, 21 Sep 2016 08:44:33 +0200
Ulrich Mueller  wrote:

> Obviously they could be identified by their explicit ${FILESDIR}.

My question is because I've had a difficulty understanding how repoman
"sees" the ebuild.

I was under the impression repoman would either have to rely on people to be 
good
programmers and not do tricks like ${FILESDIR}/${MY_PN}-${PATCHLEVEL}.patch

( so that it could verify the part after the explicit filesdir variable
), or it would have to source the ebuild and expand variable
interpolations, and have some way of knowing *postinterpolation* what
the variables expanded to.

Given I think programmers *should* be allowed to use variables other
than "${FILESDIR}" in $PATCHES, it seems only reasonable that repoman
would need a post-interpolation way of checking they're all accounted
for.

Partly, because a certain usecase that would be good to guard against
is $PATCHES contains ${PV}, and the ebuild is bumped, and nobody
notices there is a patch that also needs bumping to keep that working.

Ideally committers should detect this when they compile their package
and the package explodes in src_prepare, but we do have a few cases
where people skip that stage and there's no safeguard, and autorepoman
can't even warn anyone that's happened as it doesn't poke into $PATCHES

( at least, irrc )



pgpY7MLGnbCpO.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Ulrich Mueller]

> On Wed, 21 Sep 2016, Kent Fredric wrote:

>> Under that new scheme, how would I apply patches unpacked into
>> WORKDIR? In EAPI 6, I can put them into the PATCHES variable and use
>> the default src_prepare to process it.

> oh. Right. Huh, I had somehow overlooked there was already a PATCHES
> in EAPI6, because I'd been using an equivalent since EAPI5 :)

> Right, so this would kill 90% of my suggestion by
> already-being-defined.

> So the only remaining thing to think about in terms of usefulness is
> the ability for repoman to potentially validate entries in PATCHES.

> In your scenario where you have local patches instead of patches in
> ${FILESDIR}, what does the contents of PATCHES look like?

Well, it could contain entries for both types of paths, like
"${WORKDIR}"/path/to/foo.patch or "${FILESDIR}"/path/to/bar.patch.

> Would there be a way to identify entries in PATCHES that are
> children of ${FILESDIR} during `repoman full` and assert they exist?

Obviously they could be identified by their explicit ${FILESDIR}.

Ulrich


pgpcMa4FqOQgk.pgp
Description: PGP signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Kent Fredric]

On Tue, 20 Sep 2016 18:14:58 +0200
Ulrich Mueller  wrote:

> Under that new scheme, how would I apply patches unpacked into
> WORKDIR? In EAPI 6, I can put them into the PATCHES variable and use
> the default src_prepare to process it.

oh. Right. Huh, I had somehow overlooked there was already a PATCHES in EAPI6,
because I'd been using an equivalent since EAPI5 :)

Right, so this would kill 90% of my suggestion by already-being-defined.

So the only remaining thing to think about in terms of usefulness is the ability
for repoman to potentially validate entries in PATCHES.

In your scenario where you have local patches instead of patches in 
${FILESDIR}, what
does the contents of PATCHES look like?

Would there be a way to identify entries in PATCHES that are children of 
${FILESDIR}
during `repoman full` and assert they exist?


pgpNXTJSX7cbP.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Kent Fredric]

On Tue, 20 Sep 2016 18:14:58 +0200
Ulrich Mueller  wrote:

> Under that new scheme, how would I apply patches unpacked into
> WORKDIR? In EAPI 6, I can put them into the PATCHES variable and use
> the default src_prepare to process it.

Can you give me a clearer example of what you mean?



pgpPMY7o5g8T4.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Ulrich Mueller]

> On Tue, 20 Sep 2016, Kent Fredric wrote:

> So a reduced suggestion would be:

> 1. Add a PATCHES var to EAPI7
> 2. PATCHES is analogous to SRC_URI, a string
> 3. PATCHES supports USE conditionals

I am not convinced that USE-conditional patching should be encouraged.
Because so far the policy was that whenever possible, patches should
be applied unconditionally and any conditionals should be configure
options. So that patches could eventually be sent upstream.

It is somewhat similar to epatch's arch conditional patching (as in
10_sparc_foo.patch) which for the same reason was labelled as "highly
discouraged" in 2010. Also that feature wasn't added to eapply.

> 4. PATCHES is ordered.
> 5. ${FILESDIR} works as it always did
> 6. PATCHES describes components of FILESDIR
> 7. PATCHES is evaluated in terms of USE to yield EFFECTIVE_PATCHES, an array
> 8. Default src_prepare applied eapply to all values in EFFECTIVE_PATCHES
> 9. Items in PATCHES are checked by repoman "they must exist"

Under that new scheme, how would I apply patches unpacked into
WORKDIR? In EAPI 6, I can put them into the PATCHES variable and use
the default src_prepare to process it.

> The difference between this suggestion and the previous one importantly
> is it can't be used to whitelist, it can only be used to 

> 1. Simplify how patches are applied for people who want it
> 2. Add extra assurance to repoman if people use the feature

> Neither of these are exactly "Compelling" features, but it would
> in part reduce the work. It would also avoid double-bookkeeping
> by adding no features that require double-bookkeeping, and would simply be a 
> generalization/enhancement of similar features already found in ebuilds.

Ulrich


pgp50ZJoj5EVF.pgp
Description: PGP signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Kent Fredric]

On Tue, 20 Sep 2016 09:02:41 +0200
Michał Górny  wrote:

> Finally, the same though occurred to me as to ulm. People will forget
> to update the variable. They will forget to update it when adding,
> and they will waste their time on build that is going to fail somewhere
> at doinit in the end -- how nice is that? Then, they will forget to
> update it when removing and the file will be kept stale.
> 
> Any solution you can try to invent to solve those problems, or make
> the system any more friendly, is actually making it more complex,
> harder to comprehend and even more costly.

Yeah, I'd probably have to scrap most of my suggestions and restrict the design
to something that was only an "optional extra" in order to be useful.

If I was to re-do the concept, it would be mostly a hashover of what we use in 
Perl
for patches.

So a reduced suggestion would be:

1. Add a PATCHES var to EAPI7
2. PATCHES is analogous to SRC_URI, a string
3. PATCHES supports USE conditionals
4. PATCHES is ordered.
5. ${FILESDIR} works as it always did
6. PATCHES describes components of FILESDIR
7. PATCHES is evaluated in terms of USE to yield EFFECTIVE_PATCHES, an array
8. Default src_prepare applied eapply to all values in EFFECTIVE_PATCHES
9. Items in PATCHES are checked by repoman "they must exist"

The difference between this suggestion and the previous one importantly
is it can't be used to whitelist, it can only be used to 

1. Simplify how patches are applied for people who want it
2. Add extra assurance to repoman if people use the feature

Neither of these are exactly "Compelling" features, but it would
in part reduce the work. It would also avoid double-bookkeeping
by adding no features that require double-bookkeeping, and would simply be a 
generalization/enhancement of similar features already found in ebuilds.

Anything that needed something more fancy than PATCHES would sill be able
to and do its own stuff, and it would simply not get any benefits of repoman
side consistency checks.

And people who needed extra magic could also do:


src_prepare() { 
# extra stuff
default; # does eapply EFFECTIVE_PATCHES && eapply_user
}




pgpVONcynPAL_.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Michał Górny]

On Sat, 17 Sep 2016 00:30:31 +1200
Kent Fredric  wrote:

> Just an idea that seemed obvious enough and obviously missing.

Sounds like a great way to discourage people from contributing even
further. I'm going to say that developers leaving mess in their
FILESDIR is rather a pathological case, so why does everyone need to
suffer for their carelessness?

And even then, stale files are a minor nuisance compared to the cost of
maintaining redundant list of all files. Not to mention that the size
of the lists will easily exceed size of the stale patches.

Then, some eclasses access FILESDIR directly, and do magic if some
files exist in there. Not saying it's anywhere close to sanity -- just
that people do it.

Finally, the same though occurred to me as to ulm. People will forget
to update the variable. They will forget to update it when adding,
and they will waste their time on build that is going to fail somewhere
at doinit in the end -- how nice is that? Then, they will forget to
update it when removing and the file will be kept stale.

Any solution you can try to invent to solve those problems, or make
the system any more friendly, is actually making it more complex,
harder to comprehend and even more costly.

-- 
Best regards,
Michał Górny



pgpae0cEEM6s6.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Kent Fredric]

On Fri, 16 Sep 2016 20:34:49 +0200
Ulrich Mueller  wrote:

> That is, similar syntax as for SRC_URI? That would make parsing of the
> FILES variable by ebuilds practically impossible. So presumably, one
> would need another variable similar to A then, containing the files
> from FILES but without the USE-disabled ones.

Yeah, that's the sort of misgivings I had when I thought of a similar
idea myself.

I'd much rather a way to label groups of files by purpose, have portage
validate them, and then apply certain labels manually against certain
USE flags

For instance,

   FILES["foo"] = "foo.patch bar.patch"
   FILES["quux"] = "quux.patch"

...

   if use foo; then
 eapply $FILES["foo"]
   else
eapply $FILES["quux"]
   fi

But that gets into things that are too hard to do in bash, like named
arrays ( doable but unfamilar for most ) and arrays-in-named-arrays
( ummm )

Probably not a good idea to push the limitations of bash in this way.


pgpgCzd_2tn71.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Ulrich Mueller]

> On Fri, 16 Sep 2016, Zac Medico wrote:

> If FILES supports USE conditionals,

That is, similar syntax as for SRC_URI? That would make parsing of the
FILES variable by ebuilds practically impossible. So presumably, one
would need another variable similar to A then, containing the files
from FILES but without the USE-disabled ones.

> then it's possible to use inotify to implement a QA check that
> detects unused files in FILES.

Ulrich


pgpuaunFS5UHR.pgp
Description: PGP signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Ian Stakenvicius]

On 16/09/16 11:20 AM, Ulrich Mueller wrote:
> 
> AFAICS that proposal goes into a direction which is somewhat opposite
> to what we have pursued in EAPI 6. There, we have allowed directories
> as arguments to eapply, in order to simplify application of patchsets.
> Now maintainers would have to list all single files contained in the
> directory again.

I had thought that particular functionality was more to support
patchset tarballs that get extracted to ${WORKDIR} rather than
supporting such (ab)use in ${FILESDIR} ?





signature.asc
Description: OpenPGP digital signature

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Zac Medico]

On 09/16/2016 08:20 AM, Ulrich Mueller wrote:
>> On Sat, 17 Sep 2016, Kent Fredric wrote:
>> 4. Due to referential integrity, it should be trivial to identify which
>> files are needed by a given ebuild, and which files are now redundant,
>> assisting with keeping the tree pruned.
> 
> How does a file in FILESDIR get stale? The typical scenario is that a
> patch will no longer be needed after a version bump and pruning of old
> ebuild versions. I fear that with FILES the problem would simply be
> shifted: instead of forgetting to delete the stale file, people would
> forget removing the patch from the FILES list.

If FILES supports USE conditionals, then it's possible to use inotify to
implement a QA check that detects unused files in FILES.
-- 
Thanks,
Zac

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Kent Fredric]

On Fri, 16 Sep 2016 17:20:28 +0200
Ulrich Mueller  wrote:

> > 3. Each entry in FILES dictates that the given file is "Used by" the
> > ebuild.  
> 
> Do you mean "file" in its Unix meaning here, i.e. including
> directories? Or only regular files?

I'd say regular files for now. Mostly because I think it should be explicit
if you want to include directory children.

/expressions/ could be expanded during MD5 generation against ${REPO}.../files/ 
path, which would probably work in the case of "${PV}/*" for instance.

But I'm undecided if I like that, only suggesting to get people thinking about 
it as a possibility.

> 
> > 4. the FILES variable is expanded and interpreted during package
> > sourcing  
> 
> > 5. "repoman full" and "repoman ci" must ensure any entry listed in FILES
> > exists  
> 
> > 6. "repoman full" and "repoman ci" must ensure any use of ${FILESDIR}
> > without a declared FILES variable is a failure.  
> 
> > 7. Under this future EAPI, the location of where ${FILESDIR} expands to
> > changes to a location within
> > ${PORTAGE_TEMPDIR}/portage/${CATEGORY}/${PF}/files  
> 
> > 8. The contents of ${PORTAGE_TEMPDIR}/portage/${CATEGORY}/${PF}/files
> > is generated from the source-time $FILES entry by mapping entries from
> > ${repostory_location}, either by symlink or by copy (though copy is
> > preferred to avoid potential side effects), mapping their heirachy
> > exactly ( that is, preserving directory structure )  
> 
> > 9. Files not listed in/indicated by $FILES are thus unavailable to the
> > ebuild at runtime and will not be seen in ${FILESDIR}, even if they are
> > in ${repository_location}  
> 
> AFAICS that proposal goes into a direction which is somewhat opposite
> to what we have pursued in EAPI 6. There, we have allowed directories
> as arguments to eapply, in order to simplify application of patchsets.
> Now maintainers would have to list all single files contained in the
> directory again.

This wouldn't change the ability for eapply to do that when used with 
patch-tarballs in SRC_URI, and it wouldn't change the ability to call:

   eapply ${FILESDIR}

It would only change where ${FILESDIR} was and what it contained.

But the use of that against directories in $FILEDIR is slightly more
cause for concern at present, though mostly due to abuse where files/*
are all considered. Such abuses /could/ possibly be made more obvious
with this mechanism.

> Also I am not sure if I like the additional burden imposed on ebuild
> maintainers by requiring double bookkeeping. FILESDIR is already a
> well defined location for the support files needed by a package.

Agreed to an extent. Though I was of the opinion that to a greater extent
at least, such concern cases would simply be relocating the book-keeping
logic for defining the file-sets to be at the FILES location, which would
give you a canonical variable to consume elsewhere.

In theory under the expression system, you could have "FILES=*" which
would basically regress the feature to what it currently is, but then having
the feature at all wouldn't be very useful.

It could be an "opt-in" feature instead of a mandatory one mind, where it would
basically degrade to a default mechanic of "Like a PATCHES array, but repoman
can check it"

> 
> > :: Benefits ::  
> 
> > [...]  
> 
> > 4. Due to referential integrity, it should be trivial to identify which
> > files are needed by a given ebuild, and which files are now redundant,
> > assisting with keeping the tree pruned.  
> 
> How does a file in FILESDIR get stale? The typical scenario is that a
> patch will no longer be needed after a version bump and pruning of old
> ebuild versions. I fear that with FILES the problem would simply be
> shifted: instead of forgetting to delete the stale file, people would
> forget removing the patch from the FILES list.
> 
> Ulrich

I Know those seem like similar problems, but the subtle difference does stick 
out
for me.

With the existing scenario the basic assumption is that *every* ebuild
needs a given file, and you have to check every ebuild and make sure
you don't break one.

And this is true *even* if you're a maintainer who tries to keep things tight.

A good maintainer can prune the contents of an ebuilds FILES= list *at
the time they perform the bump*, where as by contrast, you *cant* prune
the files themselves at the time of the bump, because they are still
needed.

So in a sense, the use of a FILES= list allows you to keep things
contextualised, you can mark which you still need, and remove that
which you don't.

Which means when a second maintainer comes along and removes an ebuild,
they don't need to have the knowledge the first maintainer internalised
when they made the bump.

Removing the ebuild will immediately alert them of the presense of
unused files, because they will cease to be indicated by any ebuilds.

So you're keeping the knowledge of "I changed the ebuild" attached to
"I changed which files are needed", 

Re: [gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Mart Raudsepp]

Ühel kenal päeval, R, 16.09.2016 kell 17:20, kirjutas Ulrich Mueller:
> How does a file in FILESDIR get stale? The typical scenario is that a
> patch will no longer be needed after a version bump and pruning of
> old
> ebuild versions. I fear that with FILES the problem would simply be
> shifted: instead of forgetting to delete the stale file, people would
> forget removing the patch from the FILES list.
> 
> Ulrich

While I'm not sure I would like this feature as a whole, compared to
the status quo, the only way this would really work in a reasonable way
(and maybe not at all in some cases without the duplication) is if you
don't eapply them in a way that lists them again, but it would replace
PATCHES functionality or you'd do some bash magic to eapply everything
in $FILES that ends in .patch or something.


Mart

[gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Ulrich Mueller]

> On Sat, 17 Sep 2016, Kent Fredric wrote:

> Just an idea that seemed obvious enough and obviously missing.

> 1. Have, in a future EAPI, a variable (I'm going to call it "FILES" as
> a stand-in for now ).

> 2. FILES contains an array (or perhaps whitespace delimited string) of
> entries (or perhaps expressions) relative to
> ${repository_location}/${CATEGORY}/${PN}/files/

> 3. Each entry in FILES dictates that the given file is "Used by" the
> ebuild.

Do you mean "file" in its Unix meaning here, i.e. including
directories? Or only regular files?

> 4. the FILES variable is expanded and interpreted during package
> sourcing

> 5. "repoman full" and "repoman ci" must ensure any entry listed in FILES
> exists

> 6. "repoman full" and "repoman ci" must ensure any use of ${FILESDIR}
> without a declared FILES variable is a failure.

> 7. Under this future EAPI, the location of where ${FILESDIR} expands to
> changes to a location within
> ${PORTAGE_TEMPDIR}/portage/${CATEGORY}/${PF}/files

> 8. The contents of ${PORTAGE_TEMPDIR}/portage/${CATEGORY}/${PF}/files
> is generated from the source-time $FILES entry by mapping entries from
> ${repostory_location}, either by symlink or by copy (though copy is
> preferred to avoid potential side effects), mapping their heirachy
> exactly ( that is, preserving directory structure )

> 9. Files not listed in/indicated by $FILES are thus unavailable to the
> ebuild at runtime and will not be seen in ${FILESDIR}, even if they are
> in ${repository_location}

AFAICS that proposal goes into a direction which is somewhat opposite
to what we have pursued in EAPI 6. There, we have allowed directories
as arguments to eapply, in order to simplify application of patchsets.
Now maintainers would have to list all single files contained in the
directory again.

Also I am not sure if I like the additional burden imposed on ebuild
maintainers by requiring double bookkeeping. FILESDIR is already a
well defined location for the support files needed by a package.

> :: Benefits ::

> [...]

> 4. Due to referential integrity, it should be trivial to identify which
> files are needed by a given ebuild, and which files are now redundant,
> assisting with keeping the tree pruned.

How does a file in FILESDIR get stale? The typical scenario is that a
patch will no longer be needed after a version bump and pruning of old
ebuild versions. I fear that with FILES the problem would simply be
shifted: instead of forgetting to delete the stale file, people would
forget removing the patch from the FILES list.

Ulrich


pgpy7uZbzdsnL.pgp
Description: PGP signature

[gentoo-dev] Proposed future EAPI feature: FILES whitelist

[Kent Fredric]

Just an idea that seemed obvious enough and obviously missing.

1. Have, in a future EAPI, a variable (I'm going to call it "FILES" as
a stand-in for now ).

2. FILES contains an array (or perhaps whitespace delimited string) of
entries (or perhaps expressions) relative to
${repository_location}/${CATEGORY}/${PN}/files/

3. Each entry in FILES dictates that the given file is "Used by" the
ebuild.

4. the FILES variable is expanded and interpreted during package
sourcing

5. "repoman full" and "repoman ci" must ensure any entry listed in FILES
exists

6. "repoman full" and "repoman ci" must ensure any use of ${FILESDIR}
without a declared FILES variable is a failure.

7. Under this future EAPI, the location of where ${FILESDIR} expands to
changes to a location within
${PORTAGE_TEMPDIR}/portage/${CATEGORY}/${PF}/files

8. The contents of ${PORTAGE_TEMPDIR}/portage/${CATEGORY}/${PF}/files
is generated from the source-time $FILES entry by mapping entries from
${repostory_location}, either by symlink or by copy (though copy is
preferred to avoid potential side effects), mapping their heirachy
exactly ( that is, preserving directory structure )

9. Files not listed in/indicated by $FILES are thus unavailable to the
ebuild at runtime and will not be seen in ${FILESDIR}, even if they are
in ${repository_location}



:: Benefits ::

1. Referential integrity of this field means basic tooling like repoman
can verify in advance the presence of things that are deemed necessary
for the ebuild to function, and can check for their absense.

2. Said referential integrity can propagate failures to mirror
properly and mistakes by ebuild authors (specifically, naughty ones who
side-step repoman) to escallate to being errors long before the files
are needed by executable code.

3. Due to point 2, the risk of silent failures due to lack of explicit
handling for the files being missing is reduced.

4. Due to referential integrity, it should be trivial to identify which
files are needed by a given ebuild, and which files are now redundant,
assisting with keeping the tree pruned.

5. When maintaining a given ebuild, the use of a variable means that
one can consult the list of files a given ebuild uses canonically, and
thus know which files may need oversight when performing changes, as
the canonical form of the variable can theoretically be computed during
metadata/ generation. ( but probably not wise to just flop that in
there, unless we can guarantee that this subtle change to the md5
format won't cause problems elsewhere ), but tooling can still be
written to spit out a list of FILES= entries. ( Currently, you must
grep the ebuild or approximate grep with your eyes, and that is prone
to errors, especially in cases of anything that uses horror shows like
eblits )

6. Due to this introducing a concept of connectedness between .ebuild/
and files/, this opens a door to a potential emerge feature of
--reinstall-if-files-changed , which would allow people to trigger a
rebuild for any ebuild which may have been subtly changed due to
patches to any of its dependency files. ( Though I'd expect to see
little use of this feature if added, except for weird anal people like
me who disable dynamic deps ;) ) 

:: Mixed Change/Benefit Cases ::

1. Some eclasses presently support a PATCHES= variable, which contain
an ordered list of patches that may be applied, stating the order of
application.

Here, in many cases, PATCHES could be dropped from need and a more
universal syntax of `eapply $( echo ${FILESDIR}/*.patch | sort )` as
generic logic in the default src_patch

Due to the ${FILESDIR} being a synthetic location constructed from the
contents of the FILES= variable, you can get away with doing a lot of
tricks that would otherwise be undesirable in the current situation.

For instance: 

 use foo-feature && eapply ${FILESDIR}/foo-feature/*.patch

Because the nature of what "FILESDIR" contained is already strictly
regulated elsewhere in the ebuild and validated long before the patch
phase.


:: Possible Future Abuses ::

FILES=$( cat ${FIlEDIR}/$PV/series )

Though this would stil be validated by repoman to make sure all entries
in "series" exists, and assuming the data was staticised to the md5
metadata cache, would not be so bad. But  probably should have
people shot for doing this. 


As always, this feature probably exists in some package manager other
than portage for ebuilds in EAPIs other than the one portage support,
but I have not checked those for feature comparison. 





pgpdU_r0bXafX.pgp
Description: OpenPGP digital signature