1.8 shipping invalid .py files in the startapp template

[Chris Lamb]

Hi,

I sent this first as a pull request — talk is cheap, code is better,
etc. — but now feel I should I have posted here first.

The underlying issue is that Debian packages unconditionally
byte-compile .py files under dist-packages upon installation using
`pycompile` and do not silence errors by design. Thus the "invalid"
files that form part of the `startapp` template require tedious special
attention to avoid errors.

I'm sure this would affect other, non-Debian, packaging systems, as well
as surface weird behaviour elsewhere. Shipping broken .py files
masquerading as valid ones additionally just feels dirty.

We templated .py files before, but as the '{{ foo }}' bits were inside
strings, docstrings or comments, the pre-rendered templates were
perfectly valid Python.

Therefore I think need a way of shipping the "invalid" .py files that
form part of the `startapp` template without .py extensions but, of
course, ensuring they end up as .py after rendering. (Or we use some
entirely different mechanism.)

My current implementation is
<https://github.com/django/django/pull/5735>; see some additional
comments I made there.


Regards,

--
Chris Lamb
chris-lamb.co.uk / @lolamby

[Florian Apolloner]

Hi,

Those packages/modules are clearly marked as package data (https://github.com/django/django/blob/master/setup.py#L26-L28), so imo it is a bug in Debian packaging (although I do understand that it might not be the easiest to fix…). According to the man-page of pycompile you can exclude certain packages, this is imo the way to go.

Cheers,
Florian

P.S.: Also you seem to miss the project_template in your PR.

[Tim Graham]

I believe this is a new issue in 1.9, not 1.8 as the subject says, correct?

I wouldn't mind seeing it fixed as old versions of pip report a SyntaxError when installing Django (this doesn't affect the install, but creates confusion for users and resulted in at least two bug reports like https://code.djangoproject.com/ticket/25584).

[Florian Apolloner]

I rather add a check to setup.py and tell people to upgrade pip

[Tim Graham]

Claude said, "I also encountered this error when using pip 1.5.6 (default version in Debian stable)." I guess at least some people might not want to upgrade system packages.

Is your main opposition to the change a "purity" one? Sure, we could add a pip version check, but I don't see any downside to the proposed change.

[Donald Stufft]

On Nov 28, 2015, at 11:03 AM, Tim Graham <timog...@gmail.com> wrote:

Claude said, "I also encountered this error when using pip 1.5.6 (default version in Debian stable)." I guess at least some people might not want to upgrade system packages.

Is your main opposition to the change a "purity" one? Sure, we could add a pip version check, but I don't see any downside to the proposed change.

pip 1.5.6 will print the warning but it’s just a warning. Newer pips will silence it. A failure to compile to .pyc never fails the install for pip.

-----------------
Donald Stufft
PGP: 0x6E3CBCE93372DCFA // 7C6B 7C5D 5E2B 6356 A926 F04F 6E3C BCE9 3372 DCFA

[Tim Graham]

Yes, I know but it creates confusion for users and has resulted in at least two bug reports like https://code.djangoproject.com/ticket/25584. I suspect we'll see more reports and queries about it once 1.9 goes final so I'd like to avoid that as long as there isn't any downside to the proposed change.

[Florian Apolloner]

If I am not mistaken 1.5.6 is still one of those versions which whould install from external sources etc… So from a security point of view I wanna force people to upgrade.

Regarding purity: No it is not purity, as the PR already showed, at least project_template would be missing etc… Also this is not a problem of Django but rather packaging in distribution, ie affects more packages to Django. So why add a specialcased solution in Django if we maybe should fix packaging instead?

[Tim Graham]

"Force" or "nudge" to upgrade pip? i.e. should Django not install with pip < some version? That seems controversial.

Is the "man-page of pycompile you can exclude certain packages" suggestion some change we should make in Django? As long as we have some solution to avoid support queries about this problem, I don't care which one we choose.

[lamby]

> I believe this is a new issue in 1.9, not 1.8 as the subject says, correct?

Correct, apologies. (I meant to write "since 1.8" as "1.9" is either ambiguous or not released yet.)

 

> Is the "man-page of pycompile you can exclude certain packages" suggestion some change we should make in Django?

Not really; it would simply be moving the special-casing around a bit.

> at least project_template would be missing etc

Deliberately so; these are not invalid .py files.

[Florian Apolloner]

On Saturday, November 28, 2015 at 7:16:10 PM UTC+1, lamby wrote:

> at least project_template would be missing etc

Deliberately so; these are not invalid .py files.

Yes, but by luck and/or accident. In the end they are still package data and real packages, so you should never run pycompile on them in the first place.

[Florian Apolloner]

Damn, I meant to write "and not real packages"

[Tim Graham]

Actually, the SyntaxError issue isn't related to pip but to setuptools 5.5 and 5.5.1 (Debian stable ships the latter).

Florian, your preference is to decline the change proposed by Chris and just deal with any support queries when people see a log like below? I understand it's not ideal to add workarounds for bugs in other software as the proposal does, but it seems like a pragmatic solution to me.

$ pip install Django==1.9c2
Downloading/unpacking Django==1.9c2
  Downloading Django-1.9rc2-py2.py3-none-any.whl (6.4MB): 6.4MB downloaded
Installing collected packages: Django
  Found existing installation: Django 1.9c1
    Uninstalling Django:
      Successfully uninstalled Django
Compiling /home/tim/.virtualenvs/tmp-65a61cac481337a0/build/Django/django/conf/app_template/apps.py ...
  File "/home/tim/.virtualenvs/tmp-65a61cac481337a0/build/Django/django/conf/app_template/apps.py", line 4
    class {{ camel_case_app_name }}Config(AppConfig):
          ^
SyntaxError: invalid syntax

Compiling /home/tim/.virtualenvs/tmp-65a61cac481337a0/build/Django/django/conf/app_template/models.py ...
  File "/home/tim/.virtualenvs/tmp-65a61cac481337a0/build/Django/django/conf/app_template/models.py", line 1
    {{ unicode_literals }}from django.db import models
                             ^
SyntaxError: invalid syntax

Successfully installed Django
Cleaning up...

[Florian Apolloner]

On Monday, November 30, 2015 at 7:02:05 PM UTC+1, Tim Graham wrote:

Florian, your preference is to decline the change proposed by Chris and just deal with any support queries when people see a log like below? I understand it's not ideal to add workarounds for bugs in other software as the proposal does, but it seems like a pragmatic solution to me.

I am not against including it, but depending on how packaging works in Debian it could be possible to call pycompile with a proper exclusion list instead. The PR itself only works by accident, there are also files in project_template (namely the settings file and urls) which also should not get compiled (even if they are valid). So if you want to include the PR, please make sure it properly works for __all__ package data, not just the one with invalid data.

That said, the amount of issues I have seen in #django due to the debundling of pip in debian/ubuntu is not something I can count on one hand anymore. Most of the time I just gave up and suggested to install pip manually, all problems gone. The syntax error has popped up three times so far (iirc) and requires no fixing aside from educating people.

Cheers,
Florian

[Raphael Hertzog]

Hello,

I'm another Debian developer co-maintaining Django in Debian.

Le lundi 30 novembre 2015, Florian Apolloner a écrit :
> I am not against including it, but depending on how packaging works in
> Debian it could be possible to call pycompile with a proper exclusion list
> instead. The PR itself only works by accident, there are also files in

We can always add many exceptions but in the end any distro packager wants
to avoid exceptions as we want to automate as much as possible
installations of Python modules (and update to new upstream versions
to make them quickly available to users).

There's a case to be made that the whole process should probably not loose the
information coming setup.py but I'd argue that this information is not
really useful information for packagers until it's properly stored in the
filesystem by the module installation process.

And I would also argue that this would then need to be fixed at the Python
level so that Data is actually stored in separate directories
(/usr/share/something) and not mixed up with Python code.

All in all, I believe our request to be reasonable and I would like
you to consider applying Chris's PR or similar code.

Thank you!

> That said, the amount of issues I have seen in #django due to the
> debundling of pip in debian/ubuntu is not something I can count on one hand
> anymore. Most of the time I just gave up and suggested to install pip
> manually, all problems gone. The syntax error has popped up three times so
> far (iirc) and requires no fixing aside from educating people.

I find it weird to have such feelings when you go to such lengths to
implement a very nice LTS support policy for Django itself.

Cheers,
--
Raphaël Hertzog ◈ Writer/Consultant ◈ Debian Developer

Discover the Debian Administrator's Handbook:
→ http://debian-handbook.info/get/

[Carl Johnson]

FWIW, this also bit me. I have a deploy script that runs python -m compileall as part of its build process. It was pretty easy to work around by adding -x app_template, but why make everyone else work around Django? ISTM it would be better to call the files .py_template instead since they aren't valid Python.

[Tim Graham]

The current pull request proposes to make a backwards-incompatible change by chopping off any ".tpl" suffix in app or project template files. i.e. "If you are using custom templates and these templates already use a ``.tpl``  suffix, you will need to append an additional ``.tpl`` (ie. ``filename.tpl.tpl``)".

Do you find this change acceptable in a minor release (1.9.2)? If not, we could possibly limit the behavior to the templates in "django/conf".

https://github.com/django/django/pull/5735

[Claude Paroz]

Le mercredi 6 janvier 2016 19:34:07 UTC+1, Tim Graham a écrit :

The current pull request proposes to make a backwards-incompatible change by chopping off any ".tpl" suffix in app or project template files. i.e. "If you are using custom templates and these templates already use a ``.tpl``  suffix, you will need to append an additional ``.tpl`` (ie. ``filename.tpl.tpl``)".

Do you find this change acceptable in a minor release (1.9.2)? If not, we could possibly limit the behavior to the templates in "django/conf".

https://github.com/django/django/pull/5735

What about limiting chopping to ".py.tpl" => ".py"?

Claude

[Carl Johnson]

I think the template files should be shipped out as .py-tpl, since that's their type (a Python template), and the resulting final files should just have the extension renamed. Having two extension is just confusing and leads to questions like this one about other kinds of templates.

[Tim Graham]

Seems reasonable to me.