diff mbox

[v5,2/3] python-cheetah: add host-package support

Message ID 1426011874-11011-2-git-send-email-gwenj@trabucayre.com
State Superseded
Headers show

Commit Message

Gwenhael Goavec-Merou March 10, 2015, 6:24 p.m. UTC
From: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com>

Some packages, like GNURadio for VOLK, needs cheetah on host at buildtime.

Signed-off-by: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com>
---
Changes v3 -> v4:
 * use HOST_PYTHON_CHEETAH_DEPENDENCIES instead of PYTHON_CHEETAH_DEPENDENCIES
 * suppress '+' for dependency definition
---
 package/python-cheetah/python-cheetah.mk | 2 ++
 1 file changed, 2 insertions(+)

Comments

Arnout Vandecappelle March 10, 2015, 10:39 p.m. UTC | #1
On 10/03/15 19:24, Gwenhael Goavec-Merou wrote:
> From: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com>
> 
> Some packages, like GNURadio for VOLK, needs cheetah on host at buildtime.
> 
> Signed-off-by: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com>
> ---
> Changes v3 -> v4:
>  * use HOST_PYTHON_CHEETAH_DEPENDENCIES instead of PYTHON_CHEETAH_DEPENDENCIES
>  * suppress '+' for dependency definition
> ---
>  package/python-cheetah/python-cheetah.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/package/python-cheetah/python-cheetah.mk b/package/python-cheetah/python-cheetah.mk
> index 08076b5..3155951 100644
> --- a/package/python-cheetah/python-cheetah.mk
> +++ b/package/python-cheetah/python-cheetah.mk
> @@ -9,5 +9,7 @@ PYTHON_CHEETAH_SOURCE = Cheetah-$(PYTHON_CHEETAH_VERSION).tar.gz
>  PYTHON_CHEETAH_SITE = http://pypi.python.org/packages/source/C/Cheetah
>  PYTHON_CHEETAH_LICENSE = MIT
>  PYTHON_CHEETAH_SETUP_TYPE = setuptools
> +HOST_PYTHON_CHEETAH_DEPENDENCIES = host-python-markdown

 This should probably carry a comment to remember what you explained in an
earlier mail, e.g.:

# Runtime dependency on markdown is not expressed in Config.in for host package

and perhaps a fuller explanation in the commit log (explaining that setuptools
will download it if it can't be found).



 BTW, Thomas, when you committed python-cheetah you removed the runtime
dependency on markdown because there are some examples you can run without
markdown. Is that the way we work? The cheetah package itself declares a
dependency on markdown because one of its classes (Filters.Markdown) uses it. Of
course, as long as you don't use that particular filter, there won't be a
problem. But it feels weird to me that we remove a dependency that is claimed by
a package, unless there is a good reason for it (and I don't count saving 260K
of .pyc files as a good enough reason).


 Regards,
 Arnout

>  
>  $(eval $(python-package))
> +$(eval $(host-python-package))
>
Thomas Petazzoni March 10, 2015, 10:47 p.m. UTC | #2
Dear Arnout Vandecappelle,

On Tue, 10 Mar 2015 23:39:31 +0100, Arnout Vandecappelle wrote:

>  This should probably carry a comment to remember what you explained in an
> earlier mail, e.g.:
> 
> # Runtime dependency on markdown is not expressed in Config.in for host package
> 
> and perhaps a fuller explanation in the commit log (explaining that setuptools
> will download it if it can't be found).

Agreed. All those "unusual" dependencies should have a comment in
the .mk file.

>  BTW, Thomas, when you committed python-cheetah you removed the runtime
> dependency on markdown because there are some examples you can run without
> markdown. Is that the way we work? The cheetah package itself declares a
> dependency on markdown because one of its classes (Filters.Markdown) uses it. Of
> course, as long as you don't use that particular filter, there won't be a
> problem. But it feels weird to me that we remove a dependency that is claimed by
> a package, unless there is a good reason for it (and I don't count saving 260K
> of .pyc files as a good enough reason).

"Way we work" is fairly loosely defined when it comes to interpreted
languages and more-or-less optional dependencies :-)

Should python-cheetah select all the possible dependencies it may use
for all its filters, or should it leave this responsibility to the
Buildroot user ? Not easy to decide.

Since the dependency was apparently not strictly needed, and there was
(as far as I remember) no comment justifying the dependency, it seemed
natural to me to remove this dependency. That being said, I wouldn't be
offended at all if we decide to re-introduce the dependency, provided a
comment indicating why we're adding this not-really mandatory
dependency.

Cheers,

Thomas
Arnout Vandecappelle March 10, 2015, 11:02 p.m. UTC | #3
Note to Gwenhael (removed from Cc): the discussion below is no longer relevant
for your patch.

On 10/03/15 23:47, Thomas Petazzoni wrote:
> Dear Arnout Vandecappelle,
> 
> On Tue, 10 Mar 2015 23:39:31 +0100, Arnout Vandecappelle wrote:

[snip]

>>  BTW, Thomas, when you committed python-cheetah you removed the runtime
>> dependency on markdown because there are some examples you can run without
>> markdown. Is that the way we work? The cheetah package itself declares a
>> dependency on markdown because one of its classes (Filters.Markdown) uses it. Of
>> course, as long as you don't use that particular filter, there won't be a
>> problem. But it feels weird to me that we remove a dependency that is claimed by
>> a package, unless there is a good reason for it (and I don't count saving 260K
>> of .pyc files as a good enough reason).
> 
> "Way we work" is fairly loosely defined when it comes to interpreted
> languages and more-or-less optional dependencies :-)
> 
> Should python-cheetah select all the possible dependencies it may use
> for all its filters, or should it leave this responsibility to the
> Buildroot user ? Not easy to decide.

 For me, the acid test is what the package declares itself.
Cheetah.egg-info/requires.txt says Markdown >= 2.0.1
For perl packages, we trust whatever dependencies the package declares. If we
would have a scancpan-like script for pypi, the dependency would be there.

> 
> Since the dependency was apparently not strictly needed, and there was
> (as far as I remember) no comment justifying the dependency, it seemed
> natural to me to remove this dependency. That being said, I wouldn't be
> offended at all if we decide to re-introduce the dependency, provided a
> comment indicating why we're adding this not-really mandatory
> dependency.

 On the other hand, as long as the lack of dependency is not bothering anyone,
we can leave it as it is :-) The purpose of discussion is what should be the
guiding principle for future python packages. My proposal is: follow the egg info.


 Regards,
 Arnout
Peter Korsgaard March 10, 2015, 11:07 p.m. UTC | #4
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >  On the other hand, as long as the lack of dependency is not bothering anyone,
 > we can leave it as it is :-) The purpose of discussion is what should be the
 > guiding principle for future python packages. My proposal is: follow the egg info.

Sounds sensible to me.
Thomas Petazzoni March 11, 2015, 8:34 a.m. UTC | #5
Dear Arnout Vandecappelle,

On Wed, 11 Mar 2015 00:02:29 +0100, Arnout Vandecappelle wrote:

>  For me, the acid test is what the package declares itself.
> Cheetah.egg-info/requires.txt says Markdown >= 2.0.1
> For perl packages, we trust whatever dependencies the package declares. If we
> would have a scancpan-like script for pypi, the dependency would be there.

I actually did start such a script at some point :-)

>  On the other hand, as long as the lack of dependency is not bothering anyone,
> we can leave it as it is :-) The purpose of discussion is what should be the
> guiding principle for future python packages. My proposal is: follow the egg info.

I'm fine with that.

Thomas
diff mbox

Patch

diff --git a/package/python-cheetah/python-cheetah.mk b/package/python-cheetah/python-cheetah.mk
index 08076b5..3155951 100644
--- a/package/python-cheetah/python-cheetah.mk
+++ b/package/python-cheetah/python-cheetah.mk
@@ -9,5 +9,7 @@  PYTHON_CHEETAH_SOURCE = Cheetah-$(PYTHON_CHEETAH_VERSION).tar.gz
 PYTHON_CHEETAH_SITE = http://pypi.python.org/packages/source/C/Cheetah
 PYTHON_CHEETAH_LICENSE = MIT
 PYTHON_CHEETAH_SETUP_TYPE = setuptools
+HOST_PYTHON_CHEETAH_DEPENDENCIES = host-python-markdown
 
 $(eval $(python-package))
+$(eval $(host-python-package))