Changes between Version 8 and Version 17 of Ticket #1074


Ignore:
Timestamp:
2010-07-15T03:49:29Z (14 years ago)
Author:
davidsarah
Comment:

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 zooko:

Here are my comments:

  1. This requires updates to 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).

  1. There should be some theory-of-operation doc, perhaps just at the top of the 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.

  1. 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 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 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.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #1074

    • Property Status changed from assigned to new
  • Ticket #1074 – Description

    v8 v17  
    1111 * it's unnecessary complexity;
    1212 * 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;
    13  * a Python script called {{{tahoe.py}}} can be run just fine on Windows as {{{tahoe}}}, provided that the {{{PATHEXT}}} environment variable includes "{{{.py}}}".
     13
     14A 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}}}".)