get rid of tahoe.exe launcher #1074
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
2 Participants
Notifications
Due Date
No due date set.
Reference: tahoe-lafs/trac-2024-07-25#1074
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?
There is a binary (
cli.exe
) in the Tahoe-LAFS source tree under revision control. It is built fromlauncher.c
in zetuptoolz:http://tahoe-lafs.org/trac/zetuptoolz/browser/launcher.c
source:setup.py copies it to
bin\tahoe.exe
when building.There are several reasons to get rid of this launcher:
A Python script called
tahoe.py
, say, can be run just fine on Windows astahoe
, provided that thePATHEXT
environment variable includes ".py
". (Actually we're going to use ".pyscript
".)there is a binary (cli.exe) in the Tahoe-LAFS source tree under revision controlto get rid of tahoe.exe launcherAttachment unicode-args-and-rm-tahoe-exe-wip.dpatch (63266 bytes) added
Attaching my work-in-progress for Unicode argument support on Windows and removal of tahoe.exe, so that it doesn't get lost.
Is this going to require users to edit their PATHEXT environment variables in order to run Tahoe-LAFS? Or will installing Python do that for them? If we're going to make this change for v1.7.1 let us make it early in the release cycle so that users on Windows have plenty of time t let us know if it changes the behavior in a way that causes them problems.
Replying to zooko:
Installing Python does not edit
PATHEXT
. But it's very easy to do so without going through the GUI:This takes effect for any new instances of
cmd.exe
run by the current user. (There is an/m
option to set the variable for all users on the current machine, but it will typically fail on Vista and later unless you runsetx
with elevation, and I don't think we need to do that anyway.)setx.exe
is an executable in%SystemRoot%\system32
, it's not acmd.exe
builtin. To setPATHEXT
for the currentcmd.exe
instance as well (e.g. in a batch file), also doset "PATHEXT=%PATHEXT%;.PY"
, which ''is'' a builtin.Installing Python does associate the
.py
extension with that Python interpreter, so even without thesetx
command, it's sufficient to runtahoe.py
from the command-line instead oftahoe
.OK, that makes sense.
Replying to [davidsarah]comment:4:
On WinXP Service Pack 2 and later, apparently. But we can probably assume that. (If any system running earlier Windows is connected to the Internet and still usably free of malware, I'd be surprised.)
This issue appears to be blocking some of our buildbots from working (unless I misunderstand):
http://tahoe-lafs.org/buildbot/builders/Soultcer%20Win7-64%20py2.6
http://tahoe-lafs.org/buildbot/builders/Dcoder%20Win7-64%20py2.6
Also I think it is blocking #565. Let's prioritize this ticket! What's the next step? I think it is to decide if we want the new install docs to instruct all users on all platforms to execute a file named "tahoe.py" instead of a file named "tahoe", or if we want the install docs to differ depending on platform here, or what.
I don't fully understand how things would change at this level -- install docs and user experience -- if we got rid of
tahoe.exe
.I don't speak for every windows user, but I can try to. Once a windows user has gone to the trouble to install python and build tahoe, I don't think he cares whether he has to double click tahoe.exe or tahoe.py. It is going to feel gross to him, and very likely unacceptably so, compared to a redistributable zipfile with an executable in it, e.g. a py2exe frozen thingy.
I advise you not to sweat this exe at all and focus on making sure the py2exe process is working instead.
Folks who use python and windows will have a different opinion, because they're used to this.
Replying to zeromus:
This is the command-line interface. The user isn't double-clicking anything; they're typing "
tahoe ...args...
" at a Command Prompt. (Or a cygwin shell prompt; we don't support cygwin, but I don't want to break Tahoe on cygwin unnecessarily.)Apart possibly from running an extra command in the install process to set up PATHEXT, the difference between
tahoe.exe
andtahoe.py
is an implementation detail that doesn't affect the user interface.For bbfreeze, which is similar to py2exe, see #585 -- but I think that's a distraction as far as this ticket is concerned, since we don't want to create a dependency on bbfreeze (or py2exe).
Agh I am mixed up. You're right. but--
There is one huge difference between the .exe and the .py, if I recall correctly: the .exe has the c:\path\to\python baked into it which renders it completely non redistributable. The .py has no such dependency in it. As a consequence, I can send my python directory to someone else and have them run the .py, but the .exe isn't going to work unless they put it in the same place in their filesystem.
Samuel Neves wrote on tahoe-dev:
Please try the attached zetuptoolz-dont-write-script-exe.2.dpatch (on top of unicode-args-and-rm-tahoe-exe-wip.dpatch).
If your PATHEXT does not already contain .py, also do:
Then test the tahoe cli in a new Command Prompt.
Attachment zetuptoolz-dont-write-script-exe.2.dpatch (6775 bytes) added
zetuptoolz: don't write .exe files for scripts (fixed version)
If you already have a non-working
tahoe.exe
file in your scripts directory from a previous install, you will need to delete it.Here is a new version that handles more corner cases and is less likely to break other scripts installed using zetuptoolz. This also fixes Unicode console output: it now magically works independently of the code page setting. (If stdout or stderr are redirected, the output is UTF-8.)
To answer zeromus' point,
cli.exe
(or name.exe
) did not have any baked-in paths; it usedargv[0]
to find its name-script.py
file. The name.pyscript
and name files generated by the new version of zetuptoolz also do not, so there is no regression.Attachment zetuptoolz-dont-write-script-exe.3.dpatch (26851 bytes) added
Changes to zetuptoolz to get rid of .exe wrappers, and support Unicode arguments.
Attachment unicode-args-and-rm-tahoe-exe.3.dpatch (21557 bytes) added
Changes to Tahoe needed to work with new zetuptoolz (that does not use .exe wrappers on Windows), and to support Uni code arguments and stdout/stderr.
Attachment pyscript.reg (182 bytes) added
regedit file to set up .pyscript association (run with 'regedit pyscript.reg' and accept the scary prompts).
Replying to davidsarah:
Correction: the name script (which is only used on cygwin) does have hard-coded paths. I'll see if I can fix that.
Here are my comments:
This could be a narrower catch:
(copied from http://tahoe-lafs.org/trac/pyutil/browser/trunk/pyutil/fileutil.py?rev=218#L205 )
Okay overall I don't feel like I can really review this patch very well without more doc because I don't understand it very well. It looks to me like there are at least two parts that deserve to be publicized as being of interest to people outside of Tahoe-LAFS: 1. the "deep magic" in windows_fixups() seems like it should be offered to setuptools, distribute, and/or python core teams via their bug trackers, 2. the "bug in cygwin" bash seems like it ought to be reported to cygwin via their bug tracker. Let's open those tickets and link them back to this one and probably link them into launchpad.net (I'm happy to do some of that ticket gardening.)
Now what about testing? I think that source:trunk/src/allmydata/test/test_runner.py will test this code, although it may be (rightly or wrongly) disabled on Windows and/or Cygwin. Also we don't have a Cygwin buildslave. Also this patches doesn't update tests that of the unicode stdout on Windows. Probably there is already a test from the v1.7.0-cycle unicode work that tests unicode stdout and those tests need to be marked as no-longer-TODOs on Windows?
Note that the only discussion of cygwin on this ticket should be about supporting Windows Python (
sys.platform == "win32"
, which confusingly includes Win64), but allowing the scripts to run correctly from a cygwin shell.Supporting cygwin Python (
sys.platform == "cygwin"
) is an entirely different issue, for which I've opened ticket #1119.Replying to davidsarah:
OK, done. You can now move or copy the name and name
.pyscript
files, provided they are in the same directory. (The path to the Python interpreter is still hard-coded, because it should be possible to move the script relative to the interpreter; also, because that is what setuptools does for cygwin Python.)Replying to zooko:
quickstart.html needs to be updated to add a step just before running
python setup.py build
. I've been waiting to do that until I know what command needs to be run (probablypython setup.py winsetup
).I'm not sure it should be at the top of that file, but I'll add a new
zetuptoolz.txt
file describing the differences between setuptools and zetuptoolz.I've changed it (and the similar case for
tahoe.exe
) to:Yes.
It turns out that this was only because we were writing the script with CRLF line endings. It's still a bug that cygwin bash doesn't tolerate that (and I'll report this bug if it is present in the latest version), but it doesn't affect us any more.
test_runner.py
will test (to the same extent as on other platforms) that we respond correctly to Unicode arguments given that they are passed to the [source:src/allmydata/scripts/runner.py@4329#L42 runner] function. We previously had no tests that Unicode arguments were interpreted correctly when thebin/tahoe
script is run as a separate process, but I've added one.(Due to limitations of Python's and twisted's process-spawning primitives, it has to use the mangled encoding on Windows rather than testing with an actual Unicode argument, but that at least tests that the implementation is behaving as we expect.)
The tests that involve twisted acting as a daemon are disabled on Windows (#27). We can test Unicode arguments without that. The disabling of runner tests on cygwin is ticket #908.
They were not marked as TODO; they were skipped because the Unicode arguments and/or output was not representable in the output encoding. They're no longer skipped.
Attachment unicode-args-and-rm-tahoe-exe.4.dpatch (33575 bytes) added
Changes to Tahoe needed to work with new zetuptoolz (that does not use .exe wrappers on Windows), and to support Uni code arguments and stdout/stderr -- v4.
unicode-args-and-rm-tahoe-exe.4.dpatch moves the Windows-specific code to src/allmydata/windows/fixups.py, and makes the tests pass for the right reasons :-) We also have a new test that Unicode arguments and output works (added to TheRightCode in runner.py), and a change to quickstart.html. The zetuptoolz doc isn't written yet.
I just updated the wiki/PatchReviewProcess with this ticket in mind. I added:
I chose not to sign off on this patch when I reviewed it because I didn't think I quite achieved number 1 there, and also because I thought it needed more/better doc or code-refactoring to make it easier to understand. It sounds like from comment:77841 that unicode-args-and-rm-tahoe-exe.4.dpatch may have accomplished some of that.
Hm, in fact, regarding number 3 there, this patch probably needs to come with a NEWS snippet since it has a user-visible impact.
Here is a proposed NEWS snippet for this ticket:
Some notes from IRC as fodder for
zetuptoolz.txt
:The current ticket1074 branch has a couple of interesting failures on my Mac OS 10.6 "zomp":
Attachment zetuptoolz.dpatch (164966 bytes) added
Changes to handling of scripts on Windows: get rid of .exe wrappers and use .pyscript/.pyw files instead. (Note: this patch is to zetuptoolz trunk, not Tahoe.)
Attachment add-zetuptoolz-txt.dpatch (103892 bytes) added
Add zetuptoolz.txt documenting changes made by the fork
Attachment win_script_wrapper.txt (4744 bytes) added
Replacement of file documenting Windows wrappers (this is in zetuptoolz.dpatch, but may be too hard to read in the diffs).
I decided not to mess about with
sys.path
:.pyscript
is that the user doesn't have any reason to override that association, whereas they might want to override.py
.Replying to zooko:
That looks fine, but
s/unicode/Unicode/
. Also reference #565 and #1073.Replying to zooko:
Answered in ticket:1137#comment:77832.
Attachment zetuptoolz-only-warn-if-not-our-site-py.dpatch (101276 bytes) added
setuptools/command/easy_install.py: if site.py does not start with the 'def __boot():' signature, warn rather than failing the installation. (This is a patch to zetuptools, not Tahoe-LAFS.)
Attachment zetuptoolz-only-warn-if-not-our-site-py-v2.dpatch (101454 bytes) added
setuptools/command/easy_install.py: if site.py does not start with the 'def __boot():' signature, warn (and do not overwrite it) rather than failing the installation.
In [4627/ticket798]:
Fixed, leaving open for review.
The bundled zetuptoolz was updated to 0.6c16dev on trunk in changeset:c045241a55056848 changeset:d112df80ff4b0265 changeset:58cce54ada16c012 changeset:e16c794f192f0cd7. These correspond to 584 586 588 on zetuptoolz trunk, and are documented in zetuptoolz.txt and win_script_wrapper.txt.
The changes to Tahoe-LAFS to take account of the zetuptoolz changes and enable Unicode arguments on Windows are changeset:37b07a545f17f8bb changeset:9d04b2a317c2ecf4 changeset:1fad717aaae6382c changeset:54bbc5325f9dae8b changeset:ec6a674a5c87fd55 changeset:b5f052a0d7abfd55 changeset:1a5a33886d771db6 changeset:88b6c57a1c12187f changeset:d81e8acf882a7784. (Sorry, those should probably have been refactored a bit more before committing them to trunk.)
Here's a combined view of the Tahoe-LAFS changes (excluding changeset:d81e8acf882a7784).
This ticket introduced some newly written code that calls Windows APIs from Python, which is tricky and error-prone:
Ideally these would be reviewed by someone familiar with Python on Windows.
Replying to zooko:
Later (less buggy :-) versions of the console output and Unicode argument fixes have been submitted to Python issues 1602 and 2128. I'm not entirely sure they belong in setuptools or distribute (that would have to be implemented by putting this code in the Windows wrapper script, and I think it's probably better that application authors run it explicitly).
Replying to davidsarah:
#1302 (installing Python 3 breaks bin\tahoe on Windows) is evidence that I didn't think carefully enough about this code :-( -- although I think it would be too disruptive to fix that issue for 1.8.2.
I've reviewed this. I didn't understand the Windows-specific parts so I asked sneves to look at that. I didn't really study the code carefully to look for errors but I read through it and it looked good to me.
Let me say that the comments are really helpful in understanding this patch (in particular what bugs/issues are being worked around; it'd be impossible to figure it out otherwise).
I went through the code (particularly the Win32 portions) and was unable to find anything blatantly wrong with it. It seems to be working, too.