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
Labels
No Label
0.2.0
0.3.0
0.4.0
0.5.0
0.5.1
0.6.0
0.6.1
0.7.0
0.8.0
0.9.0
1.0.0
1.1.0
1.10.0
1.10.1
1.10.2
1.10a2
1.11.0
1.12.0
1.12.1
1.13.0
1.14.0
1.15.0
1.15.1
1.2.0
1.3.0
1.4.1
1.5.0
1.6.0
1.6.1
1.7.0
1.7.1
1.7β
1.8.0
1.8.1
1.8.2
1.8.3
1.8β
1.9.0
1.9.0-s3branch
1.9.0a1
1.9.0a2
1.9.0b1
1.9.1
1.9.2
1.9.2a1
LeastAuthority.com automation
blocker
cannot reproduce
cloud-branch
code
code-dirnodes
code-encoding
code-frontend
code-frontend-cli
code-frontend-ftp-sftp
code-frontend-magic-folder
code-frontend-web
code-mutable
code-network
code-nodeadmin
code-peerselection
code-storage
contrib
critical
defect
dev-infrastructure
documentation
duplicate
enhancement
fixed
invalid
major
minor
n/a
normal
operational
packaging
somebody else's problem
supercritical
task
trivial
unknown
was already fixed
website
wontfix
worksforme
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Reference: tahoe-lafs/trac-2024-07-25#1190
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
(Split from /tahoe-lafs/trac-2024-07-25/issues/6127#comment:79901.)
Log for the test step on 'Ruben Fedora':
I believe this is due to the same underlying issue as #1137, that is, the handling of
sys.path
by the setuptools-installedsite.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':
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 forsetup.py trial
.Attachment requires.patch.txt (5400 bytes) added
Attachment ignore-DNF.patch.txt (965 bytes) added
I think I have a working isolated, deterministic test of this functionality now, and I think I understand how to fix it, too.
Attachment 1190-nospawn.darcs.patch (7957 bytes) added
import+exec twistd, instead of find+spawn, to remove an intermediate process
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 to green 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 vs. for branch=trunk.
Okay in the cold light of morning I see that the current failures on [the ticket1190 branch]source:ticket1190 are of three kinds:
test_no_noise
to go red: arthur build 503allmydata-tahoe
is getting added to the working set even though it is incompatible with the requirement ofallmydata-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[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 theappname
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-2024-07-25/issues/6221#comment:-1). Node directories should contain only data, not code.I added a hefty comment to /tahoe-lafs/trac-2024-07-25/issues/6221 . 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 intahoe 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.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 inbin/tahoe
beforetahoe start
tries to import anything of interest.In changeset:270322ad4762f47f:
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 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 tosetup.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 reverted the changes to
.tac
generation on the ticket1190 branch: [4768/ticket1190].Build results: http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1190
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:
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.
Attachment test-with-fake-pkg.py (1301 bytes) added
Oh, the output in comment:79905, made using test-with-fake-pkg.py, also had this patch applied to source:misc/build_helpers/run_trial.py:
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! :-)
I ran the build steps locally and this shows me more clearly the problem. The filesystem layout for the
test-from-egg
step is:For the
test-from-prefixdir
step it is:and finally for the nascent
test-with-fake-pkg.py
it would be: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 namedbuild
, but (unless I'm already confused) these are different -- buildbot checks out the source into a subdirectory of the buildbot namedbuild
, and then runspython setup.py build
in there, which creates a subdirectory of that directory which is also namedbuild
.Replying to zooko:
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 haveallmydata
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.)In [4777/ticket1190]:
[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 usingrun_trial.py
. If there are other test dependencies in future, they can be specified in atest_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.
In [4780/ticket1190]:
In changeset:390c40cd8ce1e579:
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
Replying to davidsarah:
Yes, it does. Also to add the test-with-fake-pkg step.
In [4781/ticket1190]:
In [4783/ticket1190]:
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 requiresmock
) 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?
Oh, and this will need an entry in source:NEWS.
I will rebase these patches (no need to leave evidence of my mistakes in the history ;-)
I'll look at the patch once it's rebased and I can think about just one change instead of 18.
In changeset:fd02946074821be9:
In changeset:647aa74d687157a9:
In changeset:8835f009d0e6907a:
In changeset:2a8f700026a207d9:
In changeset:1950d5a719b00175:
In changeset:7cec440a14745159:
We no longer have any
VersionConflict
errors on any of the builders. We do haveDistributionNotFound
errors (e.g. here for twisted) andImportError
s (e.g. here for nevow and here for mock), but I think those are different problems.Replying to davidsarah:
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 thoughimport twisted
works.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 becauseimport 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.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 thesys.path
from the source base dir (the dir that hassetup.py
instead of the CWD and that fixed it: test-from-egg step.David-Sarah has written a NEWS entry. Once they commit it, this ticket can be closed.
In changeset:cb764da0edc2b161:
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:79913@@)
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.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!
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?
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.
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.)Okay I finally isolated the difference between getting this
VersionConflict
and not getting it on my laptop (zomp). I determined that if Irm src/allmydata/_version.py
then runpython misc/build_helpers/test-with-fake-dists.py
I get theVersionConflict
but if I don'trm src/allmydata/_version.py
then I don't get theVersionConflict
.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:79941, we're seeing this
VersionConflict
on some buildslaves now because I moved the invocation oftest-with-fake-dists.py
(by the buildmaster) to happen before anybuild
step. (This is necessary because the build step provides a good version of pycryptopp so once it has run then thetest-with-fake-dists.py
step can't determine how well our setup scripts handle the lack of a good version of pycryptopp...)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, the buildbot is nicely green now with
&branch=ticket1233
:http://tahoe-lafs.org/buildbot/waterfall?branch=ticket1233
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.
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:79945 as a +0 and remove the
review-needed
flag and addreviewed
.Committed to trunk as changeset:50f8c37a2b0049a5.
So I think we need to wait and see if my patches fix #1233 on trunk and if so then we can close this ticket as
fixed
as well as that one.Okay! Looks like it worked.