'make clean' shouldn't remove the src/allmydata_tahoe.egg-info directory #2092

New issue

Closed

opened 2013-10-09 11:13:59 +00:00 by bertagaz · 21 comments

bertagaz commented

2013-10-09 11:13:59 +00:00

Owner

When running make clean, the process removes the src/allmydata_tahoe.egg-info directory. This directory is distributed within Tahoe-LAFS sources, which is inconsistent with the fact that make clean should only reset the Tahoe-LAFS directory to a freshly uncompressed state.

This doesn't help in packaging Tahoe-LAFS in distributions like Debian, where tools usually run a make clean before attempting to build. They fail after this first clean, because they detected this directory was removed but is part of the release tarball.

So either make clean shouldn't take care of this directory, or it has to be removed from the release tarball.

When running `make clean`, the process removes the `src/allmydata_tahoe.egg-info directory`. This directory is distributed within Tahoe-LAFS sources, which is inconsistent with the fact that `make clean` should only reset the Tahoe-LAFS directory to a freshly uncompressed state. This doesn't help in packaging Tahoe-LAFS in distributions like Debian, where tools usually run a `make clean` before attempting to build. They fail after this first clean, because they detected this directory was removed but is part of the release tarball. So either `make clean` shouldn't take care of this directory, or it has to be removed from the release tarball.

tahoe-lafs added the

labels 2013-10-09 11:13:59 +00:00

tahoe-lafs added this to the undecided milestone 2013-10-09 11:13:59 +00:00

tahoe-lafs changed title from ~~,'make clean' shouldn't remove the src/allmydata_tahoe.egg-info directory~~ to 'make clean' shouldn't remove the src/allmydata_tahoe.egg-info directory

2013-10-09 11:15:24 +00:00

zooko commented

2013-10-10 01:55:13 +00:00

(https://github.com/tahoe-lafs/tahoe-lafs/pull/63)

daira commented

2013-10-10 12:57:11 +00:00

src/allmydata_tahoe.egg-info is a generated directory that doesn't exist in the checked-in source. Removing review-needed so that this doesn't get committed before I've checked whether this directory should exist in tarballs.

`src/allmydata_tahoe.egg-info` is a generated directory that doesn't exist in the checked-in source. Removing `review-needed` so that this doesn't get committed before I've checked whether this directory should exist in tarballs.

daira commented

2013-10-10 13:13:12 +00:00

A tarball with src/allmydata_tahoe.egg-info removed builds fine and passes tests. So I conclude that this directory should not be in future sdist tarballs. Note that it is standard behaviour of python setup.py sdist to include it, but that is probably a bug in setuptools/zetuptoolz.

A tarball with `src/allmydata_tahoe.egg-info` removed builds fine and passes tests. So I conclude that this directory should not be in future sdist tarballs. Note that it is standard behaviour of `python setup.py sdist` to include it, but that is probably a bug in setuptools/zetuptoolz.

daira added

p/major

and removed

p/normal

labels 2013-10-10 13:13:12 +00:00

daira modified the milestone from undecided to 1.11.0

2013-10-10 13:13:12 +00:00

daira changed title from ~~'make clean' shouldn't remove the src/allmydata_tahoe.egg-info directory~~ to the src/allmydata_tahoe.egg-info directory shouldn't be included in sdist tarballs

2013-10-10 13:25:32 +00:00

daira commented

2013-10-10 13:30:52 +00:00

Alternatively, the Debian etc. packaging should remove the egg-info directory from the source package only. (I emphasize that because Debian packaging of some Python packages has incorrectly tried to avoid installing egg-info directories.)

Alternatively, the Debian etc. packaging should remove the egg-info directory *from the source package only*. (I emphasize that because Debian packaging of some Python packages has incorrectly tried to avoid installing ``egg-info`` directories.)

zooko commented

2013-10-10 15:00:42 +00:00

Either way is fine with me!

zooko commented

2013-10-10 15:02:32 +00:00

Replying to daira:

A tarball with src/allmydata_tahoe.egg-info removed builds fine and passes tests. So I conclude that this directory should not be in future sdist tarballs. Note that it is standard behaviour of python setup.py sdist to include it, but that is probably a bug in setuptools/zetuptoolz.

Actually I disagree with the above. The directory in question is metadata declaring in machine-readable, standard format some facts about the distribution. The fact that it is not necessary to pass tests doesn't mean it should be removed, and I don't see any justification for assuming that its inclusion is a bug.

Replying to [daira](/tahoe-lafs/trac/issues/2092#issuecomment-395426): > A tarball with `src/allmydata_tahoe.egg-info` removed builds fine and passes tests. So I conclude that this directory should not be in future sdist tarballs. Note that it is standard behaviour of `python setup.py sdist` to include it, but that is probably a bug in setuptools/zetuptoolz. Actually I disagree with the above. The directory in question is metadata declaring in machine-readable, standard format some facts about the distribution. The fact that it is not necessary to pass tests doesn't mean it should be removed, and I don't see any justification for assuming that its inclusion is a bug.

daira commented

2013-10-10 15:05:36 +00:00

Replying to [zooko]comment:9:

Replying to daira:

A tarball with src/allmydata_tahoe.egg-info removed builds fine and passes tests. So I conclude that this directory should not be in future sdist tarballs. Note that it is standard behaviour of python setup.py sdist to include it, but that is probably a bug in setuptools/zetuptoolz.

Actually I disagree with the above. The directory in question is metadata declaring in machine-readable, standard format some facts about the distribution.

It's regenerated by python setup.py build. We don't include anything else that is generated, apart from source:src/allmydata/_version.py which is only generated when building from a git checkout.

Replying to [zooko]comment:9: > Replying to [daira](/tahoe-lafs/trac/issues/2092#issuecomment-395426): > > A tarball with `src/allmydata_tahoe.egg-info` removed builds fine and passes tests. So I conclude that this directory should not be in future sdist tarballs. Note that it is standard behaviour of `python setup.py sdist` to include it, but that is probably a bug in setuptools/zetuptoolz. > > Actually I disagree with the above. The directory in question is metadata declaring in machine-readable, standard format some facts about the distribution. It's regenerated by `python setup.py build`. We don't include anything else that is generated, apart from source:src/allmydata/_version.py which is only generated when building from a git checkout.

zooko commented

2013-10-10 15:06:12 +00:00

I don't understand what the issue is here. It sounds like there is a difference of opinion between Debian, which thinks that "make clean" should return the directory to the state it was after it was untarred, and Daira, who thinks that "make clean" should return the directory to the state it was after it was checked out from revision control. Those can't both be true.

zooko commented

2013-10-10 15:07:12 +00:00

Replying to zooko:

I don't understand what the issue is here. It sounds like there is a difference of opinion between Debian, which thinks that "make clean" should return the directory to the state it was after it was untarred, and Daira, who thinks that "make clean" should return the directory to the state it was after it was checked out from revision control. Those can't both be true.

Following-up to myself, the reason those can't both be true is that a source dist is not just a tarball of a source checkout. It also has a version number and (currently) metadata auto-generated.

zooko commented

2013-10-10 15:09:11 +00:00

The second question is the only one that is blocking Debian, and I don't use "make clean" and I don't care what "make clean" does, so I propose that we make it do what Debian wants it to do, thus answering question 2, and leave question 1 unchanged (in this ticket).

Another self-follow-up: there are two questions here: 1. should generated stuff (in this case the Python packaging metadata) go into the source dist, and 2. should "make clean" return the directory to the state of a source checkout, or return the directory to the state of a source dist unpack? The second question is the only one that is blocking Debian, and I don't use "make clean" and I don't care what "make clean" does, so I propose that we make it do what Debian wants it to do, thus answering question 2, and leave question 1 unchanged (in this ticket).

daira commented

2013-10-10 15:15:01 +00:00

Replying to zooko:

I don't understand what the issue is here. It sounds like there is a difference of opinion between Debian, which thinks that "make clean" should return the directory to the state it was after it was untarred, and Daira, who thinks that "make clean" should return the directory to the state it was after it was checked out from revision control. Those can't both be true.

I have no disagreement with Debian's policy here. Debian apparently requires that running make clean after a build should not delete files that were in the source package; I agree with that. (Note that this only partially constrains the answer to your second question in comment:13.) It's also clearly necessary that a source package contains all the files necessary to build -- so it must include src/allmydata/_version.py, but need not include src/allmydata_tahoe.egg_info.

Replying to [zooko](/tahoe-lafs/trac/issues/2092#issuecomment-395435): > I don't understand what the issue is here. It sounds like there is a difference of opinion between Debian, which thinks that "make clean" should return the directory to the state it was after it was untarred, and Daira, who thinks that "make clean" should return the directory to the state it was after it was checked out from revision control. Those can't both be true. I have no disagreement with Debian's policy here. Debian apparently requires that running `make clean` after a build should not delete files that were in the source package; I agree with that. (Note that this only partially constrains the answer to your second question in comment:13.) It's also clearly necessary that a source package contains all the files necessary to build -- so it must include `src/allmydata/_version.py`, but need not include `src/allmydata_tahoe.egg_info`.

gdt commented

2013-10-10 15:21:15 +00:00

Author

Owner

Replying to [zooko]comment:12:

Historically, "make clean" should return to the unpacked-distfile state, and "make distclean" to the revision control system state.

Replying to [zooko]comment:12: Historically, "make clean" should return to the unpacked-distfile state, and "make distclean" to the revision control system state.

dstufft commented

2013-10-10 15:23:18 +00:00

Author

Owner

A Python sdist should contain the .egg-info (or in the future .dist-info). While right now it will technically work without it, there is a push towards making setup.py a build dependency which generates static metadata that is placed in the generated distributions.

I don't know 100% what form this will take but setuptools will be updated to do it automatically so if you start mucking with egg-info it's possible in the future you'll end up with uninstallable distributions, or distributions that require a setup.py fallback.

The long term goal is that the only place setup.py is ever executed is the machine of the person who packages the project.

A Python sdist should contain the ``.egg-info`` (or in the future ``.dist-info``). While right now it will technically work without it, there is a push towards making setup.py a build dependency which generates static metadata that is placed in the generated distributions. I don't know 100% what form this will take but setuptools will be updated to do it automatically so if you start mucking with egg-info it's possible in the future you'll end up with uninstallable distributions, or distributions that require a setup.py fallback. The long term goal is that the only place ``setup.py`` is ever executed is the machine of the person who packages the project.

daira commented

2013-10-10 15:26:03 +00:00

Replying to dstufft:

A Python sdist should contain the .egg-info (or in the future .dist-info).

This assertion is good enough for me; I withdraw my objection to including it. But let's also add make distclean as mentioned by gdt.

Replying to [dstufft](/tahoe-lafs/trac/issues/2092#issuecomment-395440): > A Python sdist should contain the ``.egg-info`` (or in the future ``.dist-info``). This assertion is good enough for me; I withdraw my objection to including it. But let's also add `make distclean` as mentioned by gdt.

daira commented

2013-10-10 15:27:12 +00:00

removed

zooko commented

2013-10-10 15:52:15 +00:00

I'm fine with adding "make distclean", if it doesn't already exist, and changing "make clean" to not rm the egg-info (as in https://github.com/tahoe-lafs/tahoe-lafs/pull/63). Is there anything else we should do at the same time?

I'm fine with adding "`make distclean`", if it doesn't already exist, and changing "`make clean`" to not rm the egg-info (as in <https://github.com/tahoe-lafs/tahoe-lafs/pull/63>). Is there anything else we should do at the same time?

daira commented

2013-10-10 16:52:03 +00:00

Merged in [5b55ccc526ac9abf9b2189baeff796e41196fb30/trunk].

daira commented

2013-10-10 16:54:43 +00:00

Please review https://github.com/tahoe-lafs/tahoe-lafs/pull/64 for make distclean. (It removes _version.py and _appname.py as well, and I've verified that it is still possible to build and test successfully from a git checkout after this.)

Please review <https://github.com/tahoe-lafs/tahoe-lafs/pull/64> for `make distclean`. (It removes `_version.py` and `_appname.py` as well, and I've verified that it is still possible to build and test successfully from a git checkout after this.)

zooko was assigned by daira

2013-10-10 16:54:43 +00:00

zooko commented

2013-10-10 17:00:28 +00:00

reviewed: https://github.com/tahoe-lafs/tahoe-lafs/pull/64#commitcomment-4300256

reviewed: <https://github.com/tahoe-lafs/tahoe-lafs/pull/64#commitcomment-4300256>

daira commented

2013-10-10 18:43:47 +00:00

Merged in [90c1b8fb570df5c116a76412e7e4ca55bd5d5a8e/trunk]. Thanks!

daira added the

r/fixed

label 2013-10-10 18:43:47 +00:00

daira closed this issue

2013-10-10 18:43:47 +00:00

daira commented

2013-10-10 18:52:34 +00:00

Updated the comment for make clean: [7f10bf07df8b936e0d62f123072e704bfb8cc963/trunk]

Updated the comment for `make clean`: [7f10bf07df8b936e0d62f123072e704bfb8cc963/trunk]

daira changed title from ~~the src/allmydata_tahoe.egg-info directory shouldn't be included in sdist tarballs~~ to 'make clean' shouldn't remove the src/allmydata_tahoe.egg-info directory