takluyver / pynsist

Build Windows installers for Python applications
https://pynsist.readthedocs.io/
Other
883 stars 119 forks source link

Inconsistencies when running a CLI constructed with pynsist #166

Closed adferrand closed 5 years ago

adferrand commented 5 years ago

So I continue to build my installer for certbot, and I would like to expose a certbot command on Windows command line.

The relevant part of installer.cfg is the following:

[Command certbot]
entry_point=certbot.main:main

Installer is built correctly, installation behaves correctly. However, from one build + install to another, sometimes the certbot command fails. And when it fails, it just returns failed to create process. Nothing more, and I did not find any way to trigger some debug mode and provide more logs.

So I understood that the certbot.exe executable, which is copied from the NSIS installer into %installdir%\bin\, is available to the user through an injection in PATH. Direct running of certbot.exe leads to the same inconsistencies.

I also understood that certbot.exe is in fact a copy of cli-{32|64}.exe from the project https://github.com/takluyver/win_cli_launchers, that is itself a 3 years old extract of the cli-{32|64}.exe from the well-known setuptools project. Tried to copy the executables from a fresh and recent copy of setuptools, same inconsistencies.

Finally, I understood that cli-{32|64}.exe is a VisualStudio binary that reads a python script in the same folder, with the name of type cli-{32|64}-script.py (or something-script.py if the executable is renamed to something.exe). It will then resolve the Python executable path from the environment or an optional shebang available at the beginning of the python script file (for instance #!C:\some\path\to\python).

I could not succeed in repairing this behavior from the existing executables files provided by setuptools or win_cli_launchers launchers. However, I succeeded in reproducing the expected behavior with a batch file. This one:

@echo off
setlocal ENABLEDELAYEDEXPANSION

set env_dir=%~dp0
set install_dir=%env_dir:\bin\=%
set python_path=%install_dir%\Python\python.exe
set script_path=%install_dir%\bin\%~n0-script.py
set error=false

if not exist "%python_path%" 2> nul (
    echo Error, python executable not found on path: %python_path%
    set error=true
)

if not exist "%script_path%" 2> nul (
    echo Error, python script file not found on path: %script_path%
    set error=true
)

if %error%==true (
    exit /b 1
)

"%python_path%" "%script_path%" %*

This batch file, assuming to be copied in the bin directory constructed by pynsist, and assuming to be named something.bat, will execute the python script file something-script.py, using the Python distribution prepared by pynsist, and passing all command lines arguments to the python script file.

It is everything needed, when the bin directory is in the user PATH, to have a working something command available in CLI. And this works everytime.

One question and one proposition

1) Question

Is there an explanation or a process to find it, about the inconsistencies I noticed with cli.exe ?

2) Proposition

We need to consider also that cli.exe comes from the easy_install approach of setuptools, but the setuptools maintainers advise to not use easy_install anymore, because there are better alternatives. One of them is that starting with Python 3.3 for Windows, the py launcher can start scripts with a correct interpretation of the shebang to find the correct Python environment.

But anyway the cli.exe executable here is used only to expose CLI commands. And because the Python environment is strictly controlled and isolated using a NSIS installer, one can have the expected behavior with a simple batch file as I shown before.

So why not replacing the cli.exe executable by a cli.bat batch script quite similar to what I wrote ?

takluyver commented 5 years ago

I think you've correctly understood what's going on with the exe wrappers; sorry it's not more straightforward.

I've played around with batch files in the past, but I was swayed by @pfmoore's notes documenting that .exe files are the only way to reliably make a command interface on Windows; everything else behaves strangely in some situation or other. Paul describes how he too was initially keen to avoid exes and was eventually convinced by the problems people encountered, so I don't think I'm going to find a solution he has missed.

Last time I checked, pip-installing a package with a command-line entry point on Windows still created a .exe wrapper, e.g. pynsist.exe. I think the way it works has changed a bit since the pynsist-script.py mechanism I borrowed; I don't know if the new way is more reliable.

It looks like this can occur if there are spaces in the path, and one solution is to quote the shebang line (SO answer, setuptools issue). I'm pretty sure I didn't think to do that, so commands are probably broken for all systemwide installs in the "Program Files" directory. Does that fit with what you're seeing? Can you try quoting the shebang manually in an installed foo-script.py file and see if that fixes the problem?

adferrand commented 5 years ago

Indeed, I understand that bat scripts will push some troubles one day. Really I would like to stay with a exe wrapper approach.

About the error, I saw also discussions about spaces in Python path, but the exe wrapper was already working before with the same installation path, that includes spaces in my situation, and the shebang is already quoted.

I really do not know where the problem came from. I thought it could be about the new version of pynsist or about the last modifications on my building script. After reverting the two, no improvements. I also thought it was about specific particularities that occurred on my computer only, and tried with another one. Without success. I reduced the targeted script to its minimum, a simple print, and also tried to use directly the cli.exe from your repository, and from the setuptools distribution.

With setuptools, it worked once with a minimal python script, then not anymore.

I will try with my professional laptop tomorrow and let you know.

takluyver commented 5 years ago

Ah rats, I thought I'd found the problem there. Have you checked that the shebang path is correct?

Other than that, I don't know what can cause the "failed to create process" error. Maybe Paul might have some ideas?

When you're testing installing the same app multiple times, it may be useful to manually uninstall it between installs, in case you're not already. I'm not sure if installing over existing files always replaces them.

On Sun, 7 Oct 2018, 10:03 p.m. Adrien Ferrand, notifications@github.com wrote:

Indeed, I understand that bat scripts will push some troubles one day. Really I would like to stay with a exe wrapper approach.

About the error, I saw also discussions about spaces in Python path, but the exe wrapper was already working before with the same installation path, that includes spaces in my situation, and the shebang is already quoted.

I really do not know where the problem came from. I thought it could be about the new version of pynsist or about the last modifications on my building script. After reverting the two, no improvements. I also thought it was about specific particularities that occurred on my computer only, and tried with another one. Without success. I reduced the targeted script to its minimum, a simple print, and also tried to use directly the cli.exe from your repository, and from the setuptools distribution.

With setuptools, it worked once with a minimal python script, then not anymore.

I will try with my professional laptop tomorrow and let you know.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/takluyver/pynsist/issues/166#issuecomment-427686842, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUA9VmSr8DpARoxwao-3_luhFhjd1HVks5uimwdgaJpZM4XLz6N .

pfmoore commented 5 years ago

I'm not sure I'm really following what the problem is here. Do I gather that pynsist uses the old setuptools foo.exe <-> foo-script.py executable wrapper? I've had problems with that in the past, but mostly around the setuptools cruft that was used in the script file.

The .exe wrappers used by pip are the ones from distlib, and these have seemed pretty reliable (there have been problems in the past, but Vinay is pretty good at fixing things, so if you use the latest version you should be OK).

"Failed to create process" is in my experience typically caused by a required DLL not being available on PATH. So what I'd want to know is the exact details of:

  1. Where is certbot.exe located when you run it?
  2. Are you running it by explicit path name, or is it being found on PATH?
  3. What precisely is the shebang line of the script? (embedded in the exe if it's the distlib one, in the -script file if it's the setuptools one?
  4. What DLLs are needed/being loaded by certbot and/or Python (particularly the C runtime) and are they all accessible on the DLL search path (basically PATH, but with a few additional possibilities like "the directory containing the exe")?

Hope that helps.

takluyver commented 5 years ago

Thanks Paul!

Do I gather that pynsist uses the old setuptools foo.exe <-> foo-script.py executable wrapper?

Yup, that's right. We could switch to the distlib wrapper if that's better - I didn't know about that when I added the mechanism.

I'll let @adferrand answer the questions about specifics.

If it's related to DLLs, would examining the exe with dependency walker help, or is that so old it's not useful any more?

pfmoore commented 5 years ago

If it's related to DLLs, would examining the exe with dependency walker help, or is that so old it's not useful any more?

It should be fine. Personally, I tend not to use it, but that's because I find the output too detailed (dependencies of dependencies, etc...) rather than any other reason. Also, it has a tendency to mark some things as unresolvable, which are actually optional and so aren't actual issues. My normal approach is mainly intuition and "Fiddle around until it works" :-) But dependency walker is fine, particularly if you're used to interpreting its output ;-)

pfmoore commented 5 years ago

We could switch to the distlib wrapper if that's better

It's probably not going to matter for this problem. I prefer it mostly for familiarity and because I don't like having to copy 2 files about rather than just one.

adferrand commented 5 years ago

Thanks for your responses @pfmoore and @takluyver. First, I want to apologize about the bug I submit without a reliable way to make the bug appear. It is really hard to find it, and you are giving me good insights to understand what is going on.

I am pretty sure that the bug is primarily due to something I screwed up on the two machines I tested. But the software (certbot) that am trying to package an installer for is used by many users, and maybe way more if it becomes accessible to Windows. On Certbot project, there are already 1000 issues that the few Certbot maintainers have already a lot of difficulties to handle. So I do not want to submit something that is not totally reliable, or there is a risk that dozen of people will push new issues about 'failed to create process' on the github repository.

Be sure that for any solution that could be found, I am willing to make the appropriate developments on the pynsist project.

So to answer your questions @pfmoore first:

  1. The file certbot.exe is located on C:\Users\Adrien Ferrand\AppData\Local\Certbot\bin\ when installed by the installer build with pynsist. Yes, there is a space. It is really suspicious, but I tried with another machine, and the shebang is correctly quoted. But I go to that after.
  2. I tried both, only certbot letting system to find it through the PATH, and explicitly calling certbot.exe from the bin directory. Same results.
  3. So it is a shebang in a separated script file because it is the cli.exe from setuptools. Looks fine to me, correctly quoted. But I will double check in this evening with my machines.
  4. Certbot has quite complex dependencies, but they are shadowed by the generated certbot-script.py: the script created by pynsist, using only the standard Python library, will load in os.path the actual dependencies at runtime, then start the target entry point. So at the time to start certbot-script.py throught certbot.exe, only Python DLLs are required.

So from what your are saying, I definitively think that certbot.exe could not find the Python distribution installed by NSIS installer. I do not know the reason yet, but I will search in that direction. I suspect the shebang to be invalid. I will also try dependency walker to have a more deep insight of the wrapping process.

Note that I tried this morning on my professional laptop, and everything worked correctly :). So I am pretty sure that is an quite isolated bug.

Nevertheless, either if wrapper from setuptools or from distutils is used, I think it can only better if pynsist tries to use the most up-to-date wrapper it can find;

What do you think of that, @takluyver ?

pfmoore commented 5 years ago

Do you have any case where the wrapper worked in a situation where the path has a space? I don't want to point the finger without justification, but I have vague recollections of problems with the setuptools wrapper and spaces. It's not simply a matter of "quoting correctly" - the quoting is interpreted by the wrapper, not the OS, so it's entirely possible for what looks correct in the script to be misinterpreted by the wrapper.

If it's possible to test with the distlib wrappers, that would also be a good idea, just for comparison.

Also, while you're testing this sort of scenario, I'd also suggest testing weird cases like paths with non-ASCII characters (both ones that are valid in the user's codepage and ones that aren't) as those are other cases where we had issues with pip.

takluyver commented 5 years ago

Yeah, the fact that your username has a space in there makes me wonder if there's some other issue, e.g. if the cwd path has a space in, or the home directory, maybe the wrapper has some problems. What do the paths look like on your professional laptop, where you said it was working?

I'd rather not depend on setuptools, and I think you mentioned that using the newer exe wrappers from setuptools didn't solve your problem anyway, so it doesn't seem like that's going to help the current problem.

I think the next step might be for you to try with the distlib wrappers, which work by appending the script to the exe file. I think the ScriptMaker class is what will write an exe:

https://distlib.readthedocs.io/en/latest/tutorial.html#using-the-scripts-api

adferrand commented 5 years ago

Yes I think also that spaces in path are definitively the ideal suspect. My professional laptop is in a situation where there is no spaces. What bother me a lot however, is that the wrapper used to work in the exact same situation than now, with spaces and everything else. It just stopped working from one to next day. What bother me a lot more is that I tried to do the same on a laptop without any pre-installed python or anything related to code development. With the same error.

But I do not bother a lot about an issue quite isolated. I will check the paths, try other one, verify non-ASCII... and state myself the issue as closed because "your os is crappy, reformat instead of waiting for a fix".

adferrand commented 5 years ago

But I will try the distlib approach. Seems promising and I did not tried yet.

If it does solve the issue, I think it would worth to integrate it to ensure stability.

By the way @takluyver, why do you not want to rely on setuptools ? It is almost always installed in a Python distribution, so often that pip freeze does not include it if you do not ask explicitly. And the distlib wrapper itself is in fact included in setuptools, like said here https://docs.python.org/3/library/distutils.html, and used as the mainstream approach here https://setuptools.readthedocs.io/en/latest/setuptools.html#automatic-script-creation.

takluyver commented 5 years ago

Well, I'd like to find a solution to the problem - even if there's something weird in your environment that triggers it, you're probably not the only person with a weird environment, and I want Pynsist to build installers that work for as many people as possible. If you want to reformat to work around it, you can, but if you've got the time, I'd rather keep trying to understand the problem and find resolutions.

I think you're confusing distlib with distutils: setuptools is based on distutils, which is part of the Python standard library and doesn't include any functionality for wrapper exes. Distlib is a third-party library maintained by @vsajip, which is now used in pip to create the exe wrappers when you install from a wheel.

I'm against relying on setuptools because I try to avoid it wherever possible. The history of Python packaging is complex, but briefly: setuptools was created to fix problems and limitations of distutils: it improved on a number of things (like exe wrappers for Windows), but introduced a whole lot of its own complexity in the process (like the egg package format). I've been bitten by side-effects of setuptools enough times that I actively avoid it; win_cli_launchers exists specifically to provide those launchers without the rest of setuptools.

vsajip commented 5 years ago

Just for information - the distlib launchers are actually developed under a separate open source project:

https://bitbucket.org/vinay.sajip/simple_launcher/

It provides 32-bit and 64-bit console and windowed launchers, and those four .exe files are bundled with distlib. The ScriptMaker creates .exe files from a source script and launcher, which append the script to the launcher to create the .exe. When it's run, the executed code finds the appended script, decodes its shebang to determine the interpreter to launch, and then launches the interpreter to actually execute the script.

vsajip commented 5 years ago

As far as I know, distlib has resolved all reported issues relating to spaces in shebang paths. However, if a failure in a launcher can be readily reproduced (whether it be a launcher failing to handle a valid shebang, or ScriptMaker writing an invalid shebang), I will certainly take a look at a distlib issue raised for it!

takluyver commented 5 years ago

Thanks Vinay, it sounds like it might be best for Pynsist to adopt your launchers.

Are you aware of any unresolved issues with the setuptools launchers around spaces in path names which @adferrand might be encountering?

adferrand commented 5 years ago

Ok guys, I have some news.

So first, yes, there is definitively something with paths that have spaces. If I use the default installation path (C:\Users\Adrien Ferrand\AppData\Local\Certbot), the certbot.exe in bin fails to start. But if I install in C:\Certbot, then it works correctly. Always with quoted shebangs ...

Then I tried the approach to build a merged executable using distlib (sorry for the confusion I did by the way). The script was this:

from distlib import scripts

def build_executable():
    script_template = '''# -*- coding: utf-8 -*-
import sys, os
import site
installdir = os.path.dirname(os.path.dirname(__file__))
pkgdir = os.path.join(installdir, 'pkgs')
sys.path.insert(0, pkgdir)
# Ensure .pth files in pkgdir are handled properly
site.addsitedir(pkgdir)
os.environ['PYTHONPATH'] = pkgdir + os.pathsep + os.environ.get('PYTHONPATH', '')

# Allowing .dll files in Python directory to be found
os.environ['PATH'] += ';' + os.path.dirname(sys.executable)

if __name__ == '__main__':
    from %(module)s import %(func)s as main
    main()
'''
    maker = scripts.ScriptMaker('.', 'build', add_launchers=True)
    maker.script_template = script_template
    maker.make('certbot1 = certbot.main:main')

if __name__ == '__main__':
    build_executable()

The script is launched from the installation directory C:\Users\Adrien Ferrand\AppData\Local\Certbot created by the NSIS installer, using the Python distribution embedded, and after adding the extracted distutils wheel in the relevant place to be found by the embedded Python distribution.

It generated under build the executables certbot1.exe and certbot1-3.7.exe.

And they are working perfectly !

So yes, the wrapper from distlib seems more reliable than from setuptools.

adferrand commented 5 years ago

Ok so now @takluyver, if you want the distlib approach to be used in pynsist, there is a big thing to consider.

The wrapper from @vsajip project simple_launcher can be compiled in two modes. In the first one, the script to be wrapped needs to be in the same directory than the wrapper, with its name as [wrapper_name]-script.py. So the same than for setuptools.

In the second one, the script needs to be appended as a ZIP archive at the end of the wrapper file. This is that approach that is used in distlib.

So depending on the choosen mode, integration will be hugely different.

With the first approach, we need to compile the simple_launcher project without the embedding mechanism (or maybe @vsajip can give a way to get precompiled binary). But nothing that a good AppVeyor could not do for instance on your win_cli_launcher project. Then, new version of win_cli_launcher, new version of pynsist using it, and we are good to go.

Two drawbacks with this simple approach. First, I did not tested it (yet, installing Visual C++ Tools is painful but I will try). Second, its quite a manual approach: win_cli_launcher should be redeployed each time a modification is pushed to simple_launcher ideally.

With the second approach, it is more elegant: pynsist relies on distlib to build the wrapper. As it is the base to construct Python on Windows, it is far more covered. But the integration path is waaaaay harder. Basically, the wrapper needs to be constructed during the execution of the NSIS installer for each [Command something] specified. So: 1) Gather all command entrypoints in a file copied during installation 2) Install distlib during the NSIS installer execution (certainly under Python folder to not interfere with pkgs 3) Modification of the NSIS template to execute a function similar to what I wrote, that consumes the entrypoints file to generate each wrapper required in bin 4) Remove _rewrite_shebangs.py not needed anymore (shebangs will be resolved from sys.executable as the script in 3) will be called using the embedded Python distribution 5) Some cleaning (like the something-python_version.exe)

vsajip commented 5 years ago

@adferrand, good news that the distlib wrappers are working for your use case here. Caveat: although the simple_launcher allows compilation in two modes, the side-by-side XXX-script.py option has not been maintained or tested for quite a while - when I originally wrote the launcher I initially thought to keep the method similar to setuptools, but then realised there was a better way. The old way was left around with conditional compilation directives, but I've never need to look back at that old way, as the appended zip archive works well IMO. Although in theory it should work the same way with XXX-script.py (as only the python interpreter invocation part needs to be different), I don't plan on doing anything special to support that (as its only benefit IMO is that it's similar to how setuptools does it).

Some cleaning (like the something-python_version.exe)

Not sure what you mean here, but if you're referring to e.g. certbot1-3.7.exe, you can avoid ScriptMaker creating that altogether by setting an option on the instance used, before calling make() - maker.variants = set(['']) - which would only write e.g. certbot1.exe.

takluyver commented 5 years ago

Thanks both. It sounds like the distlib launchers do solve the issue with spaces in the path, and we should stick to using them with the appended script like distlib itself does.

I'll spend a while digging into distlib and the simple_launcher code and figure out how I want to integrate them into Pynsist.

adferrand commented 5 years ago

Thanks @vsajip, yes the variants was what I was thinking !

To add another possibility with distlib @takluyver, you can also still have, as a command line, a plain python script generated during the NSIS installer configuration, as currently in pynsist.

But instead of rewriting its shebang, stick with #!python. Then you will need to execute a script during this NSIS installer execution, as its done with _system_path.py with the embedded python distribution. Distlib will need to be available to the embedded python distribution. The script can be like this:

from distlib import scripts
maker = scripts.ScriptMaker('scripts', 'bin', add_launchers=True)
maker.variants = set([''])
maker.make('mycommand.py')

This will generate bin/mycommand.exe from the script scripts\mycommand.py. The trick is just to do that during the installation, not the installer build, to allow distlib to correctly resolve the shebang to point at the actual location of the embedded python distribution.

adferrand commented 5 years ago

Anyway once you defined the approach you want for pynsist, I will be glad to help.

takluyver commented 5 years ago

@adferrand could you have a go with the distlib-launchers branch I've just started?

In keeping with the design principles for Pynsist, I've aimed to prepare as much as possible at build time, so all that needs to happen at install time is generating the shebang and concatenating the pieces together.

@vsajip Just to check, simple_launcher doesn't have any way of using a shebang relative to the launcher exe location? I can't see any way to do that, but it would be convenient if it did, because we could prepare a relative shebang at build time.

adferrand commented 5 years ago

Something like #!..\Python\python.exe ?

takluyver commented 5 years ago

Yup, that's what I'm thinking. I'm pretty sure it's not supported, but I thought I'd ask Vinay, because it would be super helpful if it worked. :-)

adferrand commented 5 years ago

@takluyver I test your branch on my fork right away

pfmoore commented 5 years ago

Yup, that's what I'm thinking. I'm pretty sure it's not supported, but I thought I'd ask Vinay, because it would be super helpful if it worked. :-)

Oddly, that's something I was looking at today, in a completely different context. I don't think it's supported yet, but it would be useful for me, too. I'd be willing to work up a PR to add it, if @vsajip is interested...

adferrand commented 5 years ago

Ok @takluyver, I tested your branch and launched the installer on the installation path C:\Certbot is my friend ¤ ^^, great path with spaces and non-ASCII characters, and ... it is working ! 🥇 (and I confirm with a reproductible pattern that is was not the case with the old wrapper).

I think your implementation is a good tradeoff to resolve the shebank at "runtime" without the need to install distlib in the embedded python distribution. It tested with a hard coded shebang #!..\Python\python.exe. Indeed @pfmoore, it fails, but not by far: it is just that the wrapper executes the shebang provided untouched, so it works only if you run it from the bin directory.

Certainly the wrapper only needs a routine to detect that the shebang is relative, and append its own current location path to it.

takluyver commented 5 years ago

OK, great. I think that's the way to go for the moment, then; I'll merge it when I get a bit more time. If the 'relative shebang' idea goes ahead, we can simplify it further.

I may also look at deleting the .zip files after the launchers have been assembled at install time.

For future reference: I investigated assembling the exe launchers purely with NSIS, avoiding the need for a Python script running at install time. I decided that the functionality for manipulating binary files and encoding text in NSIS were too clumsy and badly documented. I'm sure it's possible to make it work, but it was taking a lot of effort to figure out the details, and I wouldn't have much confidence in the resulting code. In contrast, the necessary operations in Python are totally straightforward.

vsajip commented 5 years ago

@pfmoore What's your use case for a relative interpreter location? The one for Pynsist is a very niche use case, ISTM, but I have no problem in principle with supporting relative paths in shebangs in simple_launcher. I need to think about any downside ... one problem with relative shebangs is that after moving foo.exe to a new location, it suddenly stops working ... not a great user experience.

vsajip commented 5 years ago

Am I right in thinking that if a user installs two different applications with Pynsist, they would get two separate Python installations, one for each application?

pfmoore commented 5 years ago

@vsajip I'm writing a tool that bundles Python code into executables - something like a combination of zipapp and pipsi. One case I want to support is having a relocatable directory structure, something like

.local
+---- bin
      + - app1.exe
      + - app2.exe
+---- runtimes
      + - app1
      + - app2

The exes would locate their appropriate runtime (which might be a virtual environment, or an embedded Python distribution) relative to the location of the exe.

This is possible at the moment, but the resulting structure isn't portable because the exe wrappers contain absolute paths. It's not the end of the world (one of the points of my tool is to make rebuilding the structure simple to do) but it would be useful (e.g., for use on removable drives).

It's still a niche usage - and I haven't decided for certain to use the existing wrappers (for most of my use cases, an embedded exe calling Py_Main is probably sufficient) but using existing code has its attractions :smile:

adferrand commented 5 years ago

Yes @vsajip, you are right, there is one independant python distribution for each installation, accessible by a predictible relative path from the relevant installation directory.

About the user experience to use an invalid path, for pynsist, if it happens, it means that a user messed up the installation directory constructed by NSIS... Well he knew the risk :). But at least your executable provides some useful messages when something goes wrong, it helps a lot to debug.

adferrand commented 5 years ago

The use case of @pfmoore is really similar to the situation with pynsist.

takluyver commented 5 years ago

In terms of moving things around: absolute paths are good if you want to move the launcher exe but not the Python interpreter. Relative paths are useful if you want to move the directory containing both of them, which is essentially what Pynsist does.

I wouldn't quite call it a niche use case - I think it's a pretty general pattern for an application with a bundled copy of Python. But I totally understand if you'd rather not support it: it's in everyone's interests for the launcher to be as simple as possible, and I think we have a satisfactory solution for Pynsist.

Speaking of simplicity, is it worth dropping the APPENDED_ARCHIVE compile option and making that the only behaviour? You indicated above that the alternative mode hasn't been maintained or tested for some time. I was reading the launcher code to better understand it, and I find that every #ifdef takes an extra little bit of mental effort.

vsajip commented 5 years ago

But I totally understand if you'd rather not support it: it's in everyone's interests for the launcher to be as simple as possible, and I think we have a satisfactory solution for Pynsist.

As I mentioned above, I'm not against the idea in principle and if @pfmoore works up a patch like he said he might, I'll certainly look at it seriously. (Or a patch from someone else for the same functionality, of course).

I had a quick look and I think the functionality for handling relative paths could be localised to just one extra function and one call to it, so it may not too bad in terms of increased complexity.

Speaking of simplicity, is it worth dropping the APPENDED_ARCHIVE compile option and making that the only behaviour?

Perhaps. The launchers are pretty stable and they don't need much maintenance, so it's not a big problem.

adferrand commented 5 years ago

Hello @takluyver, did you have some time do finish the next release with the new wrappers? Your implementation works very well!

takluyver commented 5 years ago

Thanks for the reminder, it had kind of slipped my mind as I worked on other things. I've opened PR #169 with those changes; I'll leave that for a day or two for anyone who wants to review the changes, then I'll merge and cut another release with it.

vsajip commented 5 years ago

I just checked, and it looks as if relative shebang paths in the distlib launchers work already (without me doing anything :smiley: ) , at least in some cases. See this simple_launcher issue. You are welcome to post any problems you find to that issue, and I will look at them.

adferrand commented 5 years ago

If it is working out of the box, it means certainly that relative paths are resolved against current working directory, as this kind of path expansion will occur naturally in any execution inside a shell.

But here, it is about resolving a relative path against the actual location of the wrapper, and with a shell of without it. This would require specific code embedded in the wrapper runtime.