* [PATCH] python-setuptools.inc: avoid using += with an over-ride
@ 2018-07-04 0:55 Andre McCurdy
2018-07-04 3:15 ` Christopher Larson
2018-07-04 12:53 ` Burton, Ross
0 siblings, 2 replies; 10+ messages in thread
From: Andre McCurdy @ 2018-07-04 0:55 UTC (permalink / raw)
To: openembedded-core
An over-ride replaces the original value regardless of whether or
not it's set up with +=. As replacing the original value seems to be
the intention here, drop the += to make it more explicit. Also some
minor recipe formatting tweaks.
Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
---
meta/recipes-devtools/python/python-setuptools.inc | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/meta/recipes-devtools/python/python-setuptools.inc b/meta/recipes-devtools/python/python-setuptools.inc
index bf654be..3f1bfaa 100644
--- a/meta/recipes-devtools/python/python-setuptools.inc
+++ b/meta/recipes-devtools/python/python-setuptools.inc
@@ -2,7 +2,6 @@ SUMMARY = "Download, build, install, upgrade, and uninstall Python packages"
HOMEPAGE = "https://pypi.python.org/pypi/setuptools"
SECTION = "devel/python"
LICENSE = "MIT"
-
LIC_FILES_CHKSUM = "file://LICENSE;beginline=1;endline=19;md5=9a33897f1bca1160d7aad3835152e158"
PYPI_PACKAGE_EXT = "zip"
@@ -13,16 +12,12 @@ SRC_URI[md5sum] = "dd4e3fa83a21bf7bf9c51026dc8a4e59"
SRC_URI[sha256sum] = "f7cddbb5f5c640311eb00eab6e849f7701fa70bf6a183fc8a2c33dd1d1672fb2"
DEPENDS += "${PYTHON_PN}"
-DEPENDS_class-native += "${PYTHON_PN}-native"
-DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
+DEPENDS_class-native = "${PYTHON_PN}-native"
+DEPENDS_class-nativesdk = "nativesdk-${PYTHON_PN}"
DISTUTILS_INSTALL_ARGS += "--install-lib=${D}${PYTHON_SITEPACKAGES_DIR} \
--script-dir=${bindir}"
-RDEPENDS_${PN}_class-native = "\
- ${PYTHON_PN}-distutils \
- ${PYTHON_PN}-compression \
-"
RDEPENDS_${PN} = "\
${PYTHON_PN}-compile \
${PYTHON_PN}-compression \
@@ -41,6 +36,11 @@ RDEPENDS_${PN} = "\
${PYTHON_PN}-xml \
"
+RDEPENDS_${PN}_class-native = "\
+ ${PYTHON_PN}-compression \
+ ${PYTHON_PN}-distutils \
+"
+
do_install_prepend() {
install -d ${D}${PYTHON_SITEPACKAGES_DIR}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-04 0:55 [PATCH] python-setuptools.inc: avoid using += with an over-ride Andre McCurdy
@ 2018-07-04 3:15 ` Christopher Larson
2018-07-04 5:06 ` Andre McCurdy
2018-07-04 12:53 ` Burton, Ross
1 sibling, 1 reply; 10+ messages in thread
From: Christopher Larson @ 2018-07-04 3:15 UTC (permalink / raw)
To: Andre McCurdy; +Cc: Patches and discussions about the oe-core layer
[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]
On Tue, Jul 3, 2018 at 5:55 PM Andre McCurdy <armccurdy@gmail.com> wrote:
> An over-ride replaces the original value regardless of whether or
> not it's set up with +=. As replacing the original value seems to be
> the intention here, drop the += to make it more explicit. Also some
> minor recipe formatting tweaks.
>
> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
>
Technically this is not the case, not precisely. += is appending to any
existing DEPENDS_class-native value, and only *then* is the override
applied to replace DEPENDS. So it's not appending to DEPENDS, it's
appending to DEPENDS_class-native. In this case, it's most likely doing it
this way in the .inc so it's possible for the recipe including it to define
DEPENDS_class-native alongside DEPENDS before the inclusion, but this is
largely pointless, since they can always += to it *after* the inclusion
instead. So I think you're good, most likely it's still fine to remove it,
but I wanted to clarify, as there *are* times when this is a useful thing
to do.
--
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics
[-- Attachment #2: Type: text/html, Size: 1716 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-04 3:15 ` Christopher Larson
@ 2018-07-04 5:06 ` Andre McCurdy
0 siblings, 0 replies; 10+ messages in thread
From: Andre McCurdy @ 2018-07-04 5:06 UTC (permalink / raw)
To: Patches and discussions about the oe-core layer
On Tue, Jul 3, 2018 at 8:15 PM, Christopher Larson <kergoth@gmail.com> wrote:
> On Tue, Jul 3, 2018 at 5:55 PM Andre McCurdy <armccurdy@gmail.com> wrote:
>>
>> An over-ride replaces the original value regardless of whether or
>> not it's set up with +=. As replacing the original value seems to be
>> the intention here, drop the += to make it more explicit. Also some
>> minor recipe formatting tweaks.
>>
>> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
>
> Technically this is not the case, not precisely. += is appending to any
> existing DEPENDS_class-native value, and only *then* is the override applied
> to replace DEPENDS. So it's not appending to DEPENDS, it's appending to
> DEPENDS_class-native.
Yes, that's what I said, or at least tried to. It doesn't matter if
you assign to the over-ride directly or use += to append to it... it's
still an over-ride which completely replaces the original value.
> In this case, it's most likely doing it this way in
> the .inc so it's possible for the recipe including it to define
> DEPENDS_class-native alongside DEPENDS before the inclusion, but this is
> largely pointless, since they can always += to it *after* the inclusion
> instead. So I think you're good, most likely it's still fine to remove it,
> but I wanted to clarify, as there *are* times when this is a useful thing to
> do.
Right. I did look into the recipe, the include and the various classes
it uses fairly carefully and I'm pretty sure this is the right fix.
As to whether allowing += with an over-ride is a useful feature of the
language I'm not sure. There are other ways to achieve the same result
but it's a huge trap for new users who assume that it behaves like a
conditional append - I've seen that bug many times.
> --> Christopher Larson
> kergoth at gmail dot com
> Founder - BitBake, OpenEmbedded, OpenZaurus
> Senior Software Engineer, Mentor Graphics
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-04 0:55 [PATCH] python-setuptools.inc: avoid using += with an over-ride Andre McCurdy
2018-07-04 3:15 ` Christopher Larson
@ 2018-07-04 12:53 ` Burton, Ross
2018-07-05 23:39 ` Andre McCurdy
1 sibling, 1 reply; 10+ messages in thread
From: Burton, Ross @ 2018-07-04 12:53 UTC (permalink / raw)
To: Andre McCurdy; +Cc: OE-core
On 4 July 2018 at 01:55, Andre McCurdy <armccurdy@gmail.com> wrote:
> DEPENDS += "${PYTHON_PN}"
> -DEPENDS_class-native += "${PYTHON_PN}-native"
> -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
> +DEPENDS_class-native = "${PYTHON_PN}-native"
> +DEPENDS_class-nativesdk = "nativesdk-${PYTHON_PN}"
Whilst there's nothing wrong with this patch, as the BBCLASSEXTEND
handles DEPENDS-munging isn't this a better fix:
DEPENDS += "${PYTHON_PN}"
-DEPENDS_class-native += "${PYTHON_PN}-native"
-DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
Ross
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-04 12:53 ` Burton, Ross
@ 2018-07-05 23:39 ` Andre McCurdy
2018-07-06 0:07 ` Burton, Ross
0 siblings, 1 reply; 10+ messages in thread
From: Andre McCurdy @ 2018-07-05 23:39 UTC (permalink / raw)
To: Burton, Ross; +Cc: OE-core
On Wed, Jul 4, 2018 at 5:53 AM, Burton, Ross <ross.burton@intel.com> wrote:
> On 4 July 2018 at 01:55, Andre McCurdy <armccurdy@gmail.com> wrote:
>> DEPENDS += "${PYTHON_PN}"
>> -DEPENDS_class-native += "${PYTHON_PN}-native"
>> -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
>> +DEPENDS_class-native = "${PYTHON_PN}-native"
>> +DEPENDS_class-nativesdk = "nativesdk-${PYTHON_PN}"
>
> Whilst there's nothing wrong with this patch, as the BBCLASSEXTEND
> handles DEPENDS-munging isn't this a better fix:
>
> DEPENDS += "${PYTHON_PN}"
> -DEPENDS_class-native += "${PYTHON_PN}-native"
> -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
Removing the over-rides completely results in both the native and
nativesdk variants gaining a new dependency on
python-distribute-native (courtesy of setuptools.bbclass).
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-05 23:39 ` Andre McCurdy
@ 2018-07-06 0:07 ` Burton, Ross
2018-07-06 0:57 ` Andre McCurdy
0 siblings, 1 reply; 10+ messages in thread
From: Burton, Ross @ 2018-07-06 0:07 UTC (permalink / raw)
To: Andre McCurdy; +Cc: OE-core
On 6 July 2018 at 00:39, Andre McCurdy <armccurdy@gmail.com> wrote:
> On Wed, Jul 4, 2018 at 5:53 AM, Burton, Ross <ross.burton@intel.com> wrote:
>> On 4 July 2018 at 01:55, Andre McCurdy <armccurdy@gmail.com> wrote:
>>> DEPENDS += "${PYTHON_PN}"
>>> -DEPENDS_class-native += "${PYTHON_PN}-native"
>>> -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
>>> +DEPENDS_class-native = "${PYTHON_PN}-native"
>>> +DEPENDS_class-nativesdk = "nativesdk-${PYTHON_PN}"
>>
>> Whilst there's nothing wrong with this patch, as the BBCLASSEXTEND
>> handles DEPENDS-munging isn't this a better fix:
>>
>> DEPENDS += "${PYTHON_PN}"
>> -DEPENDS_class-native += "${PYTHON_PN}-native"
>> -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
>
> Removing the over-rides completely results in both the native and
> nativesdk variants gaining a new dependency on
> python-distribute-native (courtesy of setuptools.bbclass).
setuptools.bbclass:DEPENDS += "python-distribute-native"
If python-distribute-native is a build dependency for target then its
a dependency for native and nativesdk too, right?
Considering python-distribute is just an alias for python-setuptools
(its a RPROVIDES in the recipe, and the pip entry is just a
dependnecy) I guess the setuptools class should follow setuptools3 and
depend on python-setuptools-native instead of
python-distribute-native.
Context is this from 2015:
commit c4553e07cdbec5c29a74a43e0c5fdb2bb5583f7a
Author: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
Date: Thu May 14 16:49:56 2015 +0000
python3-distribute: Upgrade to python3-setuptools 15.2
python3-distribute was merged back to python3-setuptools in 2013,
and it is no longer being maintained, this upgrade also provides
functionality that will be needed for python3-pip.
The same needs to be applied to the py2 classes, as both Py2 and Py3
use the same version of setuptools.
Ross
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-06 0:07 ` Burton, Ross
@ 2018-07-06 0:57 ` Andre McCurdy
2018-07-06 9:11 ` Richard Purdie
2018-07-06 15:10 ` Burton, Ross
0 siblings, 2 replies; 10+ messages in thread
From: Andre McCurdy @ 2018-07-06 0:57 UTC (permalink / raw)
To: Burton, Ross; +Cc: OE-core
On Thu, Jul 5, 2018 at 5:07 PM, Burton, Ross <ross.burton@intel.com> wrote:
> On 6 July 2018 at 00:39, Andre McCurdy <armccurdy@gmail.com> wrote:
>> On Wed, Jul 4, 2018 at 5:53 AM, Burton, Ross <ross.burton@intel.com> wrote:
>>> On 4 July 2018 at 01:55, Andre McCurdy <armccurdy@gmail.com> wrote:
>>>> DEPENDS += "${PYTHON_PN}"
>>>> -DEPENDS_class-native += "${PYTHON_PN}-native"
>>>> -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
>>>> +DEPENDS_class-native = "${PYTHON_PN}-native"
>>>> +DEPENDS_class-nativesdk = "nativesdk-${PYTHON_PN}"
>>>
>>> Whilst there's nothing wrong with this patch, as the BBCLASSEXTEND
>>> handles DEPENDS-munging isn't this a better fix:
>>>
>>> DEPENDS += "${PYTHON_PN}"
>>> -DEPENDS_class-native += "${PYTHON_PN}-native"
>>> -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
>>
>> Removing the over-rides completely results in both the native and
>> nativesdk variants gaining a new dependency on
>> python-distribute-native (courtesy of setuptools.bbclass).
>
> setuptools.bbclass:DEPENDS += "python-distribute-native"
>
> If python-distribute-native is a build dependency for target then its
> a dependency for native and nativesdk too, right?
DEPENDS has been over-ridden for native and nativesdk for some time,
so either the dependency gets provided indirectly or it's not required
at all?
> Considering python-distribute is just an alias for python-setuptools
> (its a RPROVIDES in the recipe, and the pip entry is just a
> dependnecy) I guess the setuptools class should follow setuptools3 and
> depend on python-setuptools-native instead of
> python-distribute-native.
>
> Context is this from 2015:
>
> commit c4553e07cdbec5c29a74a43e0c5fdb2bb5583f7a
> Author: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
> Date: Thu May 14 16:49:56 2015 +0000
>
> python3-distribute: Upgrade to python3-setuptools 15.2
>
> python3-distribute was merged back to python3-setuptools in 2013,
> and it is no longer being maintained, this upgrade also provides
> functionality that will be needed for python3-pip.
>
> The same needs to be applied to the py2 classes, as both Py2 and Py3
> use the same version of setuptools.
That all sounds plausible, but starts to unravel into a bigger set of
potential cleanups. For example, digging a little more uncovers this
from distutils-base.bbclass:
DEPENDS += "${@["${PYTHON_PN}-native ${PYTHON_PN}",
""][(d.getVar('PACKAGES') == '')]}"
RDEPENDS_${PN} += "${@['', '${PYTHON_PN}-core']['${CLASSOVERRIDE}'
== 'class-target']}"
The first line adding to DEPENDS for target and nativesdk (but not
native) and the second line adding to RDEPENDS for target only. Should
this be re-written to make use of over-rides?
And looking at nativesdk builds in general, why does
DISTRO_FEATURES_NATIVE contain ipv6 and xattr but
DISTRO_FEATURES_NATIVESDK does not? I couldn't find any comments to
explain that.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-06 0:57 ` Andre McCurdy
@ 2018-07-06 9:11 ` Richard Purdie
2018-07-06 15:10 ` Burton, Ross
1 sibling, 0 replies; 10+ messages in thread
From: Richard Purdie @ 2018-07-06 9:11 UTC (permalink / raw)
To: Andre McCurdy, Burton, Ross; +Cc: OE-core
On Thu, 2018-07-05 at 17:57 -0700, Andre McCurdy wrote:
> On Thu, Jul 5, 2018 at 5:07 PM, Burton, Ross <ross.burton@intel.com>
> wrote:
> > On 6 July 2018 at 00:39, Andre McCurdy <armccurdy@gmail.com> wrote:
> > > On Wed, Jul 4, 2018 at 5:53 AM, Burton, Ross <ross.burton@intel.c
> > > om> wrote:
> > > > On 4 July 2018 at 01:55, Andre McCurdy <armccurdy@gmail.com>
> > > > wrote:
> > > > > DEPENDS += "${PYTHON_PN}"
> > > > > -DEPENDS_class-native += "${PYTHON_PN}-native"
> > > > > -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
> > > > > +DEPENDS_class-native = "${PYTHON_PN}-native"
> > > > > +DEPENDS_class-nativesdk = "nativesdk-${PYTHON_PN}"
> > > >
> > > > Whilst there's nothing wrong with this patch, as the
> > > > BBCLASSEXTEND
> > > > handles DEPENDS-munging isn't this a better fix:
> > > >
> > > > DEPENDS += "${PYTHON_PN}"
> > > > -DEPENDS_class-native += "${PYTHON_PN}-native"
> > > > -DEPENDS_class-nativesdk += "nativesdk-${PYTHON_PN}"
> > >
> > > Removing the over-rides completely results in both the native and
> > > nativesdk variants gaining a new dependency on
> > > python-distribute-native (courtesy of setuptools.bbclass).
> >
> > setuptools.bbclass:DEPENDS += "python-distribute-native"
> >
> > If python-distribute-native is a build dependency for target then
> > its a dependency for native and nativesdk too, right?
>
> DEPENDS has been over-ridden for native and nativesdk for some time,
> so either the dependency gets provided indirectly or it's not
> required at all?
I'd suspect indirectly but hard to tell :/.
> > Considering python-distribute is just an alias for python-
> > setuptools
> > (its a RPROVIDES in the recipe, and the pip entry is just a
> > dependnecy) I guess the setuptools class should follow setuptools3
> > and
> > depend on python-setuptools-native instead of
> > python-distribute-native.
> >
> > Context is this from 2015:
> >
> > commit c4553e07cdbec5c29a74a43e0c5fdb2bb5583f7a
> > Author: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
> > Date: Thu May 14 16:49:56 2015 +0000
> >
> > python3-distribute: Upgrade to python3-setuptools 15.2
> >
> > python3-distribute was merged back to python3-setuptools in
> > 2013,
> > and it is no longer being maintained, this upgrade also
> > provides
> > functionality that will be needed for python3-pip.
> >
> > The same needs to be applied to the py2 classes, as both Py2 and
> > Py3
> > use the same version of setuptools.
>
> That all sounds plausible, but starts to unravel into a bigger set of
> potential cleanups. For example, digging a little more uncovers this
> from distutils-base.bbclass:
>
> DEPENDS += "${@["${PYTHON_PN}-native ${PYTHON_PN}",
> ""][(d.getVar('PACKAGES') == '')]}"
> RDEPENDS_${PN} += "${@['', '${PYTHON_PN}-core']['${CLASSOVERRIDE}'
> == 'class-target']}"
>
> The first line adding to DEPENDS for target and nativesdk (but not
> native) and the second line adding to RDEPENDS for target only.
> Should this be re-written to make use of over-rides?
I suspect class- overrides did not exist when that code was written.
Rewriting it using them would be a good cleanup.
> And looking at nativesdk builds in general, why does
> DISTRO_FEATURES_NATIVE contain ipv6 and xattr but
> DISTRO_FEATURES_NATIVESDK does not? I couldn't find any comments to
> explain that.
I suspect nativesdk should have those and its just a oversight.
Unfortunately we have a number of little details like this, I do take a
pass at some things when I have time but clearly there are things which
can be improved.
FWIW I tend to look at them from a risk perspective too and how well we
can know whether the cleanup will or won't risk breakage before
deciding where to make changes.
Cheers,
Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-06 0:57 ` Andre McCurdy
2018-07-06 9:11 ` Richard Purdie
@ 2018-07-06 15:10 ` Burton, Ross
2018-07-06 18:35 ` Andre McCurdy
1 sibling, 1 reply; 10+ messages in thread
From: Burton, Ross @ 2018-07-06 15:10 UTC (permalink / raw)
To: Andre McCurdy; +Cc: OE-core
On 6 July 2018 at 01:57, Andre McCurdy <armccurdy@gmail.com> wrote:
> That all sounds plausible, but starts to unravel into a bigger set of
> potential cleanups. For example, digging a little more uncovers this
> from distutils-base.bbclass:
Let's take these cleanups one at a time. I've just submitted the
distribute->setuptools change as a start.
Ross
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] python-setuptools.inc: avoid using += with an over-ride
2018-07-06 15:10 ` Burton, Ross
@ 2018-07-06 18:35 ` Andre McCurdy
0 siblings, 0 replies; 10+ messages in thread
From: Andre McCurdy @ 2018-07-06 18:35 UTC (permalink / raw)
To: Burton, Ross; +Cc: OE-core
On Fri, Jul 6, 2018 at 8:10 AM, Burton, Ross <ross.burton@intel.com> wrote:
> On 6 July 2018 at 01:57, Andre McCurdy <armccurdy@gmail.com> wrote:
>> That all sounds plausible, but starts to unravel into a bigger set of
>> potential cleanups. For example, digging a little more uncovers this
>> from distutils-base.bbclass:
>
> Let's take these cleanups one at a time. I've just submitted the
> distribute->setuptools change as a start.
In that spirit, should we merge the original patch, which cleans up +=
with an over-ride without changing any dependencies?
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-07-06 18:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-04 0:55 [PATCH] python-setuptools.inc: avoid using += with an over-ride Andre McCurdy
2018-07-04 3:15 ` Christopher Larson
2018-07-04 5:06 ` Andre McCurdy
2018-07-04 12:53 ` Burton, Ross
2018-07-05 23:39 ` Andre McCurdy
2018-07-06 0:07 ` Burton, Ross
2018-07-06 0:57 ` Andre McCurdy
2018-07-06 9:11 ` Richard Purdie
2018-07-06 15:10 ` Burton, Ross
2018-07-06 18:35 ` Andre McCurdy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.