[tahoe-dev] [tahoe-lafs] #1074: get rid of tahoe.exe launcher

[tahoe-lafs]

#1074: get rid of tahoe.exe launcher
---------------------------+------------------------------------------------
     Reporter:  zooko      |       Owner:  davidsarah                      
         Type:  defect     |      Status:  new                             
     Priority:  critical   |   Milestone:  1.7.1                           
    Component:  packaging  |     Version:  1.6.1                           
   Resolution:             |    Keywords:  windows win64 setuptools unicode
Launchpad Bug:             |  
---------------------------+------------------------------------------------

Old description:

> There is a binary ({{{cli.exe}}}) in the Tahoe-LAFS source tree under
> revision control. It is built from {{{launcher.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:
>  * it mangles any non-ASCII arguments, preventing us from fixing #565 on
> Windows;
>  * it does not work on Win64;
>  * it adds a small amount of overhead to running CLI commands;
>  * it's unnecessary complexity;
>  * it isn't in the spirit of open source to have a binary that is not
> compiled from source as part of our build process -- getting rid of it
> fixes this without complicating the build or requiring a C compiler;
>  * a Python script called {{{tahoe.py}}} can be run just fine on Windows
> as {{{tahoe}}}, provided that the {{{PATHEXT}}} environment variable
> includes "{{{.py}}}".

New description:

 There is a binary ({{{cli.exe}}}) in the Tahoe-LAFS source tree under
 revision control. It is built from {{{launcher.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:
  * it mangles any non-ASCII arguments, preventing us from fixing #565 on
 Windows;
  * it does not work on Win64;
  * it adds a small amount of overhead to running CLI commands;
  * it's unnecessary complexity;
  * it isn't in the spirit of open source to have a binary that is not
 compiled from source as part of our build process -- getting rid of it
 fixes this without complicating the build or requiring a C compiler;

 A Python script called {{{tahoe.py}}}, say, can be run just fine on
 Windows as {{{tahoe}}}, provided that the {{{PATHEXT}}} environment
 variable includes "{{{.py}}}". (Actually we're going to use
 "{{{.pyscript}}}".)

--

Comment (by davidsarah):

 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:
 > > To answer zeromus' point, {{{cli.exe}}} (or ''name''{{{.exe}}}) did
 not have any baked-in paths; it used {{{argv[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.
 >
 > Correction: the ''name'' script (which is only used on cygwin) does have
 hard-coded paths. I'll see if I can fix that.

 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 [comment:16 zooko]:
 > Here are my comments:
 > 1. This requires updates to [source:docs/running.html] (although I'm not
 yet sure what exactly needs to be changed)

 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 (probably {{{python setup.py winsetup}}}).

 > 2. There should be some theory-of-operation doc, perhaps just at the top
 of the [source:setuptools/command/easy_install.py], explaining how we set
 up an executable on Windows.

 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.

 > 3.
 > {{{
 >     except Exception:
 >         # okay, probably it was already gone
 > }}}
 > This could be a narrower catch:
 > {{{
 >     except EnvironmentError, le:
 >         # Ignore "No such file or directory", collect any other
 exception.
 >         if (le.args[0] != 2 and le.args[0] != 3) or (le.args[0] !=
 errno.ENOENT):
 >             excs.append(le)
 > }}}
 > (copied from http://tahoe-
 lafs.org/trac/pyutil/browser/trunk/pyutil/fileutil.py?rev=218#L205 )

 I've changed it (and the similar case for {{{tahoe.exe}}}) to:
 {{{
       except Exception:
           if os.path.exists(tahoe_script):
               raise
 }}}

 > Okay overall I don't feel like I [can't] ''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,

 Yes.

 > 2. the "bug in cygwin" bash seems like it ought to be reported to cygwin
 via their bug tracker.

 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.

 > 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,

 {{{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 at 4329#L42 runner] function.
 We previously had no tests that Unicode arguments were interpreted
 correctly when the {{{bin/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.)

 > although it may be (rightly or wrongly) disabled on Windows and/or
 Cygwin.

 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.

 > 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?

 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.

-- 
Ticket URL: <http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1074#comment:17>
tahoe-lafs <http://tahoe-lafs.org>
secure decentralized file storage grid