stop using .tac files: make it possible to change appname, Python package-directory name, perhaps other names #1159
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#1159
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
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?
(Reported to tahoe-dev by Nathan Eisenberg.)
In the 1.8beta release, when a user attempts to start a node using a node directory generated by an earlier version (or trunk), it will fail to start and will report a traceback similar to:
This happens because the appname was changed to "
tahoe-lafs-ticket798" for the beta [4595/ticket798], but existing.tacfiles contain references to the previous "allmydata-tahoe" appname. For example,tahoe-client.tactypically contains:Note that the appname change [has not been made on trunk]source:setup.py@4668#L329, only on the ticket798 branch from which the beta was built.
We should therefore withdraw the beta, because otherwise users may generate new node directories that will not work in future versions (due to the opposite problem, i.e.
DistributionNotFound: tahoe-lafs-ticket798).This failed to be detected because there are no tests that check compatibility with a node directory generated by a previous version. To prevent this happening again, we need the test suite to run at least a smoke test with a node directory that looks like one generated by an earlier version (perhaps check one in as a resource, with a big-fat-warning not to change it).
Hm, there are four overlapping namespaces here.
One is the "appname" which is defined by us (the Tahoe-LAFS project) to distinguish different variants or forks of Tahoe-LAFS or independent implementations of the LAFS protocol. We agreed on how to do this versioning in this message to tahoe-dev. The appname is used in [in allmydata/init.py]source:src/allmydata/init.py@4597#L61 to construct a variable named
*full_version*, which is used in a few places such as [storage/server.py]source:src/allmydata/storage/server.py@4595#L270. The appname for Tahoe-LAFS has been "allmydata-tahoe" for the last couple of years, and we would like to change the appname to "tahoe-lafs" someday.Another namespace is the Python "distribution" namespace. (A "distribution" is what the Python world calls a package.) This is a global namespace where everyone on the planet is supposed to avoid choosing colliding names. An important central locus of this namespace is the Python Package Index (which I suppose ought to be called the "Python Distribution Index")—installers such as
easy_installandpip, when asked to install a distribution named$DIST, will by default look for it at<http://pypi.python.org/pypi/$DIST>. The distribution name for Tahoe-LAFS has been "allmydata-tahoe" and we would like to change it to "tahoe-lafs".The third namespace is the Python "package" namespace. (A "package" is what the Python world calls a directory containing an
*init*.pyfile.) This namespace is global and is populated by all of the Python distributions that you install on your system. Collisions are silently resolved by a complicated algorithm involving yourPYTHONPATHand a certain widely disliked hack due originally to setuptools (setuptool #53). Thetahoe-client.tacfile loads a package namedallmydata. We would probably like to change that in the future totahoe.Now currently [our build scripts]source:trunk/setup.py@4616#L329 ensure that the Python distribution name is always equal to the appname, and the [create-node command]source:src/allmydata/scripts/create_node.py@4641#L31 adds
pkg_resources.require(APPNAME)to thetahoe-client.tacfile.The fourth namespace is the scripts and executables in your system, where we currently create a file named
tahoe.One thing that we can do right away to ease this is change create-node to stop putting the call to
pkg_resources.require()in the tahoe-client.tac.Replying to zooko:
As far as I understood, these were intended to be the same by definition, at least for Python implementations of the protocol. I.e. the "
application-version" field in introduction messages is specifying the implementation version (not a protocol version), and the distribution string is also specifying the implementation version, and so we put the latter string intoapplication-version. (Clearly there can be non-Python implementations of the protocol, and they would use whatever is the nearest equivalent of the distribution string.)Is there any reason for these not to be the same by definition, for Python implementations?
No, absolutely not! That's the module namespace. A package is something else entirely; see below.
(Okay, much of the Python world may be inconsistent in the terminology they use, but the official docs are consistent on this point.)
More precisely (and it's important to be precise here), a distribution is named by
appname+'-'+version. The things named by anappnamealone are called "packages" (hence "Python Package Index"). A distribution contains a particular version of a package.(Packages are also called "projects" in some of the setuptools documentation, but no-one uses that, not even the rest of the setuptools docs and code.)
As far as I understand, the ostensible purpose of that call is to ensure that when we import modules named
allmydata.*, we're getting them from some version of theallmydata-tahoepackage (this doesn't help to ensure that we're getting the right version). On the other hand, I don't think it actually succeeds in doing even that, if another package defines anallmydatamodule and happens to be earlier onsys.path, which is currently possible. However I think it is also necessary in order for the call toload_entry_pointin the CLI wrapper script to work correctly.(The implementation of
pkg_resources.requirein zetuptoolz is here. Good luck tracing that :-)I seem to recall that the
pkg_resources.requireAPI is intended to let programs express a dependency on specific versions (or version ranges) of other package/modules, in the actual code that does theimport(as opposed to off in some remotesetup.pyworld). Using it without a version number specification seems to be not particularly useful, although I think it probably does serve to tell setuptools to fuss around with PYTHONPATH enough to allow theimportto maybe work.I think I'd like to see a list of all the things in our tree that depend upon PYTHONPATH being set in some particular way (probably bin/tahoe and test_runner), and then see if there's a way to accomplish this by either removing those places or purely by manipulating PYTHONPATH, depending less upon setuptools mechanisms. In particular, I think that
twistdhas been loadable as a library for several years now, so I think that the subprocess-spawning done bytahoe start/tahoe runcan go away, which might remove one of the needs to change PYTHONPATH. (the test_runner code that really does want to spawn a subprocess, because it wants to kill the subprocess, may be replaceable with anos.forkor something, so the child can simply inherit the environment of the parent).This bug is not present in the 1.8.0c1 release candidate, but leaving open because we need a test.
Replying to [davidsarah]comment:2:
Agreed.
I can't think of any at the moment.
What docs are you referring to? I really wish that there was some official justification to use "package" to mean a package, but sadly I was unable to persuade the distutils-sig mailing list to get behind a full-scale renaming. Therefore we are currently stuck with this:
http://guide.python-distribute.org/glossary.html
Distribution
That might be our only hope -- the word "project" isn't already used to mean something else within Python, so maybe we could start consistently using it to mean all-versions-of the source that is used to produce distributions (each of which has a version number).
By the way I think that Brian in comment:79167 is showing that he is confused about the difference between distributions (sometimes called packages, especially by the rest of the world outside of Python), packages (directories that have
*init*.pyfiles in them), and modules (importable bundles of Python code). Thepkg_resourcesAPI is intended to express a dependency on a project, i.e. a range (possibly "any") of versions of distributions of that project. (Note the name begins with "pkg", which stands for "distribution". Ha ha.)David-Sarah wrote in comment:2:
My purpose in adding that call to
pkg_resourceswas to ensure that if the distribution ("package"? "project"?) was not installed that we would get an explicit error at that point instead of proceding. Actually, it was more to ensure that the transitive closure of theinstall_requiresof theallmydata-tahoedistribution were installed. It has been a long time since I've felt like I needed that double-check and I don't object to removing it.Replying to [zooko]comment:5:
I meant that the python.org docs are consistent in not using "package" when they mean "module" (at least I haven't seen any counterexamples on python.org).
OK, that's not too bad.
Gahh. This is completely wrong and unnecessarily confusing. Pretend they didn't say that.
The right term for the latter concept is "module directory".
(Module code can also be stored in zipfiles, so this isn't necessarily a directory in a filesystem.)
[...]
I've no objection to using "project", if you think "package" is ambiguous.
The strings specifying a package/project possibly with a version constraint, e.g. "allmydata-tahoe>=1.7.0", are called "requirements".
The argument to
pkg_resources.requireis a requirement. In this case it is just a requirement for an unspecified version of theallmydata-tahoeproject, which is not all that useful, I think.I don't think it actually checks that.
Let's try that (for after 1.8.0) and see if it breaks anything.
My vague recollection is that it does, because I think I remember having
a Foolscap installed in such a way that 'import foolscap' worked fine but
setuptools/pkg_resources didn't believe it was present (or wasn't of the
required version), and that commenting out the
pkg_resources.requirecall was necessary to make it stop throwing
NoSuchDistributionexceptions or the like.
I wasn't too thorough about checking that, so I could be wrong.
Replying to warner:
Yeah, it does:
[by davidsarah: this test is misleading. it's not the call to
pkg_resources.require('allmydata-tahoe')in the tac file that performs this check; the check is done as a side-effect of "import pkg_resources" after setting*requires*in the setuptools-generated support script. All explicit calls topkg_resources.requirefor package names that were already in*requires*are redundant.]editWhat are the next steps on this ticket? I think it is to remove the
pkg_resources.require()in intahoe-client.tac, and possibly to make a facility to overwritetahoe-client.tacwith a new version on install so that when people install Tahoe-LAFS v1.9.0 they get this new version.1.8beta is incompatible with existing node directories due to change of appnameto make it possible to change appname, Python package-directory name, perhaps other namesReplying to zooko:
How would we know where all of the
.tacfiles are on install? I think that to be sufficiently compatible, we'd have to work correctly even when the.tacfile used when starting a node is one that we've never seen before.Someone please explain what the benefit is of using
.tacfiles at all? Is that just "the way it's done" in Twisted?Re .tac files:
It's probably time for them to die. When I wrote the very first
get-a-client-running framework (30-Nov-2006, wow), I copied the scheme I'd
used in buildbot, since it was an easy way to get a daemon process running.
The original idea of .tac files (and .tap and .tax files, their black-sheep
siblings) was to decouple the implementation of a service from the
deployment. From a developer's point of view, it's pretty easy to create a
service in the Twisted world: you make a subclass of
twisted.application.service.Service, overridestartService/stopServiceto control startup/shutdown, and connectother services in a tree shape with
setServiceParent.To actually start this, the cheap "
__main__" way is to useservice.startService(); reactor.run(), and the cheap way to configureit is to pass arguments into your service's constructor. But ops folks (or,
you know, users) who want to run your service don't want to write python
code. And they might want to configure the service without writing python
code.
The "official" way to do this, at least as of 2006, was a Twisted tool named
'
mktap'. This uses plugins (which you write just after you write yourService subclass) to transform command-line arguments (a
twisted.python.usage.Usage instance) into a fully-configured Service
instance, and then serializes (with
pickle) the Service out into a .tapfile. Then, later, you use '
twistd foo.tap' to thaw out the Service andstart it running (properly daemonized and setuided and all).
twistdwould also serialize the Service object when you quit the process, so you
could achieve persistence by running '
twistd foo-shutdown.tap' onsubsequent runs.
Eventually
twistdlearned how to use themktapplugins directly,so I think you can run
twistd web --path .to do the same thing asmktap web --path . ; twistd web.tapbut without the leftover .tap file.But, pickle files are a drag, and the auto-persistence -shutdown.tap feature
turned out to be a bad idea. Once you stop using pickle, then having a
non-human-readable configuration/deployment file is a drag. So .tac (which is
a python program that provides a distinguished 'application' symbol, and is
evaluated by
twistd) becomes reasonable.Anyways, the benefit of writing out a .tac file is that you could use the
generic
twistdto launch the service (assuming that everything is onyour PYTHONPATH), which gives you a bunch of maybe-useful options for free:
--syslog, --profile, --rundir, --chroot, --uid, --reactor. There arealso some tools which make it easy to launch twistd-style jobs from
/etc/init.d/initscripts, or from macLaunchAgentplists.But, now that twistd is invokable as a library, it's easier to get those free
arguments without needing to duplicate them all in the '
tahoe start'option parser. If we commit to having
bin/tahoebe the only way tolaunch tahoe, then it can set up
sys.pathbefore importingtwisted.scripts.twistd.So maybe what's left is to define a data file (perhaps named
client.tac, for backwards compatibility), from whichtahoe startcan determine that it ought to instantiate an
allmydata.client.Clientinstead of an
allmydata.introducer.IntroducerNode. And then changetahoe startto do whateverpkg_resources.requiremight reallyreally be necessary before scanning the data file and doing the conditional
import.
Or maybe
tahoe startcould look for "tahoe.cfg" to decide if the targetdirectory is really a tahoe nodedir, look either for a key in it or the
presence of
client.tac/introducer.tacto decide what top-levelService to instantiate, but then otherwise ignore the .tac file. And new
versions of
tahoe create-nodecould write thenodetype=key intotahoe.cfg and not write the .tac file.
Oh, one caveat: since we delegate daemonization/etc to
twistd, we're limited in the set of entry points we can use. The easiest one istwisted.scripts.twistd.run, for which the arguments are effectivelysys.argv, which means it really wants to find a.tacfile. What we want is a way to build thetwisted.application.service.Applicationor.MultiServiceinstance and hand it fully-formed into twistd. So we'd need to find a stable/reasonable point-of-entry into the twistd code that accepts a Service but is still before the daemonization/logging/etc setup.one hack might be to set up the plugins table, then set
argvto something with a "subcommand" which vectors into a special tahoe-Service-making "plugin" (which really doesn't exist anywhere else on the filesystem, only in RAM). Another would be to somehow overridetwisted.application.app.ApplicationRunner.createOrGetApplication, possibly by monkeypatchingtwisted.scripts.twistd._SomeApplicationRunner, which sounds like a bad idea. (ApplicationRunneris the parent class of bothWindowsApplicationRunnerandUnixApplicationRunner, andtwisted/scripts/twistd.pyis responsible for loading the right one).It might also be possible to grab control by wrapping the
runAppintwistd.pybefore it is passed toapp.run. I've not yet investigated whether that would help, though, it might be too late.I'm not completely following the details, but the general idea sounds great to me.
I just started to write up a duplicate ticket.. fixing up the tags on this one so I can find it next time.
make it possible to change appname, Python package-directory name, perhaps other namesto stop using .tac files: make it possible to change appname, Python package-directory name, perhaps other namesReplying to warner:
Or we can put a
.tacfile in the source tree, and direct twistd to that one (using something likeos.path.join(os.path.dirname(allmydata.*file*), 'tahoe_node.tac')), rather than to the one in the node directory. Even though it doesn't "stop using .tac files", that would solve the problems in this ticket because the in-source.taccan change without any compatibility issues.This depends on the Tahoe source not being zipped, but we already say in source:setup.py that it is not zip-safe.
Elrond on
#tahoe-lafssaid that they had a problem starting nodes with a (slightly modified) version of the Tahoe-LAFS 1.8.2-3 Debian package, due to thepkg_resources.require('twisted')in the .tac file. Apparently twisted had been manually installed in a way that wasn't recognized by pkg_resources(possibly due to http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=631163). Commenting out thepkg_resources.requireworked.The output of
tahoe --version-and-pathwasWhen investigating workarounds, note that we can now depend on Twisted >= 10.1.
This is one of those tickets that needs to spawn off smaller and more closeable subtickets.
I just created #1539 which is a small step we could do (I think) right away and that might ease things in the future.
Replying to warner:
Brian made this change in changeset:ac3b26ecf29c08cb.
Okay, having re-read this ticket, it sounds like the way forward is for someone to figure out how to use
twistdas an importable library instead of as a command that gets executed and then loads our code. See comment:79179, comment:79180, comment:79181, comment:17. Is that the next step?Replying to zooko:
Yes. comment:17 is a way of doing that that will work, but is ugly. I don't know whether there's a more elegant way. (Duplicating some of twistd's daemonizing code would also work but would be even uglier.)
I was able to use the first technique from comment:79181 in my "toolbed" app (i.e. creating a fake in-RAM "plugin", then a sys.argv that references it). I'll build a patch that changes 'tahoe start' to use that technique so we can give it a spin.
Attachment notac.diff (10818 bytes) added
stop using contents of .tac files
davidsarah: How's that look?
Will review.
Starting review.
The patch applies cleanly to trunk and passes existing tests.
Re: this comment
That's OK but the error message that you're likely to get is "--basedir '--nodaemon' doesn't exist", which is not as helpful as it could be.
The patch adjusts some tests but doesn't appear to fully test the change in functionality. Will think about what tests there should be.
Continuing review...
tahoe start --helpsays "Usage: tahoe start options NODEDIR, which is wrong if the options should be after the nodedir. (Similarly forrestartetc.)The
twistdoptions are available but not documented intahoe start --help. Seetahoe debug trial --helpfor an example of how it is possible to get those options listed.Does the fact that "
# also ("--logfile", "tahoesvc.log")" is commented out mean that there is some regression in logging functionality?Replying to davidsarah:
Oh, that applies only to
startstop_node.run, and it has the effect of not redirecting stdout/stderr fortahoe runand so fixing #1489. That's a good change, but the comment should be removed or clarified, and/or we should document the change.Replying to [davidsarah]comment:34:
in source:git/docs/logging.rst for example.
"XYZ"could probably be something less mysterious like"StartTahoeNode", while still having negligible chance of colliding with any existing subcommand.Could an argument to
tahoethat does not start with"--"be confused bytwistdfor the command name?If I try
tahoe start ~/.tahoe foo(which should be an error), I get a twistd options listing followed by a TypeError traceback, which appears to be due to a bug atstartstop_node.pyline 114. This change fixes the bug, and snips some implementation details about twistd from the end of the options listing:I pushed patches addressing some of the above comments to https://github.com/davidsarah/tahoe-lafs/commits/1159-no-tac.
https://github.com/davidsarah/tahoe-lafs/commit/a7e4cab9c3367048d8ea326f559f65a6accdb8c1 is warner's original patch and subsequent ones are mine. They do not yet address comment:79200.
The
-d/--node-directory=/-C/--basedir=option isn't working correctly when twistd options are given, because there's nothing to separate the tahoe options from the twistd options. Not sure what to do about that, but it interacts with #166.I want this, but won't block 1.10 for it. Kicking it and its cousin #1539 into 1.11 . Should be easy to get done early in the next cycle.
+1 for kicking this out.
This is a blocker for #1950.
I've updated my branch with daira's recommended patches. https://github.com/warner/tahoe-lafs/tree/1159-notac
Is the code review completed on this one? Does it need to be updated or done once more?
zs260: hi, i'm trying to create systemd for tahoe introducer. i got the client working fine, but for introducer, i need to run it in foreground mode, tahoe run, and it complains " looks like it contains a non-client node (tahoe-introducer.tac). Use tahoe start instead of tahoe run"
zs260: introducer works fine with tahoe start, but when i use with systemd, type="forking", it does not work. systemd keeps waiting
daira: oh, zs260 left. I was just about to answer their question
daira: the fact that 'tahoe run' doesn't work for introducers (or key-generators or stats-gatherers) is a ticketed bug
daira: #1159 (also #937 which was closed as a duplicate of #1159)
daira: #1159 is likely to be fixed for 1.11
daira: if you need me to rebase my branch, please let me know. I'll run tests locally and make sure it hasn't bitrotted.
Yes please! Reassign me when you're done.
I've rebased the branch (and fixed one test which was failing.. not sure what exactly was happening there, the fix was to revert a test change that I made to allow the original patch to pass). The new branch is here:
https://github.com/warner/tahoe-lafs/tree/1159-notac-2
I think we should land this, and then close #1539 because this branch supercedes that one.
Note that this patch doesn't completely close #1159: it stops using the contents of the .tac files, but still relies upon their presence, in particular their filename: if "client" is a substring of the filename of the first .tac file found in the node's basedir, then we instantiate a
Client, if "introducer" is in the string, we instantiate anIntroducerNode, etc.We'll want another patch, after 1.11, to move this indicator into tahoe.cfg (and look for a .tac filename as a backwards-compatibility tool).
Rebased at https://github.com/tahoe-lafs/tahoe-lafs/tree/1159-notac-3. Reviewing now.
Fixed in changeset:87a6894e6299148d639279fbd909110ba00d0080/trunk.
Replying to warner:
Filed as #2325. What we've done with this patch is sufficient to "make it possible to change appname (#2011), Python package-directory name (#1950), perhaps other names".