#1960 closed defect (fixed)

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

Reported by: daira Owned by: daira
Priority: minor Milestone: 1.10.1
Component: packaging Version: 1.10.0
Keywords: warning build install setuptools Cc:
Launchpad Bug:

Description (last modified by 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)

Change History (13)

comment:1 Changed at 2013-05-01T22:42:46Z by daira

  • Owner set to daira
  • Status changed from new to assigned
  • Version changed from unknown to 1.10.0

comment:2 Changed at 2013-05-14T18:20:00Z by daira

  • Description modified (diff)

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.

Version 0, edited at 2013-05-14T18:20:00Z by daira (next)

comment:3 follow-up: Changed at 2013-05-14T21:42:00Z by 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. ☹

comment:4 Changed at 2013-05-14T22:08:05Z by Daira Hopwood <david-sarah@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 2fa832b060ac8c4ceadb94d74d0ac5e0586e432c/trunk:

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

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

comment:5 in reply to: ↑ 3 ; follow-up: Changed at 2013-05-14T22:10:47Z by 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.

comment:6 Changed at 2013-05-14T22:16:25Z by daira

  • Keywords setuptools added

Blame where blame is due ;-)

comment:7 follow-up: Changed at 2013-05-14T22:21:20Z by 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@…>

comment:8 in reply to: ↑ 5 Changed at 2013-05-15T16:12:32Z by 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?

comment:9 in reply to: ↑ 7 Changed at 2013-05-15T16:13:44Z by 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?

comment:10 follow-ups: Changed at 2013-05-15T16:34:37Z by 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.

comment:11 Changed at 2013-05-17T00:16:34Z by 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).

comment:12 in reply to: ↑ 10 Changed at 2013-05-17T00:20:11Z by 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).

comment:13 in reply to: ↑ 10 Changed at 2013-05-17T00:23:03Z by 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.

Note: See TracTickets for help on using tickets.