Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Michał Górny]

Dnia 29 stycznia 2018 09:07:47 CET, Michael Haubenwallner  
napisał(a):
>On 01/25/2018 10:11 AM, Michał Górny wrote:
>> W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael
>> Haubenwallner napisał:
>>> Hi,
>>>
>>> ${Subject} ringing a bell here:
>>>
>>> dev-db/oracle-instantclient is fetch restricted. As a binary package
>with
>>> multiple USE options there's a bunch of files to download - even for
>>> multiple archs when multilib is active.
>>>
>>> So in pkg_nofetch() I'm telling the user whether a file to download
>is
>>> "already here" or "still absent", by testing if $A exists in
>$DISTDIR.
>>>
>>> With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch
>too.
>>>
>>
>> You're doing the wrong thing then. DISTDIR is not allowed
>> in pkg_nofetch().
>
>Is there a supported way to tell the user exactly which files are still
>missing?

Sounds like a featurereq for Portage itself.

>
>> Furthermore, you're touching files whose hashes have
>> not been verified which is twice wrong.
>
>Well - does portage actually provide unverified files in the shadow
>DISTDIR?

No. The phases in which the directory is present are only run if Manifest 
verification succeeds.

>
>Thanks!
>/haubi/


-- 
Best regards,
Michał Górny (by phone)

[gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Michael Haubenwallner]

On 01/25/2018 10:11 AM, Michał Górny wrote:
> W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael
> Haubenwallner napisał:
>> Hi,
>>
>> ${Subject} ringing a bell here:
>>
>> dev-db/oracle-instantclient is fetch restricted. As a binary package with
>> multiple USE options there's a bunch of files to download - even for
>> multiple archs when multilib is active.
>>
>> So in pkg_nofetch() I'm telling the user whether a file to download is
>> "already here" or "still absent", by testing if $A exists in $DISTDIR.
>>
>> With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch too.
>>
>
> You're doing the wrong thing then. DISTDIR is not allowed
> in pkg_nofetch().

Is there a supported way to tell the user exactly which files are still missing?

> Furthermore, you're touching files whose hashes have
> not been verified which is twice wrong.

Well - does portage actually provide unverified files in the shadow DISTDIR?

Thanks!
/haubi/

Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Michał Górny]

W dniu czw, 25.01.2018 o godzinie 22∶55 -0800, użytkownik Zac Medico
napisał:
> On 01/25/2018 10:42 PM, Michał Górny wrote:
> > W dniu czw, 25.01.2018 o godzinie 21∶30 -0800, użytkownik Zac Medico
> > napisał:
> > > On 01/25/2018 01:11 AM, Michał Górny wrote:
> > > > W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael
> > > > Haubenwallner napisał:
> > > > > Hi,
> > > > > 
> > > > > ${Subject} ringing a bell here:
> > > > > 
> > > > > dev-db/oracle-instantclient is fetch restricted. As a binary package 
> > > > > with
> > > > > multiple USE options there's a bunch of files to download - even for
> > > > > multiple archs when multilib is active.
> > > > > 
> > > > > So in pkg_nofetch() I'm telling the user whether a file to download is
> > > > > "already here" or "still absent", by testing if $A exists in $DISTDIR.
> > > > > 
> > > > > With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch 
> > > > > too.
> > > > > 
> > > > 
> > > > You're doing the wrong thing then. DISTDIR is not allowed
> > > > in pkg_nofetch().
> > > 
> > > It seems to be a common assumption that it's allowed, this command
> > > currently shows 163 results in the gentoo repo:
> > > 
> > > git grep -l pkg_nofetch | xargs grep 'e\(log\|info\).*DISTDIR' | wc -l
> > > 
> > > We should double check with the PMS maintainers to see if they think
> > > it's worthy of an exception. Otherwise, we need to announce the issue on
> > > the gentoo-dev mailing list.
> > 
> > PMS maintainers already verified that back during the first run of those
> > patches. However, we believe the only reasonable way to get this out of
> > pkg_nofetch() is to actually stop it from working, so people would stop
> > using it.
> 
> Okay, that works for me. The patches looks good. Please merge.

Merged, thanks.

-- 
Best regards,
Michał Górny

Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Ulrich Mueller]

> On Thu, 25 Jan 2018, Zac Medico wrote:

> It seems to be a common assumption that it's allowed, this command
> currently shows 163 results in the gentoo repo:

> git grep -l pkg_nofetch | xargs grep 'e\(log\|info\).*DISTDIR' | wc -l

There will be little breakage if it is only used in elog messages.

> We should double check with the PMS maintainers to see if they think
> it's worthy of an exception. Otherwise, we need to announce the
> issue on the gentoo-dev mailing list.

PMS had never defined DISTDIR in pkg_* phases. You can see this e.g.
in the PMS version from 2010 [1].

In addition to this, the following clarification of variables' scope
was approved by the council in 2017 [2]:

| 6. Scope and consistency of FILESDIR and DISTDIR
| 
| Old: src_* (not consistent across ebuild phases)
| New: src_*, global scope (consistent). No access to the dir in
| global scope.
|
| Rationale: These variables must be defined in global scope for
| assignment of the PATCHES variable in EAPI 6. However, the actual
| directories are not necessarily present in global scope (e.g., when
| installing from a binary package) and therefore must not be accessed
| there.

Ulrich


[1] https://projects.gentoo.org/pms/3/pms.html#x1-117002
[2] https://bugs.gentoo.org/616206


pgpudLoADGanK.pgp
Description: PGP signature

Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Zac Medico]

On 01/25/2018 10:42 PM, Michał Górny wrote:
> W dniu czw, 25.01.2018 o godzinie 21∶30 -0800, użytkownik Zac Medico
> napisał:
>> On 01/25/2018 01:11 AM, Michał Górny wrote:
>>> W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael
>>> Haubenwallner napisał:
 Hi,

 ${Subject} ringing a bell here:

 dev-db/oracle-instantclient is fetch restricted. As a binary package with
 multiple USE options there's a bunch of files to download - even for
 multiple archs when multilib is active.

 So in pkg_nofetch() I'm telling the user whether a file to download is
 "already here" or "still absent", by testing if $A exists in $DISTDIR.

 With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch too.

>>>
>>> You're doing the wrong thing then. DISTDIR is not allowed
>>> in pkg_nofetch().
>>
>> It seems to be a common assumption that it's allowed, this command
>> currently shows 163 results in the gentoo repo:
>>
>> git grep -l pkg_nofetch | xargs grep 'e\(log\|info\).*DISTDIR' | wc -l
>>
>> We should double check with the PMS maintainers to see if they think
>> it's worthy of an exception. Otherwise, we need to announce the issue on
>> the gentoo-dev mailing list.
> 
> PMS maintainers already verified that back during the first run of those
> patches. However, we believe the only reasonable way to get this out of
> pkg_nofetch() is to actually stop it from working, so people would stop
> using it.

Okay, that works for me. The patches looks good. Please merge.
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Michał Górny]

W dniu czw, 25.01.2018 o godzinie 21∶30 -0800, użytkownik Zac Medico
napisał:
> On 01/25/2018 01:11 AM, Michał Górny wrote:
> > W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael
> > Haubenwallner napisał:
> > > Hi,
> > > 
> > > ${Subject} ringing a bell here:
> > > 
> > > dev-db/oracle-instantclient is fetch restricted. As a binary package with
> > > multiple USE options there's a bunch of files to download - even for
> > > multiple archs when multilib is active.
> > > 
> > > So in pkg_nofetch() I'm telling the user whether a file to download is
> > > "already here" or "still absent", by testing if $A exists in $DISTDIR.
> > > 
> > > With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch too.
> > > 
> > 
> > You're doing the wrong thing then. DISTDIR is not allowed
> > in pkg_nofetch().
> 
> It seems to be a common assumption that it's allowed, this command
> currently shows 163 results in the gentoo repo:
> 
> git grep -l pkg_nofetch | xargs grep 'e\(log\|info\).*DISTDIR' | wc -l
> 
> We should double check with the PMS maintainers to see if they think
> it's worthy of an exception. Otherwise, we need to announce the issue on
> the gentoo-dev mailing list.

PMS maintainers already verified that back during the first run of those
patches. However, we believe the only reasonable way to get this out of
pkg_nofetch() is to actually stop it from working, so people would stop
using it.

> Furthermore, you're touching files whose hashes have
> > not been verified which is twice wrong.
> 
> Checking if files exist is not really a security risk, but yes, we
> should conform to the spec.

There's no technical reason why the ebuild wouldn't have done more than
checking if they exist.

-- 
Best regards,
Michał Górny

Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Zac Medico]

On 01/25/2018 01:11 AM, Michał Górny wrote:
> W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael
> Haubenwallner napisał:
>> Hi,
>>
>> ${Subject} ringing a bell here:
>>
>> dev-db/oracle-instantclient is fetch restricted. As a binary package with
>> multiple USE options there's a bunch of files to download - even for
>> multiple archs when multilib is active.
>>
>> So in pkg_nofetch() I'm telling the user whether a file to download is
>> "already here" or "still absent", by testing if $A exists in $DISTDIR.
>>
>> With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch too.
>>
> 
> You're doing the wrong thing then. DISTDIR is not allowed
> in pkg_nofetch().

It seems to be a common assumption that it's allowed, this command
currently shows 163 results in the gentoo repo:

git grep -l pkg_nofetch | xargs grep 'e\(log\|info\).*DISTDIR' | wc -l

We should double check with the PMS maintainers to see if they think
it's worthy of an exception. Otherwise, we need to announce the issue on
the gentoo-dev mailing list.

Furthermore, you're touching files whose hashes have
> not been verified which is twice wrong.

Checking if files exist is not really a security risk, but yes, we
should conform to the spec.
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Michał Górny]

W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael
Haubenwallner napisał:
> Hi,
> 
> ${Subject} ringing a bell here:
> 
> dev-db/oracle-instantclient is fetch restricted. As a binary package with
> multiple USE options there's a bunch of files to download - even for
> multiple archs when multilib is active.
> 
> So in pkg_nofetch() I'm telling the user whether a file to download is
> "already here" or "still absent", by testing if $A exists in $DISTDIR.
> 
> With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch too.
> 

You're doing the wrong thing then. DISTDIR is not allowed
in pkg_nofetch(). Furthermore, you're touching files whose hashes have
not been verified which is twice wrong.

-- 
Best regards,
Michał Górny

[gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack

[Michael Haubenwallner]

Hi,

${Subject} ringing a bell here:

dev-db/oracle-instantclient is fetch restricted. As a binary package with
multiple USE options there's a bunch of files to download - even for
multiple archs when multilib is active.

So in pkg_nofetch() I'm telling the user whether a file to download is
"already here" or "still absent", by testing if $A exists in $DISTDIR.

With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch too.

/haubi/

On 01/25/2018 09:50 AM, Michał Górny wrote:
> ---
>  pym/_emerge/EbuildExecuter.py | 4 
>  pym/_emerge/EbuildPhase.py| 6 --
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/pym/_emerge/EbuildExecuter.py b/pym/_emerge/EbuildExecuter.py
> index ab79ce901..d387b42be 100644
> --- a/pym/_emerge/EbuildExecuter.py
> +++ b/pym/_emerge/EbuildExecuter.py
> @@ -8,7 +8,6 @@ import portage
>  from portage import os
>  from portage.eapi import eapi_has_src_prepare_and_src_configure, \
>   eapi_exports_replace_vars
> -from portage.package.ebuild.prepare_build_dirs import _prepare_fake_distdir
>  
>  class EbuildExecuter(CompositeTask):
>  
> @@ -25,9 +24,6 @@ class EbuildExecuter(CompositeTask):
>   cleanup = 0
>   portage.prepare_build_dirs(pkg.root, settings, cleanup)
>  
> - alist = settings.configdict["pkg"].get("A", "").split()
> - _prepare_fake_distdir(settings, alist)
> -
>   if eapi_exports_replace_vars(settings['EAPI']):
>   vardb = pkg.root_config.trees['vartree'].dbapi
>   settings["REPLACING_VERSIONS"] = " ".join(
> diff --git a/pym/_emerge/EbuildPhase.py b/pym/_emerge/EbuildPhase.py
> index aa3a66831..d3fada622 100644
> --- a/pym/_emerge/EbuildPhase.py
> +++ b/pym/_emerge/EbuildPhase.py
> @@ -1,4 +1,4 @@
> -# Copyright 1999-2013 Gentoo Foundation
> +# Copyright 1999-2018 Gentoo Foundation
>  # Distributed under the terms of the GNU General Public License v2
>  
>  import gzip
> @@ -12,7 +12,7 @@ from _emerge.MiscFunctionsProcess import 
> MiscFunctionsProcess
>  from _emerge.EbuildProcess import EbuildProcess
>  from _emerge.CompositeTask import CompositeTask
>  from portage.package.ebuild.prepare_build_dirs import (_prepare_workdir,
> - _prepare_fake_filesdir)
> + _prepare_fake_distdir, _prepare_fake_filesdir)
>  from portage.util import writemsg
>  
>  try:
> @@ -171,6 +171,8 @@ class EbuildPhase(CompositeTask):
>   def _start_ebuild(self):
>  
>   if self.phase == "unpack":
> + alist = self.settings.configdict["pkg"].get("A", 
> "").split()
> + _prepare_fake_distdir(self.settings, alist)
>   _prepare_fake_filesdir(self.settings)
>  
>   fd_pipes = self.fd_pipes
>