tahoe-lafs/trac

we can end up importing the wrong version of a dependency even though the right one is "already the active version in easy-install.pth" #1190

New issue

Closed

opened 2010-08-30 20:19:53 +00:00 by daira · 53 comments

daira commented

2010-08-30 20:19:53 +00:00

(Split from /tahoe-lafs/trac/issues/27009#comment:380891.)

Log for the test step on 'Ruben Fedora':

Searching for pycrypto==2.1.0
Best match: pycrypto 2.1.0
Processing pycrypto-2.1.0-py2.7-linux-x86_64.egg
pycrypto 2.1.0 is already the active version in easy-install.pth

Using /home/buildbot/tahoe/Ruben Fedora/build/support/lib/python2.7/
site-packages/pycrypto-2.1.0-py2.7-linux-x86_64.egg
[...]
running trial
[...]
pkg_resources.VersionConflict: (pycrypto 2.2 (/usr/lib64/python2.7/
site-packages), Requirement.parse('pycrypto==2.0.1,==2.1,>=2.3'))

I believe this is due to the same underlying issue as #1137, that is, the handling of sys.path by the setuptools-installed site.py and .pth files being broken. If the path handling were working correctly, it should not have been possible that "pycrypto 2.1.0 is already the active version in easy-install.pth", but we end up importing pycrypto 2.2 when running trial.

Here's an instance of the same problem with foolscap on 'Shawn jaunty':

Searching for foolscap==0.5.1
Best match: foolscap 0.5.1
Processing foolscap-0.5.1-py2.6.egg
foolscap 0.5.1 is already the active version in easy-install.pth
Installing flappclient script to support/bin
Installing flogtool script to support/bin
Installing flappserver script to support/bin

Using /var/lib/buildbot/tahoe/mordsith/build/support/lib/python2.6/
site-packages/foolscap-0.5.1-py2.6.egg
[...]
running trial
[...]
pkg_resources.VersionConflict: (foolscap 0.4.2 (/usr/local/lib/python2.6/
dist-packages/foolscap-0.4.2-py2.6.egg),
Requirement.parse('foolscap[secure_connections]>=0.5.1'))

So this is not specific to pycrypto or due to the use of a disjunctive requirement. I think I have also seen this when running bin/tahoe, not just for setup.py trial.

(Split from [/tahoe-lafs/trac/issues/27009](/tahoe-lafs/trac/issues/27009)#[comment:380891](/tahoe-lafs/trac/issues/1190#issuecomment-380891).) [Log for the test step on 'Ruben Fedora'](http://tahoe-lafs.org/buildbot/builders/Ruben%20Fedora/builds/610/steps/test/logs/stdio): ``` Searching for pycrypto==2.1.0 Best match: pycrypto 2.1.0 Processing pycrypto-2.1.0-py2.7-linux-x86_64.egg pycrypto 2.1.0 is already the active version in easy-install.pth Using /home/buildbot/tahoe/Ruben Fedora/build/support/lib/python2.7/ site-packages/pycrypto-2.1.0-py2.7-linux-x86_64.egg [...] running trial [...] pkg_resources.VersionConflict: (pycrypto 2.2 (/usr/lib64/python2.7/ site-packages), Requirement.parse('pycrypto==2.0.1,==2.1,>=2.3')) ``` I believe this is due to the same underlying issue as #1137, that is, the handling of `sys.path` by the setuptools-installed `site.py` and `.pth` files being broken. If the path handling were working correctly, it should not have been possible that "pycrypto 2.1.0 is already the active version in easy-install.pth", but we end up importing pycrypto 2.2 when running trial. Here's an instance of the same problem with foolscap [on 'Shawn jaunty'](http://tahoe-lafs.org/buildbot/builders/Shawn%20jaunty%20amd64/builds/563/steps/test/logs/stdio): ``` Searching for foolscap==0.5.1 Best match: foolscap 0.5.1 Processing foolscap-0.5.1-py2.6.egg foolscap 0.5.1 is already the active version in easy-install.pth Installing flappclient script to support/bin Installing flogtool script to support/bin Installing flappserver script to support/bin Using /var/lib/buildbot/tahoe/mordsith/build/support/lib/python2.6/ site-packages/foolscap-0.5.1-py2.6.egg [...] running trial [...] pkg_resources.VersionConflict: (foolscap 0.4.2 (/usr/local/lib/python2.6/ dist-packages/foolscap-0.4.2-py2.6.egg), Requirement.parse('foolscap[secure_connections]>=0.5.1')) ``` So this is not specific to pycrypto or due to the use of a disjunctive requirement. I think I have also seen this when running `bin/tahoe`, not just for `setup.py trial`.

daira added the

labels 2010-08-30 20:19:53 +00:00

daira added this to the undecided milestone 2010-08-30 20:19:53 +00:00

zooko commented

2010-10-15 06:58:50 +00:00

Attachment requires.patch.txt (5400 bytes) added

**Attachment** requires.patch.txt (5400 bytes) added

requires.patch.txt

5.3 KiB

zooko commented

2010-10-15 07:11:56 +00:00

Attachment ignore-DNF.patch.txt (965 bytes) added

**Attachment** ignore-DNF.patch.txt (965 bytes) added

ignore-DNF.patch.txt

965 B

zooko commented

2010-10-18 23:41:34 +00:00

I think I have a working isolated, deterministic test of this functionality now, and I think I understand how to fix it, too.

I think I have a working isolated, deterministic test of this functionality now, and I *think* I understand how to fix it, too.

zooko modified the milestone from undecided to 1.8.1

2010-10-18 23:41:34 +00:00

warner commented

2010-10-27 06:42:17 +00:00

Attachment 1190-nospawn.darcs.patch (7957 bytes) added

import+exec twistd, instead of find+spawn, to remove an intermediate process

**Attachment** 1190-nospawn.darcs.patch (7957 bytes) added import+exec twistd, instead of find+spawn, to remove an intermediate process

1190-nospawn.darcs.patch

7.8 KiB

zooko commented

2010-10-27 07:43:02 +00:00

A convenient way to do that might be to compare buildbot results for branch=ticket1190 vs. for branch=trunk.

Committed to [the ticket1190 branch]source:ticket1190 as [20101027052730-92b7f-4fb13712e23ceaef5b42474f599278f142010bc9], [20101027060443-92b7f-32a95aeca42279a788bff2683568b810f8e73fb7], changeset:ac3b26ecf29c08cb where it made the intended test go from [red](http://tahoe-lafs.org/buildbot/builders/Brian%20ubuntu-i386%20linode/builds/8/steps/test/logs/stdio) to [green](http://tahoe-lafs.org/buildbot/builders/Brian%20ubuntu-i386%20linode/builds/9/steps/test/logs/stdio) but also unfortunately I think it might have made different tests on different buildslaves go from green to red. I'm sorry that I'm not certain about that latter part. I'm very sleepy! Maybe we can sort it all out tomorrow. A convenient way to do that might be to compare [buildbot results for branch=ticket1190](http://tahoe-lafs.org/buildbot/waterfall?show_events=false&branch=ticket1190) vs. [for branch=trunk](http://tahoe-lafs.org/buildbot/waterfall?show_events=false&branch=trunk).

zooko removed their assignment 2010-10-27 07:43:16 +00:00

daira was assigned by zooko

2010-10-27 07:43:16 +00:00

zooko commented

2010-10-27 13:20:53 +00:00

Okay in the cold light of morning I see that the current failures on [the ticket1190 branch]source:ticket1190 are of three kinds:

a bug in darcsver so that the version number comes out as "unknown" or "0.0.0": http://tahoe-lafs.org/buildbot/builders/Arthur%20lenny%20c7%2032bit/builds/503/steps/test/logs/stdio
a UserWarning which triggers test_no_noise to go red: arthur build 503
a bug where the system-wide installation of allmydata-tahoe is getting added to the working set even though it is incompatible with the requirement of allmydata-tahoe==$THIS_PARTICULAR_VERSION: Kyle OpenBSD-4.6 amd64 build 426 bin/tahoe --version, Kyle OpenBSD-4.6 amd64 build 426 python setup.py test, FreeStorm Win7-amd64-mingw-py2.6 build 111 python setup.py test

Okay in the cold light of morning I see that the current failures on [the ticket1190 branch]source:ticket1190 are of three kinds: * a bug in darcsver so that the version number comes out as "unknown" or "0.0.0": <http://tahoe-lafs.org/buildbot/builders/Arthur%20lenny%20c7%2032bit/builds/503/steps/test/logs/stdio> * a UserWarning which triggers `test_no_noise` to go red: [arthur build 503](http://tahoe-lafs.org/buildbot/builders/lucid-amd64/builds/18/steps/test/logs/stdio) * a bug where the system-wide installation of `allmydata-tahoe` is getting added to the working set even though it is incompatible with the requirement of `allmydata-tahoe==$THIS_PARTICULAR_VERSION`: [Kyle OpenBSD-4.6 amd64 build 426 bin/tahoe --version](http://tahoe-lafs.org/buildbot/builders/Kyle%20OpenBSD-4.6%20amd64/builds/426/steps/tahoe-version/logs/stdio), [Kyle OpenBSD-4.6 amd64 build 426 python setup.py test](http://tahoe-lafs.org/buildbot/builders/Kyle%20OpenBSD-4.6%20amd64/builds/426/steps/test/logs/stdio), [FreeStorm Win7-amd64-mingw-py2.6 build 111 python setup.py test](http://tahoe-lafs.org/buildbot/builders/FreeStorm%20Win7-amd64-mingw%20py2.6/builds/111/steps/test/logs/stdio)

daira commented

2010-10-27 16:32:28 +00:00

Author

[4764/ticket1190] and [4766/ticket1190] look ok, or at least worth trying.

[4765/ticket1190] adds a *requires* line to newly generated .tac files. This is liable to cause the same kind of forward-compatibility problem we had trying to change the appname in #1159 -- i.e. when we change the requirements, existing .tac files will still reflect the requirements at the time they were generated.

(This will cause even more subtle problems than the appname change, because the effect will be to allow earlier versions of dependencies than intended, and so regress bugs we thought we'd fixed, rather than always failing when an old .tac is used.)

I'm more and more convinced that .tac files are more trouble than they're worth and that we should stop using them (see /tahoe-lafs/trac/issues/27103#comment:-1). Node directories should contain only data, not code.

[4764/ticket1190] and [4766/ticket1190] look ok, or at least worth trying. [4765/ticket1190] adds a `*requires*` line to newly generated `.tac` files. This is liable to cause the same kind of forward-compatibility problem we had trying to change the `appname` in #1159 -- i.e. when we change the requirements, existing `.tac` files will still reflect the requirements at the time they were generated. (This will cause even more subtle problems than the `appname` change, because the effect will be to allow earlier versions of dependencies than intended, and so regress bugs we thought we'd fixed, rather than always failing when an old `.tac` is used.) I'm more and more convinced that `.tac` files are more trouble than they're worth and that we should stop using them (see [/tahoe-lafs/trac/issues/27103](/tahoe-lafs/trac/issues/27103)#[comment:-1](/tahoe-lafs/trac/issues/1190#issuecomment--1)). Node directories should contain only data, not code.

daira removed their assignment 2010-10-27 16:32:28 +00:00

zooko was assigned by daira

2010-10-27 16:32:28 +00:00

warner commented

2010-10-27 22:38:34 +00:00

I added a hefty comment to /tahoe-lafs/trac/issues/27103 . Yes, it's time for .tac files to die, and certainly we shouldn't be adding any new text to them, and double-certainly we shouldn't be touching existing ones (or relying upon having specific new code in them).

Any pkg_resources stuff should be done in tahoe start before transferring control to twistd. This is possible with the import+call patch, and is probably impossible with the old spawn-twistd approach, so we should start by landing that one.

I added a hefty comment to [/tahoe-lafs/trac/issues/27103](/tahoe-lafs/trac/issues/27103) . Yes, it's time for .tac files to die, and certainly we shouldn't be adding any new text to them, and double-certainly we shouldn't be touching existing ones (or relying upon having specific new code in them). Any `pkg_resources` stuff should be done in `tahoe start` before transferring control to twistd. This is possible with the import+call patch, and is probably impossible with the old spawn-twistd approach, so we should start by landing that one.

warner commented

2010-10-28 00:51:51 +00:00

eeargh. Please don't use *requires*. It appears to be a completely undocumented hack on top of the unpleasant hack that setuptools already is. The only information I could find about it is here. Let's find a way to drain and delete the .tac file and do any sys.path munging in bin/tahoe before tahoe start tries to import anything of interest.

eeargh. Please don't use `*requires*`. It appears to be a completely undocumented hack on top of the unpleasant hack that setuptools already is. The only information I could find about it is [here](http://mail.python.org/pipermail/distutils-sig/2007-September/008195.html). Let's find a way to drain and delete the `.tac` file and do any sys.path munging in `bin/tahoe` before `tahoe start` tries to import anything of interest.

daira commented

2010-10-28 02:12:03 +00:00

Author

In changeset:270322ad4762f47f:

startstop_node.py: pyflakes import fix. refs #1190

In changeset:270322ad4762f47f: ``` startstop_node.py: pyflakes import fix. refs #1190 ```

daira commented

2010-10-28 02:39:42 +00:00

Author

IIUC, when (and if) twisted daemonizes, the child process should inherit the sys.path that was munged by the script that ran the parent process. Therefore zooko's *requires* hack should work (modulo being completely undocumented). But if that's correct, then we only need [4764/ticket1190] and the change to setup.py from [4765/ticket1190], not the change to the .tac file generation.

(We could get rid of .tac files by adapting the code used for tahoe run, I think, but that's not this ticket.)

I landed the import+call patch on trunk in changeset:ac3b26ecf29c08cb, and fixed a pyflakes warning that it introduced. This seems to have (accidentally?) fixed the test failure in <http://tahoe-lafs.org/buildbot/builders/Ruben%20Fedora/builds/639/steps/test/logs/stdio>; we [now](http://tahoe-lafs.org/buildbot/builders/Ruben%20Fedora/builds/640/steps/test/logs/stdio) have no warning on the Ruben Fedora builder due to reimporting twisted, and so `test_runner.RunNode.test_client_no_noise` passes. IIUC, when (and if) twisted daemonizes, the child process should inherit the `sys.path` that was munged by the script that ran the parent process. Therefore zooko's `*requires*` hack should work (modulo being completely undocumented). But if that's correct, then we only need [4764/ticket1190] and the change to `setup.py` from [4765/ticket1190], *not* the change to the .tac file generation. (We could get rid of .tac files by adapting the code used for `tahoe run`, I think, but that's not this ticket.)

daira commented

2010-10-28 03:04:13 +00:00

Author

I reverted the changes to .tac generation on the ticket1190 branch: [4768/ticket1190].

Build results: http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1190

I reverted the changes to `.tac` generation on the ticket1190 branch: [4768/ticket1190]. Build results: <http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1190>

zooko commented

2010-10-28 06:28:59 +00:00

I'm not completely sure I understand this, but I think these patches are wrongly thinking that run_trial.py wants to be run from the build/ directory: [20101028045609-93fa1-4ecc4f29843251b644e2f94a3c36d8eeeb91a3ab], [20101028040404-93fa1-adf26e78fa02437caa930936c33d8bd4346d7f9b], [20101028051106-93fa1-07679d72a9d09448744c1883ecf5fccc1c0110a1]. Instead I think it wants to be run from the src/ directory (or the egg, or the prefixinstall). Note that it sometimes prints out an error message like the following:

HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190/build$ python ../misc/build_helpers/test-with-fake-pkg.py 

Traceback (most recent call last):
  File "/Users/zooko/playground/tahoe-lafs/ticket1190/src/../misc/build_helpers/run_trial.py", line 81, in <module>
    raise AssertionError(msg)
AssertionError: We seem to be testing the code at '/Users/zooko/playground/tahoe-lafs/ticket1190'
(according to the source filename '/Users/zooko/playground/tahoe-lafs/ticket1190/src/allmydata/test/test_base62.py'),
but expected to be testing the code at '/Users/zooko/playground/tahoe-lafs/ticket1190/src'.
This script needs to be run from the source directory to be tested.

Which is why I think it wants to have PWD be the src dir. This message was added in this patch: changeset:3af6f19cb0f02fb4.

I will attach the "../misc/build_helpers/test-with-fake-pkg.py" file which was used to generate the above output.

I'm not completely sure I understand this, but I *think* these patches are wrongly thinking that run_trial.py wants to be run from the build/ directory: [20101028045609-93fa1-4ecc4f29843251b644e2f94a3c36d8eeeb91a3ab], [20101028040404-93fa1-adf26e78fa02437caa930936c33d8bd4346d7f9b], [20101028051106-93fa1-07679d72a9d09448744c1883ecf5fccc1c0110a1]. Instead I think it wants to be run from the src/ directory (or the egg, or the prefixinstall). Note that it sometimes prints out an error message like the following: ``` HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190/build$ python ../misc/build_helpers/test-with-fake-pkg.py Traceback (most recent call last): File "/Users/zooko/playground/tahoe-lafs/ticket1190/src/../misc/build_helpers/run_trial.py", line 81, in <module> raise AssertionError(msg) AssertionError: We seem to be testing the code at '/Users/zooko/playground/tahoe-lafs/ticket1190' (according to the source filename '/Users/zooko/playground/tahoe-lafs/ticket1190/src/allmydata/test/test_base62.py'), but expected to be testing the code at '/Users/zooko/playground/tahoe-lafs/ticket1190/src'. This script needs to be run from the source directory to be tested. ``` Which is why I think it wants to have PWD be the src dir. This message was added in this patch: changeset:3af6f19cb0f02fb4. I will attach the "../misc/build_helpers/test-with-fake-pkg.py" file which was used to generate the above output.

zooko commented

2010-10-28 06:29:50 +00:00

Attachment test-with-fake-pkg.py (1301 bytes) added

**Attachment** test-with-fake-pkg.py (1301 bytes) added

test-with-fake-pkg.py

1.3 KiB

zooko commented

2010-10-28 06:31:02 +00:00

Oh, the output in comment:380895, made using attachment:test-with-fake-pkg.py, also had this patch applied to source:misc/build_helpers/run_trial.py:

HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190$ darcs whatsnew -u
hunk ./misc/build_helpers/run_trial.py 16
         if mo:
             return mo.group(1)
 
-version = read_version_py(os.path.join('lib', 'allmydata', '_version.py'))
+version = read_version_py(os.path.join('allmydata', '_version.py'))
 
 APPNAME='allmydata-tahoe'
 
hunk ./misc/build_helpers/run_trial.py 24
 # requirements.
 
 adglobals = {}
-execfile(os.path.join('lib', 'allmydata', '_auto_deps.py'), adglobals)
+execfile(os.path.join('allmydata', '_auto_deps.py'), adglobals)
 install_requires = adglobals['install_requires']
 
 __requires__ = [APPNAME + '==' + version] + install_requires

Oh, the output in [comment:380895](/tahoe-lafs/trac/issues/1190#issuecomment-380895), made using [attachment:test-with-fake-pkg.py](/tahoe-lafs/trac/attachments/000078ac-87a5-8831-a3f0-c4f337729367), also had this patch applied to source:misc/build_helpers/run_trial.py: ``` HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190$ darcs whatsnew -u hunk ./misc/build_helpers/run_trial.py 16 if mo: return mo.group(1) -version = read_version_py(os.path.join('lib', 'allmydata', '_version.py')) +version = read_version_py(os.path.join('allmydata', '_version.py')) APPNAME='allmydata-tahoe' hunk ./misc/build_helpers/run_trial.py 24 # requirements. adglobals = {} -execfile(os.path.join('lib', 'allmydata', '_auto_deps.py'), adglobals) +execfile(os.path.join('allmydata', '_auto_deps.py'), adglobals) install_requires = adglobals['install_requires'] __requires__ = [APPNAME + '==' + version] + install_requires ```

zooko commented

2010-10-28 06:34:45 +00:00

I updated the buildmaster config to turn on test-from-egg and test-from-prefixdir on all builders and triggered a build of all builders on the ticket1190 branch. Goodnight! :-)

zooko commented

2010-10-28 17:46:41 +00:00

I ran the build steps locally and this shows me more clearly the problem. The filesystem layout for the test-from-egg step is:

HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190/egginstalldir$ find . \( -name _version.py -o -name _auto_deps.py \) 
./allmydata_tahoe-1.8.0_r4776-py2.6.egg/allmydata/_auto_deps.py
./allmydata_tahoe-1.8.0_r4776-py2.6.egg/allmydata/_version.py
./foolscap-0.5.1-py2.6.egg/foolscap/_version.py

For the test-from-prefixdir step it is:

HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190/prefixinstalldir$ find . \( -name _version.py -o -name _auto_deps.py \)
./lib/python2.6/site-packages/allmydata/_auto_deps.py
./lib/python2.6/site-packages/allmydata/_version.py

and finally for the nascent test-with-fake-pkg.py it would be:

HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190$ find . \( -name _version.py -o -name _auto_deps.py \) 
./build/lib/allmydata/_auto_deps.py
./build/lib/allmydata/_version.py
./darcsver-1.6.3.egg/darcsver/_version.py
./setuptools_darcs-1.2.11-py2.6.egg/setuptools_darcs/_version.py
./setuptools_trial-0.5.9.egg/setuptools_trial/_version.py
./src/allmydata/_auto_deps.py
./src/allmydata/_version.py
./support/lib/python2.6/site-packages/darcsver-1.6.3.egg/darcsver/_version.py
./support/lib/python2.6/site-packages/foolscap-0.5.1-py2.6.egg/foolscap/_version.py

We want to run run_trial.py from all three contexts.

Another source of confusion is that distutils creates a subdirectory named build, and buildbot creates a subdirectory named build, but (unless I'm already confused) these are different -- buildbot checks out the source into a subdirectory of the buildbot named build, and then runs python setup.py build in there, which creates a subdirectory of that directory which is also named build.

I ran the build steps locally and this shows me more clearly the problem. The filesystem layout for the `test-from-egg` step is: ``` HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190/egginstalldir$ find . $ -name _version.py -o -name _auto_deps.py $ ./allmydata_tahoe-1.8.0_r4776-py2.6.egg/allmydata/_auto_deps.py ./allmydata_tahoe-1.8.0_r4776-py2.6.egg/allmydata/_version.py ./foolscap-0.5.1-py2.6.egg/foolscap/_version.py ``` For the `test-from-prefixdir` step it is: ``` HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190/prefixinstalldir$ find . $ -name _version.py -o -name _auto_deps.py $ ./lib/python2.6/site-packages/allmydata/_auto_deps.py ./lib/python2.6/site-packages/allmydata/_version.py ``` and finally for the nascent `test-with-fake-pkg.py` it would be: ``` HACK Zooko-Ofsimplegeos-MacBook-Pro:~/playground/tahoe-lafs/ticket1190$ find . $ -name _version.py -o -name _auto_deps.py $ ./build/lib/allmydata/_auto_deps.py ./build/lib/allmydata/_version.py ./darcsver-1.6.3.egg/darcsver/_version.py ./setuptools_darcs-1.2.11-py2.6.egg/setuptools_darcs/_version.py ./setuptools_trial-0.5.9.egg/setuptools_trial/_version.py ./src/allmydata/_auto_deps.py ./src/allmydata/_version.py ./support/lib/python2.6/site-packages/darcsver-1.6.3.egg/darcsver/_version.py ./support/lib/python2.6/site-packages/foolscap-0.5.1-py2.6.egg/foolscap/_version.py ``` We want to run `run_trial.py` from all three contexts. Another source of confusion is that distutils creates a subdirectory named `build`, and buildbot creates a subdirectory named `build`, but (unless I'm already confused) these are different -- buildbot checks out the source into a subdirectory of the buildbot named `build`, and then runs `python setup.py build` in there, which creates a subdirectory of *that* directory which is also named `build`.

daira commented

2010-10-28 18:39:00 +00:00

Author

Replying to zooko:

Another source of confusion is that distutils creates a subdirectory named build, and buildbot creates a subdirectory named build, but (unless I'm already confused) these are different -- buildbot checks out the source into a subdirectory of the buildbot named build, and then runs python setup.py build in there, which creates a subdirectory of that directory which is also named build.

Yes, that was the main source of my confusion last night. run_trial.py needs to be run from the directory that contains the source that is to be tested (which will have allmydata as a direct subdirectory). So [4772/ticket1190] and [4774/ticket1190] are wrong.

[4775/ticket1190] is correct when running from the src directory of a tarball or darcs checkout, but to handle the test-from-prefixdir and test-from-egg cases, we need to tolerate the zetuptoolz egg not existing. (This will work provided that the system-installed setuptools on a builder is zetuptoolz. If it isn't, we'll still have setuptools- or distribute-specific bugs, but we'll be no worse off than we are now.)

Replying to [zooko](/tahoe-lafs/trac/issues/1190#issuecomment-380898): > Another source of confusion is that distutils creates a subdirectory named `build`, and buildbot creates a subdirectory named `build`, but (unless I'm already confused) these are different -- buildbot checks out the source into a subdirectory of the buildbot named `build`, and then runs `python setup.py build` in there, which creates a subdirectory of *that* directory which is also named `build`. Yes, that was the main source of my confusion last night. `run_trial.py` needs to be run from the directory that contains the source that is to be tested (which will have `allmydata` as a direct subdirectory). So [4772/ticket1190] and [4774/ticket1190] are wrong. [4775/ticket1190] is correct when running from the `src` directory of a tarball or darcs checkout, but to handle the test-from-prefixdir and test-from-egg cases, we need to tolerate the zetuptoolz egg not existing. (This will work provided that the system-installed setuptools on a builder is zetuptoolz. If it isn't, we'll still have setuptools- or distribute-specific bugs, but we'll be no worse off than we are now.)

zooko was unassigned by daira

2010-10-28 18:39:00 +00:00

daira self-assigned this 2010-10-28 18:39:00 +00:00

daira commented

2010-10-28 18:47:31 +00:00

Author

In [4777/ticket1190]:

misc/build_helpers/run_trial.py: fix directory layout assumptions. Use a zetuptoolz egg if it exists but don't require one. refs #1190

In [4777/ticket1190]: ``` misc/build_helpers/run_trial.py: fix directory layout assumptions. Use a zetuptoolz egg if it exists but don't require one. refs #1190 ```

daira commented

2010-10-29 01:58:32 +00:00

Author

[4778/ticket1190] gets the paths to _version.py and _auto_deps.py right this time :-)

[4779/ticket1190] adds 'mock' as a dependency in the *requires* list when using run_trial.py. If there are other test dependencies in future, they can be specified in a test_requires variable of _auto_deps.py; this defaults to ['mock'].

Several of the buildslaves are still failing on the test-from-egg and test-from-prefixdir steps because they don't have the 'mock' package installed. This includes Ruben Fedora, so I can't check that the example in the description is fixed (also Shawn ubuntu-amd64 is offline). The ones that do have this package installed are working -- e.g. Freestorm-WinXP has gone green on both these steps when it was red before.

I've changed the buildmaster config (tahoe/bbgeneral.py) so that a failure or warning on the test-from-egg and test-from-prefixdir steps will make the whole build orange. This might need a buildmaster restart to take effect.

[4778/ticket1190] gets the paths to `_version.py` and `_auto_deps.py` right this time :-) [4779/ticket1190] adds 'mock' as a dependency in the `*requires*` list when using `run_trial.py`. If there are other test dependencies in future, they can be specified in a `test_requires` variable of `_auto_deps.py`; this defaults to `['mock']`. Several of the buildslaves are still failing on the test-from-egg and test-from-prefixdir steps because they don't have the 'mock' package installed. This includes [Ruben Fedora](http://tahoe-lafs.org/buildbot/builders/Ruben%20Fedora/builds/650/steps/test-from-egg/logs/stdio), so I can't check that the example in the description is fixed (also Shawn ubuntu-amd64 is offline). The ones that do have this package installed are working -- e.g. Freestorm-WinXP has gone green on both these steps when it was red before. I've changed the buildmaster config (tahoe/bbgeneral.py) so that a failure or warning on the test-from-egg and test-from-prefixdir steps will make the whole build orange. This might need a buildmaster restart to take effect.

daira removed their assignment 2010-10-29 01:58:32 +00:00

zooko was assigned by daira

2010-10-29 01:58:32 +00:00

daira commented

2010-10-29 03:16:02 +00:00

Author

In [4780/ticket1190]:

add misc/build_helpers/test-with-fake-pkg.py. refs #1190

In [4780/ticket1190]: ``` add misc/build_helpers/test-with-fake-pkg.py. refs #1190 ```

daira commented

2010-10-29 03:19:09 +00:00

Author

In changeset:390c40cd8ce1e579:

add misc/build_helpers/test-with-fake-pkg.py. refs #1190

In changeset:390c40cd8ce1e579: ``` add misc/build_helpers/test-with-fake-pkg.py. refs #1190 ```

daira commented

2010-10-29 03:37:08 +00:00

Author

Attachment buildmaster-changes.darcs.patch (81729 bytes) added

buildmaster: * tahoe/bbgeneral.py: mark the overall build as having warnings if the test-from-egg or test-from-prefixdir steps fail or warn. * tahoe/bbgeneral.py, bbsupport.py: add test-with-fake-pkg step

**Attachment** buildmaster-changes.darcs.patch (81729 bytes) added buildmaster: * tahoe/bbgeneral.py: mark the overall build as having warnings if the test-from-egg or test-from-prefixdir steps fail or warn. * tahoe/bbgeneral.py, bbsupport.py: add test-with-fake-pkg step

buildmaster-changes.darcs.patch

80 KiB

daira commented

2010-10-29 03:40:12 +00:00

Author

Replying to davidsarah:

I've changed the buildmaster config (tahoe/bbgeneral.py) so that a failure or warning on the test-from-egg and test-from-prefixdir steps will make the whole build orange. This might need a buildmaster restart to take effect.

Yes, it does. Also to add the test-with-fake-pkg step.

Replying to [davidsarah](/tahoe-lafs/trac/issues/1190#issuecomment-380903): > I've changed the buildmaster config (tahoe/bbgeneral.py) so that a failure or warning on the test-from-egg and test-from-prefixdir steps will make the whole build orange. This might need a buildmaster restart to take effect. Yes, it does. Also to add the test-with-fake-pkg step.

daira commented

2010-10-29 05:28:24 +00:00

Author

In [4781/ticket1190]:

misc/build_helpers/run_trial.py: correct off-by-one directory level when running from a src directory. refs #1190

In [4781/ticket1190]: ``` misc/build_helpers/run_trial.py: correct off-by-one directory level when running from a src directory. refs #1190 ```

daira commented

2010-10-29 06:34:36 +00:00

Author

In [4783/ticket1190]:

startstop_node.py: pyflakes import fix. refs #1190

In [4783/ticket1190]: ``` startstop_node.py: pyflakes import fix. refs #1190 ```

zooko commented

2010-10-29 06:58:54 +00:00

Okay! There is now a buildstep named test-with-fake-pkg which tests this behavior, and all buildslaves which are able to run that buildstep (it requires mock) show that the current code in [the ticket1190 branch]source:ticket1190 passes. The next step is to ask Brian if he will not object to merging this to trunk, and then possibly rebasing it for trunk, or else just applying all 18 patches from ticket1190 to trunk.

Here are the buildbot results for ticket1190: http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1190

Brian: do you agree to merge this branch onto trunk?

Okay! There is now a buildstep named `test-with-fake-pkg` which tests this behavior, and all buildslaves which are able to run that buildstep (it requires `mock`) show that the current code in [the ticket1190 branch]source:ticket1190 passes. The next step is to ask Brian if he will not object to merging this to trunk, and then possibly rebasing it for trunk, or else just applying all 18 patches from ticket1190 to trunk. Here are the buildbot results for ticket1190: <http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1190> Brian: do you agree to merge this branch onto trunk?

zooko removed their assignment 2010-10-29 06:58:54 +00:00

warner was assigned by zooko

2010-10-29 06:58:54 +00:00

zooko commented

2010-10-29 07:02:06 +00:00

Oh, and this will need an entry in source:NEWS.

daira commented

2010-10-29 18:03:20 +00:00

Author

I will rebase these patches (no need to leave evidence of my mistakes in the history ;-)

warner was unassigned by daira

2010-10-29 18:03:20 +00:00

daira self-assigned this 2010-10-29 18:03:20 +00:00

warner commented

2010-10-29 21:38:30 +00:00

I'll look at the patch once it's rebased and I can think about just one change instead of 18.

daira commented

2010-10-29 22:56:48 +00:00

Author

In changeset:fd02946074821be9:

setup.py, misc/build_helpers/run_trial.py: use undocumented __requires__ variable to cause setuptools/zetuptoolz to put the correct versions of dependencies on sys.path. Also ensure that run_trial adds the bundled zetuptoolz egg at the start of sys.path if present. Make the source directory comparison work correctly for the test-with-fake-pkg build step. refs #1190

In changeset:fd02946074821be9: ``` setup.py, misc/build_helpers/run_trial.py: use undocumented __requires__ variable to cause setuptools/zetuptoolz to put the correct versions of dependencies on sys.path. Also ensure that run_trial adds the bundled zetuptoolz egg at the start of sys.path if present. Make the source directory comparison work correctly for the test-with-fake-pkg build step. refs #1190 ```

daira commented

2010-10-29 22:56:49 +00:00

Author

In changeset:647aa74d687157a9:

bundled zetuptoolz: if __main__.__requires__ exists then do not add packages to the working set if they provide an incompatible version of a package. Also put a complete __requires__ listing the transitive closure of dependencies at the beginning of generated scripts, rather than a shallow __requires__ specifying only the application version. refs #1190

In changeset:647aa74d687157a9: ``` bundled zetuptoolz: if __main__.__requires__ exists then do not add packages to the working set if they provide an incompatible version of a package. Also put a complete __requires__ listing the transitive closure of dependencies at the beginning of generated scripts, rather than a shallow __requires__ specifying only the application version. refs #1190 ```

daira commented

2010-10-29 23:27:05 +00:00

Author

In changeset:8835f009d0e6907a:

misc/build_helpers/run_trial.py: look for zetuptoolz egg in the parent directory, not the cwd of run_trial. refs #1190

In changeset:8835f009d0e6907a: ``` misc/build_helpers/run_trial.py: look for zetuptoolz egg in the parent directory, not the cwd of run_trial. refs #1190 ```

daira commented

2010-10-30 00:35:49 +00:00

Author

In changeset:2a8f700026a207d9:

scripts/runner.py: remove pkg_resources.require() calls. These are at best redundant because we have already called _auto_deps.require_auto_deps() (from allmydata.__init__) at that point, and they are causing failure of the test-from-prefixdir step on some buildslaves. refs #1190

In changeset:2a8f700026a207d9: ``` scripts/runner.py: remove pkg_resources.require() calls. These are at best redundant because we have already called _auto_deps.require_auto_deps() (from allmydata.__init__) at that point, and they are causing failure of the test-from-prefixdir step on some buildslaves. refs #1190 ```

daira commented

2010-10-30 00:55:49 +00:00

Author

In changeset:1950d5a719b00175:

scripts/runner.py: fix unused import of allmydata. refs #1190

In changeset:1950d5a719b00175: ``` scripts/runner.py: fix unused import of allmydata. refs #1190 ```

daira commented

2010-10-30 04:07:44 +00:00

Author

In changeset:7cec440a14745159:

misc/build_helpers/test-with-fake-pkg.py: look for eggs in the parent of the src directory. refs #1190

In changeset:7cec440a14745159: ``` misc/build_helpers/test-with-fake-pkg.py: look for eggs in the parent of the src directory. refs #1190 ```

daira commented

2010-10-30 05:30:14 +00:00

Author

We no longer have any VersionConflict errors on any of the builders. We do have DistributionNotFound errors (e.g. here for twisted) and ImportErrors (e.g. here for nevow and here for mock), but I think those are different problems.

We no longer have any `VersionConflict` errors on any of the builders. We do have `DistributionNotFound` errors (e.g. [here](http://tahoe-lafs.org/buildbot/builders/Ruben%20Fedora/builds/664/steps/test-from-prefixdir/logs/stdio) for twisted) and `ImportError`s (e.g. [here](http://tahoe-lafs.org/buildbot/builders/Eugen%20lenny-amd64/builds/645/steps/test-from-prefixdir/logs/stdio) for nevow and [here](http://tahoe-lafs.org/buildbot/builders/David%20A.%20OpenSolaris%20i386/builds/439/steps/test-from-egg/logs/stdio) for mock), but I think those are different problems.

zooko commented

2010-10-30 08:42:16 +00:00

Replying to davidsarah:

We do have DistributionNotFound errors (e.g. here for twisted)

I think this one might be a bug in Fedora: Fedora issue #523210.

You can see from this step that pkg_resources.require('Twisted') yields nothing even though import twisted works.

and ImportErrors (e.g. here for nevow

This is expected because test-from-prefixdir is explicitly not using the setuptools automatic resolution of dependencies. It disables that feature by passing --single-version-externally-managed. Since Eugen lenny-amd64 doesn't have Nevow installed into the system, it is expected to fail. Oh wait! But instead of failing by ImportError, it ought to fail by DistributionNotFound. It fails by ImportError because import nevow is present in [allmydata/init.py]source:trunk/src/allmydata/init.py?annotate=blame&rev=4687 before _auto_deps.require_auto_deps() is called.

and here for mock),

We need to apply a hack similar to changeset:7cec440a14745159 to the test-from-egg step in the buildmaster configuration to fix this... There! I've updated the buildmaster config to add eggs to the sys.path from the source base dir (the dir that has setup.py instead of the CWD and that fixed it: test-from-egg step.

Replying to [davidsarah](/tahoe-lafs/trac/issues/1190#issuecomment-380925): > We do have `DistributionNotFound` errors (e.g. [here](http://tahoe-lafs.org/buildbot/builders/Ruben%20Fedora/builds/664/steps/test-from-prefixdir/logs/stdio) for twisted) I think this one might be a bug in Fedora: [Fedora issue #523210](https://bugzilla.redhat.com/show_bug.cgi?id=523210). You can see from [this step](http://tahoe-lafs.org/buildbot/builders/Ruben%20Fedora/builds/665/steps/show-tool-versions/logs/stdio) that `pkg_resources.require('Twisted')` yields nothing even though `import twisted` works. > and `ImportError`s (e.g. [here](http://tahoe-lafs.org/buildbot/builders/Eugen%20lenny-amd64/builds/645/steps/test-from-prefixdir/logs/stdio) for nevow This is expected because `test-from-prefixdir` is explicitly not using the setuptools automatic resolution of dependencies. It disables that feature by passing `--single-version-externally-managed`. Since Eugen lenny-amd64 doesn't have Nevow installed into the system, it is expected to fail. Oh wait! But instead of failing by ImportError, it ought to fail by DistributionNotFound. It fails by ImportError because `import nevow` is present in [allmydata/*init*.py]source:trunk/src/allmydata/*init*.py?annotate=blame&rev=4687 before `_auto_deps.require_auto_deps()` is called. > and [here](http://tahoe-lafs.org/buildbot/builders/David%20A.%20OpenSolaris%20i386/builds/439/steps/test-from-egg/logs/stdio) for mock), We need to apply a hack similar to changeset:7cec440a14745159 to the `test-from-egg` step in the buildmaster configuration to fix this... There! I've updated the buildmaster config to add eggs to the `sys.path` from the source base dir (the dir that has `setup.py` instead of the CWD and that fixed it: [test-from-egg step](http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/180/steps/test-from-egg/logs/stdio).

zooko commented

2010-10-31 05:48:43 +00:00

David-Sarah has written a NEWS entry. Once they commit it, this ticket can be closed.

daira commented

2010-10-31 06:14:06 +00:00

Author

In changeset:cb764da0edc2b161:

NEWS: entries for #1190 and #1212, and minor cleanups. refs #1190, #1212

In changeset:cb764da0edc2b161: ``` NEWS: entries for #1190 and #1212, and minor cleanups. refs #1190, #1212 ```

daira added the

r/fixed

label 2010-10-31 06:53:48 +00:00

daira closed this issue

2010-10-31 06:53:48 +00:00

zooko commented

2010-11-18 08:11:48 +00:00

I'm very sleepy so I might be misunderstanding, but I think this has regressed in trunk:

(@@http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1233#comment:380907@@)

I'm very sleepy so I might be misunderstanding, but I think this has regressed in trunk: (@@http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1233#[comment:380907](/tahoe-lafs/trac/issues/1190#issuecomment-380907)@@)

zooko removed the

r/fixed

label 2010-11-18 08:11:48 +00:00

zooko reopened this issue

2010-11-18 08:11:48 +00:00

zooko commented

2010-11-18 14:58:03 +00:00

My problem that I reported in comment:19:ticket:1233 with test-with-fake-pkg not working on my mac was just due to the fact that you have to run python setup.py test before test-with-fake-pkg will work.

My problem that I reported in comment:19:ticket:1233 with test-with-fake-pkg not working on my mac was just due to the fact that you have to run `python setup.py test` before test-with-fake-pkg will work.

zooko commented

2010-11-18 15:28:20 +00:00

http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/234/steps/test-with-fake-dists/logs/stdio

aha! Same error!

pkg_resources.VersionConflict: (foolscap 0.4.2 (/Library/Python/2.6/site-packages/foolscap-0.4.2-py2.6.egg), Requirement.parse('foolscap[secure_connections]>=0.5.1'))

So it has something to do with my having downgraded just from now distribute 0.6.14 to setuptools 0.6c9, I guess.

same thing on trunk:

http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/235/steps/test-with-fake-dists/logs/stdio

I'm upgrading from setuptools-0.6c9 back to distribute-0.6.14 and trying again just to confirm.

I'm inclined to somehow mark this as a "wont-fix" if it is only a problem with that older version of setuptools.

On the other hand, I don't know how the system-installed version of setuptools manages to get into the act at all--we force our bundled zetuptoolz to be imported first thing in our setup.py. Is this VersionConflict arising from some other process than the one that loads our setup.py??

So, while I am really not motivated to develop a work-around for a bug in setuptools-0.6c9 (very old and now little-used), I am motivated to understand better what is going on here...

Help?

In the attempt to reproduce this, I installed setuptools-0.6c9 and foolscap 0.4.2, to emulate what is currently installed on Brian's linode builder. Here is the result on my laptop zomp of building source:ticket1233: <http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/234/steps/test-with-fake-dists/logs/stdio> aha! Same error! ``` pkg_resources.VersionConflict: (foolscap 0.4.2 (/Library/Python/2.6/site-packages/foolscap-0.4.2-py2.6.egg), Requirement.parse('foolscap[secure_connections]>=0.5.1')) ``` So it has something to do with my having downgraded just from now distribute 0.6.14 to setuptools 0.6c9, I guess. same thing on trunk: <http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/235/steps/test-with-fake-dists/logs/stdio> I'm upgrading from setuptools-0.6c9 back to distribute-0.6.14 and trying again just to confirm. I'm inclined to somehow mark this as a "wont-fix" if it is only a problem with that older version of setuptools. On the other hand, I don't know how the system-installed version of setuptools manages to get into the act at all--we force our bundled zetuptoolz to be imported first thing in our setup.py. Is this `VersionConflict` arising from some other process than the one that loads our setup.py?? So, while I am really not motivated to develop a work-around for a bug in setuptools-0.6c9 (very old and now little-used), I *am* motivated to understand better what is going on here... Help?

zooko commented

2010-11-18 16:26:04 +00:00

Oh look I upgraded to distribute-0.6.14 again and it still has the same error:

http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/236/steps/test-with-fake-dists/logs/stdio

So if you install foolscap-0.4.2 this is sufficient to trigger the bug.

Oh look I upgraded to distribute-0.6.14 again and it still has the same error: <http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/236/steps/test-with-fake-dists/logs/stdio> So if you install foolscap-0.4.2 this is sufficient to trigger the bug.

daira commented

2010-11-18 21:09:54 +00:00

Author

Note that although 'setup.py test' is supposed to be equivalent to 'setup.py build' followed by 'setup.py trial', it's not quite equivalent because the sys.path will be different. ('build' has built things that can be put onto the path for 'trial', whereas a 'test' without a preceding ~~'trial'~~ 'build' has to use whatever was previously installed.)

In the 'test' step (and subsequent testing steps), we have previously done a build. In the 'test-with-fake-dists' step, we have not. I believe that's the difference that is causing this regression for 'test-with-fake-dists'. Note that although 'setup.py test' is supposed to be equivalent to 'setup.py build' followed by 'setup.py trial', it's not quite equivalent because the sys.path will be different. ('build' has built things that can be put onto the path for 'trial', whereas a 'test' without a preceding ~~'trial'~~ 'build' has to use whatever was previously installed.)

zooko commented

2010-11-19 07:32:47 +00:00

Okay I finally isolated the difference between getting this VersionConflict and not getting it on my laptop (zomp). I determined that if I rm src/allmydata/_version.py then run python misc/build_helpers/test-with-fake-dists.py I get the VersionConflict but if I don't rm src/allmydata/_version.py then I don't get the VersionConflict.

No doubt this has something to do with [this code]source:trunk/setup.py@4822#L66 in our setup.py. :-)

As David-Sarah pointed out in comment:380937, we're seeing this VersionConflict on some buildslaves now because I moved the invocation of test-with-fake-dists.py (by the buildmaster) to happen before any build step. (This is necessary because the build step provides a good version of pycryptopp so once it has run then the test-with-fake-dists.py step can't determine how well our setup scripts handle the lack of a good version of pycryptopp...)

Okay I finally isolated the difference between getting this `VersionConflict` and not getting it on my laptop (zomp). I determined that if I `rm src/allmydata/_version.py` then run `python misc/build_helpers/test-with-fake-dists.py` I get the `VersionConflict` but if I don't `rm src/allmydata/_version.py` then I don't get the `VersionConflict`. No doubt this has something to do with [this code]source:trunk/setup.py@4822#L66 in our setup.py. :-) As David-Sarah pointed out in [comment:380937](/tahoe-lafs/trac/issues/1190#issuecomment-380937), we're seeing this `VersionConflict` on some buildslaves now because I moved the invocation of `test-with-fake-dists.py` (by the buildmaster) to happen before any `build` step. (This is necessary because the build step provides a good version of pycryptopp so once it has run then the `test-with-fake-dists.py` step can't determine how well our setup scripts handle the lack of a good version of pycryptopp...)

zooko commented

2010-11-19 08:17:23 +00:00

Okay I've attempted to address this issue in our testing with changeset:50f8c37a2b0049a5 (on source:ticket1233). Please review!

I triggered a build of source:ticket1233:

http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1233&first_time=1290152000&last_time=1290162000

Okay I've attempted to address this issue in our testing with changeset:50f8c37a2b0049a5 (on source:ticket1233). Please review! I triggered a build of source:ticket1233: <http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1233&first_time=1290152000&last_time=1290162000>

zooko commented

2010-11-19 14:21:24 +00:00

Okay, the buildbot is nicely green now with &branch=ticket1233:

http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1233

Okay, the buildbot is nicely green now with `&branch=ticket1233`: <http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1233>

daira commented

2010-11-19 19:56:04 +00:00

Author

changeset:b4c14421f7c2f25a is good as far as it goes. I still have reservations about the test-with-fake-dists step; the fact that it tests a package that really exists means that it can be fooled by an installed pycryptopp, and not test what it is supposed to be testing. But maybe it's sufficient for 1.8.1.

zooko commented

2010-11-19 22:06:57 +00:00

I guess really we should have a unit test of zetuptoolz which constructs a fake package with a dependency to test this behavior of zetuptoolz.

As far as a test of Tahoe-LAFS--I'm okay with this one. :-)

So I'm going to interpret your comment:380941 as a +0 and remove the review-needed flag and add reviewed.

Currently it doesn't give a false positive unless pycryptopp-0.5.14 is installed (a very rare old version of pycryptopp). It can give a Skip, due to a recent version of pycryptopp being installed or a true FAIL. I agree it isn't perfect, but I haven't been able to figure out how to do better. I guess really we should have a unit test of zetuptoolz which constructs a fake package with a dependency to test this behavior of zetuptoolz. As far as a test of Tahoe-LAFS--I'm okay with this one. :-) So I'm going to interpret your [comment:380941](/tahoe-lafs/trac/issues/1190#issuecomment-380941) as a +0 and remove the `review-needed` flag and add `reviewed`.

zooko commented

2010-11-19 22:25:13 +00:00

Committed to trunk as changeset:50f8c37a2b0049a5.