spurious warnings that look a little like errors when building/installing 1.10

[daira]

package init file 'src/allmydata/web/static/__init__.py' not found (or not a regular file)
package init file 'src/allmydata/web/static/css/__init__.py' not found (or not a regular file)

[daira]

Due to the fix for #1969, there is now another warning about 'src/allmydata/web/static/css/__init__.py'. The simplest way to fix this is by creating dummy __init.py__ files in all three directories.

[zooko]

Isn't the right fix to tell setuptools (or distribute or "packaging" or "distutils" or whatever) that src/allmydata/web/static/img is not a package? Packages are for Python code. This is "package data", which is not a package. This stuff is kind of finicky and I don't remember all the details. ☹

[Daira Hopwood <david-sarah@…>]

In 2fa832b060ac8c4ceadb94d74d0ac5e0586e432c/trunk:

Add dummy init.py files for directories containing data. fixes #1960

Signed-off-by: Daira Hopwood <david-sarah@…>

[daira]

Replying to zooko:

Isn't the right fix to tell setuptools (or distribute or "packaging" or "distutils" or whatever) that src/allmydata/web/static/img is not a package? Packages are for Python code. This is "package data", which is not a package. This stuff is kind of finicky and I don't remember all the details. ☹

We already told it that these directories are package_data, so it's a setuptools bug that it still prints the warning. However it's not one that I particularly care about fixing, since it's easier to work around it.

[daira]

Blame where blame is due ;-)

[Daira Hopwood <david-sarah@…>]

In 2fa832b060ac8c4ceadb94d74d0ac5e0586e432c/trunk:

Add dummy init.py files for directories containing data. fixes #1960

Signed-off-by: Daira Hopwood <david-sarah@…>

[zooko]

Replying to daira:

Replying to zooko:

Isn't the right fix to tell setuptools (or distribute or "packaging" or "distutils" or whatever) that src/allmydata/web/static/img is not a package? Packages are for Python code. This is "package data", which is not a package. This stuff is kind of finicky and I don't remember all the details. ☹

We already told it that these directories are package_data, so it's a setuptools bug that it still prints the warning. However it's not one that I particularly care about fixing, since it's easier to work around it.

Wait, why do you say that it is a setuptools bug?

[zooko]

Replying to Daira Hopwood <david-sarah@…>:

In 2fa832b060ac8c4ceadb94d74d0ac5e0586e432c/trunk:

Add dummy init.py files for directories containing data. fixes #1960

Signed-off-by: Daira Hopwood <david-sarah@…>

I don't agree with this patch. I think it may cause other problems. Also, I think it is masking a bug which we might benefit by fixing instead of masking. Also, don't we usually require a sign-off from a reviewer other than the author before committing patches to trunk?

[zooko]

It looks like the warning message is trying to tell us that we've labeled src/allmydata/web/static/css and src/allmydata/web/static as "packages" in setup.py line 435 and then we've labeled src/allmydata/web/*.xhtml, src/allmydata/web/static, src/allmydata/web/static/css, and src/allmydata/web/static/img as "package data" in setup.py line 454.

This seems probably-wrong in a few ways. First of all, src/allmydata/web/static and anything underneath it is not a package, since it is not a way to make Python modules importable, and should probably not be included in "packages". People sometimes put files that they want at runtime like this into "packages" because that's a way to get the files included into dists, but it is not a reliable way to make those files open'able by our code at run-time, since it is intended only for making Python modules importable, not arbitrary files open'able. For example, if our app is bundled into a zip file by "bbfreeze" or one of those other "single-file-python-app" hacks, then Python modules located in such a package would still be importable, but arbitrary files in there probably wouldn't be open'able.

In any case, I would prefer not to declare src/allmydata/web/static And its subdirectories as being "packages" unless there is a good reason to do so, since I think it is semantically wrong according to Python packaging standards/customs/voodoo.

Next weirdness: why is src/allmydata/web/static/css included in "packages" but src/allmydata/web/static/img isn't?

Okay, my current guess is that the right thing to do is revert [2fa832b060ac8c4ceadb94d74d0ac5e0586e432c/trunk] and remove allmydata.web.static and allmydata.web.static.css from the list of packages on setup.py line 435. This should avoid warnings, should make it so that import allmydata.web.static gets an ImportError, and will force us to figure out how to include the static files in such a way that they would be open'able *even* from within a packed-into-a-single-file-app or other weird contexts.

Question: what would it take to have warnings like this noticed and flagged by buildbot? We have some tests along those lines on buildbot, but I originally wrote them in-line into the buildmaster config, which was horrible, and I think Brian may have refactored them into files in misc... Yes, there they are in misc/build_helpers. Oh, actually, couldn't we just have the standard build command on buildbot treat warnings like these are errors? It is probably a setting in the buildmaster config.

[daira]

You're right that I should have waited for review before committing to trunk, sorry.

Both the entry in "packages" and the entry in "package_data" are necessary. I just tested removing the former, and that does not install the files correctly. That informs my conclusion that it's a setuptools bug. (Note that setuptools certainly has enough information to see that there are no .py[co] files in the directory, and so the absence of a __init__.py file is harmless.)

Next weirdness: why is src/allmydata/web/static/css included in "packages" but src/allmydata/web/static/img isn't?

I don't know why it appeared to work for me before without allmydata.web.static.img in "packages". In the testing I did just now, it doesn't copy the contents of the src/allmydata/web/static/img/ directory without that when doing a system install. (So, another patch will be necessary to add it.)

Note that we don't open data files directly as files; we open them using the pkg_resources module, which in principle should be able to read from zip files (but this is only relevant for bb-freeze and similar I think, because of the zip_safe=False flag).

[daira]

Replying to zooko:

Question: what would it take to have warnings like this noticed and flagged by buildbot? We have some tests along those lines on buildbot, but I originally wrote them in-line into the buildmaster config, which was horrible, and I think Brian may have refactored them into files in misc... Yes, there they are in misc/build_helpers. Oh, actually, couldn't we just have the standard build command on buildbot treat warnings like these are errors? It is probably a setting in the buildmaster config.

We could, but I'd be concerned about too many false positives due to warnings from building dependencies (which are not our problem).

[daira]

Replying to zooko:

First of all, src/allmydata/web/static and anything underneath it is not a package, since it is not a way to make Python modules importable, and should probably not be included in "packages".

By the way I think you're mistaken about this; a data-only package is still a package according to setuptools and pkg_resources conventions.