Re: [ptxdist] [PATCH] django: new package to provide Django 1.6.5

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

On Thu, Jul 31, 2014 at 10:14:14AM +0200, Michael Olbrich wrote:
> On Wed, Jul 30, 2014 at 08:12:35PM +0200, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  rules/django.in   | 29 +++++++++++++++++
> >  rules/django.make | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 122 insertions(+)
> >  create mode 100644 rules/django.in
> >  create mode 100644 rules/django.make
> > 
> > diff --git a/rules/django.in b/rules/django.in
> > new file mode 100644
> > index 000000000000..8b66702d4c67
> > --- /dev/null
> > +++ b/rules/django.in
> > @@ -0,0 +1,29 @@
> > +## SECTION=networking
> > +
> > +menuconfig DJANGO
> > +	tristate
> > +	select PYTHON
> > +	select PYTHON_SQLITE
> > +	select PYTHON_ZLIB
> > +	prompt "Django                        "
> > +	help
> > +	  Django is a high-level Python Web framework that encourages rapid
> > +	  development and clean, pragmatic design.
> > +
> > +if DJANGO
> > +
> > +menu "locales"
> 
> Hmmm, I think a string with a proper comment here is better. 'Space
> separated list of locales, e.g. "de en"...'
> No need to make it harder than necessary to add new locales.
ok.

> > new file mode 100644
> > index 000000000000..256f7b8dab0a
> > --- /dev/null
> > +++ b/rules/django.make
> > @@ -0,0 +1,93 @@
> > [...]
> > +
> > +# ----------------------------------------------------------------------------
> > +# Compile
> > +# ----------------------------------------------------------------------------
> > +
> > +$(STATEDIR)/django.compile:
> > +	@$(call targetinfo)
> > +	cd $(DJANGO_DIR) && \
> > +		$(DJANGO_PATH) $(DJANGO_MAKE_ENV) \
> > +		$(CROSS_PYTHON) setup.py build
> 
> I think the prefix should be added here as well, in case it's used in some
> code generation etc.
I wondered about that, too, but:

	.../Django-1.6.5$ python setup.py build --prefix=/usr
	usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
	   or: setup.py --help [cmd1 cmd2 ...]
	   or: setup.py --help-commands
	   or: setup.py cmd --help

	error: option --prefix not recognized

> > +	@$(call touch)
> > +
> > +# ----------------------------------------------------------------------------
> > +# Install
> > +# ----------------------------------------------------------------------------
> > +
> > +$(STATEDIR)/django.install:
> > +	@$(call targetinfo)
> > +	cd $(DJANGO_DIR) && \
> > +		$(DJANGO_PATH) $(DJANGO_MAKE_ENV) \
> > +		$(CROSS_PYTHON) setup.py install --root=$(DJANGO_PKGDIR) --prefix=/usr -O2
> 
> Why '-O2' here and not in the compile stage?
Same thing here:

	.../Django-1.6.5$ python setup.py build -O2
	usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
	   or: setup.py --help [cmd1 cmd2 ...]
	   or: setup.py --help-commands
	   or: setup.py cmd --help

	error: option -O not recognized

and this is not specific to Django. I see the same with my custom python
script that uses plain distutils.

> > +	# everything but locales
> > +	find "$(DJANGO_PKGDIR)" -type d -name locale -prune -o -type f -printf '/%P\n' | while read fn; do \
> > +		$(call install_copy, django, 0, 0, 0644, -, $$fn); \
> > +	done
> 
> I dislike just dumping everything in the package, e.g. the egg-info is not
> needed. Install /usr/bin/django-admin.py explicitly and then use:
> 
> 	@cd /usr/lib/python$(PYTHON_MAJORMINOR)/site-packages/django/ && find ...
> 
> This way it breaks in case something went wrong and the directory is
> missing.
I didn't do that because your approach also breaks when the path for
such packages changes. AFAIK this is the case for Python 3.
But I don't care much and can use your approach.

> > +	# locales
> > +	find "$(DJANGO_PKGDIR)" -type d -name locale -printf '/%P\n' | while read localedir; do \
> > +		for locale in $(DJANGO_LOCALES-y); do \
> > +			find "$(DJANGO_PKGDIR)$$localedir/$$locale" -type f -printf "$$localedir/$$locale/%P\n" | while read fn; do \
> 
> Same here with 'cd' and I think the django.po files are not needed, just
> the Python stuff and the compiled django.mo

	.../Django-1.6.5$ find django -type d -name locale  | wc -l
	14

should I really list all these explicitly?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

-- 
ptxdist mailing list
ptxdist@pengutronix.de