Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command line launchers based on distlib #169

Merged
merged 2 commits into from Oct 29, 2018
Merged

Command line launchers based on distlib #169

merged 2 commits into from Oct 29, 2018

Conversation

@takluyver
Copy link
Owner

@takluyver takluyver commented Oct 24, 2018

Replacing those based on exes extracted from setuptools. The distlib launchers are the ones pip uses when installing scripts on Windows, and they are apparently more resilient against spaces in the path.

Closes gh-166.

@takluyver takluyver merged commit 0156bca into master Oct 29, 2018
4 checks passed
4 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@takluyver takluyver deleted the distlib-launchers branch Oct 29, 2018
@adferrand
Copy link
Contributor

@adferrand adferrand commented Nov 6, 2018

Hello, just a reminder about the fact that you did not released yet a new version of pynsist with the new wrappers.

Regards,
Adrien Ferrand

@takluyver
Copy link
Owner Author

@takluyver takluyver commented Nov 6, 2018

Thanks for the reminder, I've just released 2.3. There's another change that should fix installing scripts without admin access (#170).

@bastimeyer
Copy link
Contributor

@bastimeyer bastimeyer commented Feb 11, 2019

@takluyver
Is there a reason for also removing the regular entry scripts? This change is breaking my application and I would really appreciate it if this could be partially reverted, so that both an executable and an entry script are built by pynsist.

See my comment here why this is important:
streamlink/streamlink#2287 (comment)
This linked issue was initially about installations of Streamlink via pip, as pip also only creates an executable and no entry scripts, unless --no-binary is set, so just ignore the earlier comments in there.

However, due to this change in pynsist, the Streamlink Windows installer is also affected by this (built via pynsist), and since it's a more popular way for Windows users to install Streamlink like this than using pip, this change now affects all of the Windows users without having a possible workaround.

The reason why this is a breaking change is how the python executables work on Windows. The built wrapper executables are always calling python.exe instead of pythonw.exe, which makes it impossible to use from a non-terminal context, like for example when executing them from a GUI application, because a terminal window will always be opened.

The only way to suppress this behavior is executing pythonw.exe directly and using the path to the wrapper executable as the first parameter, just like you could do with the old entry scripts. Unfortunately though, now you have to know the path to the included pythonw.exe beforehand, because now there's no shebang to parse. You could do that with the old entry scripts very easily. Now you either have to guess the path from the file/directory structure the installer generates and hope that it'll never change in the future, or you have to somehow unpack the wrapper executable and get the shebang from its contents in order to find the path to pythonw.exe, which is a bit silly.

@takluyver
Copy link
Owner Author

@takluyver takluyver commented Feb 12, 2019

Hi @bastimeyer . Sorry for breaking your application, but I consider the *-script.py files to have been an implementation detail of the old exe launchers, and I don't propose to bring them back. It sounds like you're also having the same problem with installations from wheels.

If streamlink-twitch-gui doesn't mind having logic to handle pip installs and Pynsist installs separately, there are a number of possible approaches you could take.

  • For Pynsist installers, I see you already use a heavily customised NSI template. You could add some instructions to write the necessary information to a file - whether that's a Python script to be run, or a JSON file that STG can parse to find the necessary paths.
  • For pip, you could have a gui_scripts entry point with a different name in addition to your console_scripts entry point. gui_scripts entry points get a GUI-mode wrapper executable, so it doesn't bring up a console window. Or you could install a script the old distutils way - pip doesn't create exe wrappers for these, and it doesn't look like that's going to change any time soon.

The new exes do also include a plain-text shebang, if STG wants to find and read that. It has to be fairly straightforward to do, because the exes themselves find it with code written in C. However, that mechanism might change again in the future, so be prepared to have to update it if you go down that route.

@bastimeyer
Copy link
Contributor

@bastimeyer bastimeyer commented Feb 13, 2019

For pip, you could have a gui_scripts entry point

Thanks for the hint! Sounds like the commands would use the pythonw executable then, right? That would be execatly what I'm looking for.

I don't propose to bring them back

Hm, that's very unfortunate. With this removal you're basically asking every application that's packaged with pynsist to include their own stuff in case other GUI applications depend on it, and those have to implement this custom stuff.

The reason why this change is so bad for my application is that prior to it, you could simply run which application-script.py (since the installer's /bin dir gets added to the PATH env var) and then parse the script's shebang for getting the path to the included pythonw.exe without having to know any specific file/directory structures or custom resolver instructions. The same method also worked for installations via pip (--no-binary) and of course also on Linux and macOS, regardless the installation method.

So wouldn't it make sense creating two binaries for each command in pynsist, similar to console_scripts and gui_scripts, if that's possible? Or giving us an option to specify the type of wrapper executable, so those can be listed individually in the pynsist options? Then the whole pythonw.exe shenanigans, shebang parsing and custom resolver instructions wouldn't be necessary and the (GUI) wrapper could be executed directly.

Thanks!

@takluyver
Copy link
Owner Author

@takluyver takluyver commented Feb 13, 2019

Thanks for the hint! Sounds like the [gui_scripts] commands would use the pythonw executable then, right?

I'm not exactly sure of the implementation details, but they either use that or do something that has the same effect.

Or giving us an option to specify the type of wrapper executable, so those can be listed individually in the pynsist options?

I guess it wouldn't hurt for commands to have a console=false option like shortcuts, but defaulting to true. Do you want to make a pull request?

@bastimeyer
Copy link
Contributor

@bastimeyer bastimeyer commented Feb 17, 2019

So I've tested the executables created with the gui_scripts entry scripts in setuptools and as it turns out, they are completely useless for my application, as their own processes terminate immediately.
https://github.com/pypa/setuptools/blob/v40.8.0/launcher.c#L315-L325
These launchers are not meant for being executed from another application which needs to keep them running while reading their redirected stdout/stderr streams. That's really disappointing.

Do the simple_launcher executables do the same or do they behave like what I am looking for? Unfortunately, I don't know anything about any Windows APIs.


If these console=false simple_launcher executables are not that what I am looking for, the only remaining options for solving this issue in my app, and any kind of similar application which is a non-python front end for a python application (on Windows), are:

  • Implementing a custom pythonw.exe resolver strategy for installs of pynsist-made installers, based on their (hopefully stable) directory structure. Remember, the python executables are not in the PATH here.
  • Customizing the pynsist config so that resolver instructions are included which the front end can then attempt to read (no directory guessing needed).
  • Reading the launcher binaries and finding+parsing the custom shebang logic, which isn't that trivial if done right (parsing a file read stream instead of regex-ing the entire file content in-memory).

All that because of the removal of regular python entry script text files. 😩

Do you want to make a pull request?

I'm sorry, but I have no idea where to add this. I also don't have the time for tinkering with that either.

@takluyver
Copy link
Owner Author

@takluyver takluyver commented Feb 17, 2019

Do the simple_launcher executables do the same or do they behave like what I am looking for?

It looks like they always wait for the child process and then exit with its exit code:

    WaitForSingleObject(pi.hProcess, INFINITE);
    ok = GetExitCodeProcess(pi.hProcess, &rc);

But the only way to be sure is to test. If you create a wheel with gui_scripts and then install it using pip, it will create launchers based on simple_launcher.

I'm sorry, but I have no idea where to add this.

The definition of the command sections in the config file is here:

'Command': SectionValidator([
('entry_point', True),
('extra_preamble', False),
])

This code copies the launcher exe into the build directory where it will be bundled into the installer:

def find_exe(bitness=32):
distlib_dir = osp.dirname(distlib.scripts.__file__)
return osp.join(distlib_dir, 't%d.exe' % bitness)
def prepare_bin_directory(target, commands, bitness=32):
# Give the base launcher a .dat extension so it doesn't show up as an
# executable command itself. During the installation it will be copied to
# each launcher name, and the necessary data appended to it.
shutil.copy(find_exe(bitness), str(target / 'launcher_exe.dat'))

And finally this small file runs at install time to assemble the exes with the shebang. I'm hoping a future version of the launchers will support a relative path to Python, so we can do the assembly at build time:

https://github.com/takluyver/pynsist/blob/ca4c2cea9c0c1eba277d9cd79f30eb61ac0951de/nsist/_assemble_launchers.py

I'm not saying this because you have to make a PR yourself. I might get round to it. But I want to make it clear that:

  1. It's not magic. Pynsist does a bunch of familiar operations like copying files around, filling out a template and running a subprocess. I hope that people other than me can understand it and modify it.
  2. It's purely voluntary open source. I like tinkering with it sometimes, and I'm happy that people find it useful, but I don't feel any obligation to implement specific features, and I don't feel particularly bad if a change broke what you were doing. You're maintaining a much more popular open source project already, so you know how this goes. 🙂
@bastimeyer
Copy link
Contributor

@bastimeyer bastimeyer commented Feb 17, 2019

If you create a wheel with gui_scripts and then install it using pip, it will create launchers based on simple_launcher.

Ok, tested it and it works. That's fantastic and should solve all of my issues.

so you know how this goes

:)

I will see if I can assemble a pull request in the next couple of days when I get the time. Thanks for your help so far and sorry for spamming this thread. 😅

@takluyver
Copy link
Owner Author

@takluyver takluyver commented Feb 17, 2019

Thanks! I don't think it was spam at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants