multipackage (MERGE) boken in PyInstaller 3.0 #1527

Open
matysek opened this Issue Sep 25, 2015 · 16 comments

Projects

None yet

9 participants

@matysek
Member
matysek commented Sep 25, 2015

In PyInstaller 3.0 no much attention was paid to 'multipackage' feature and it is broken:

  • MERGE should be fixed
  • MERGE tests should be migrated to pytest
@matysek matysek added this to the PyInstaller 3.1 milestone Sep 25, 2015
@htgoebel
Member

Maybe a chance to rework this. PyInstaller supports loading modules from a external file, see old_suite/basic/test_pyz_as_external_file.spec and bootloader/main. We could change MERGE into using this and dramatically simplify the code.

Edit: This test-case has now been moved to test_pyz_as_external_file in functional/test_basic.py: py.test -k test_pyz_as_external_file.

@kalikaneko

I'm interested in this feature, and I might be able to help with fixing it. I did a very simple test (just an import and a print), and it seems not to be duplicating the libs, so I don't see exactly where it could be broken.

samsara 戝 ~/tmp/pyinst-MERGE/dist 
10712 ◯ : tree                                                                                                                                                                      
.
├── bar
│   └── bar
├── baz
│   └── baz
└── foo
    ├── bar
    ├── baz
    ├── bz2.x86_64-linux-gnu.so
    ├── _codecs_cn.x86_64-linux-gnu.so
    ├── _codecs_hk.x86_64-linux-gnu.so
    ├── _codecs_iso2022.x86_64-linux-gnu.so
    ├── _codecs_jp.x86_64-linux-gnu.so
    ├── _codecs_kr.x86_64-linux-gnu.so
    ├── _codecs_tw.x86_64-linux-gnu.so
    ├── _ctypes.x86_64-linux-gnu.so
    ├── foo
    ├── _hashlib.x86_64-linux-gnu.so
    ├── _json.x86_64-linux-gnu.so
    ├── libbz2.so.1.0
    ├── libcrypto.so.1.0.2
    ├── libexpat.so.1
    ├── libffi.so.6
    ├── libpython2.7.so.1.0
    ├── libreadline.so.6
    ├── libssl.so.1.0.2
    ├── libtinfo.so.5
    ├── libz.so.1
    ├── _multibytecodec.x86_64-linux-gnu.so
    ├── pyexpat.x86_64-linux-gnu.so
    ├── readline.x86_64-linux-gnu.so
    ├── resource.x86_64-linux-gnu.so
    ├── _ssl.x86_64-linux-gnu.so
    ├── termios.x86_64-linux-gnu.so
    ├── twisted.python._sendmsg.x86_64-linux-gnu.so
    └── zope.interface._zope_interface_coptimizations.x86_64-linux-gnu.so

3 directories, 32 files
(env2) 

(using d9be7da due to #1863 affecting me at least on my local enviroment)

@htgoebel could you please elaborate a bit more on the needed rewrite when you have some time?

@htgoebel
Member

@kalikaneko Well, the tests (which are still in the old test-suite only) all fail, Try:

cd tests/old_suite/
./runtests.py multipackage/test_*.py

My idea for rewriting it is: Instead of appending the PKG to one of the executables, create an external file pkg which is used. The bootloader already contains code for this. But now as I'm checking the code, I'm no longer convinced that this will be a notable improvement.

@micahflee micahflee referenced this issue in micahflee/onionshare Apr 12, 2016
Closed

Support command line version in OSX and Windows #201

@miphip
miphip commented Aug 25, 2016

Selected PyInstaller based on this feature. Disappointed to see it not working. What to do?

@bluebad
bluebad commented Nov 24, 2016 edited

Not working still?
Now my app consists of two simple executables that build with pyinstaller so horrible large.

@letlet
letlet commented Dec 20, 2016

Please add this to your todo list.

@MerlijnWajer
Contributor
MerlijnWajer commented Jan 8, 2017 edited

I just wanted to say that this works for me, given a small hack.

For some time I was trying to fork+exec my own application, to launch some workers due to inherent issues with multiprocessing on OS X (not pyinstaller related). To prevent unpacking overhead, I would do something like this::

def pyinstaller_env():
    """
    Returns None if not in pyinstaller. (None means: Keep current environment,
    at least to Popen)

    Otherwise returns the current environment, plus the _MEIPASS2 environment
    variable, provided to ensure Pyinstaller doesn't unpack itself again when
    we exec() it.

    For multipackage bundles, this is required for the multipackage bundles to
    function at all.
    """
    if hasattr(sys, 'frozen'):
        env = os.environ
        # If Pyinstaller encounters _MEIPASS2 in the env, it will not unpack
        # and instead use that directory for the bootloader. By using this we
        # avoid unpacking again for every fork+exec.
        env['_MEIPASS2'] = sys._MEIPASS

        return env

    return


def fork_self(args):
    """ Fork self. Possibly using a different entry point (args)
    """
    print('fork_self:', args)
    env = pyinstaller_env()

    main = subprocess.Popen([sys.argv[0]] + args, env=env)

    return main

However, the second (smaller) application does not start, with an error like this:
Error loading Python lib '/var/folders/sy/ky6wtvhs3yx_b1856h3g8w6r0000gn/T/_MEIk5dZOT/.Python': dlopen(/var/folders/sy/ky6wtvhs3yx_b1856h3g8w6r0000gn/T/_MEIk5dZOT/.Python, 10): image not found

But, using my pyinstaller_env() function when spawning the second application does work.

Maybe this is useful for others - if you fork+exec to start one of your other MERGE'd + BUNDLE'd application, just pass _MEIPASS2 (with value set to the current _MEIPASS)

(Background: my fork+exec worker combo worked fine, except that it would spawn lots of dock icons, due to the main application being a windowed application, and pyinstaller setting up every time. That's why I created a second binary, which is otherwise identical, except that it has console=True, instead of console=False.)

@tallforasmurf
Contributor

Would this be suitable as a Wiki recipe?

I don't get any hits on a search in the repository for _MEIPASS2 so I wonder where in the bootloader this is tested?

@MerlijnWajer
Contributor

_MEIPASS2 is mentioned here - https://github.com/pyinstaller/pyinstaller/wiki/Recipe-Multiprocessing

I didn't find it in the documentation either, but that recipe made me try the same on OS X/Linux, and it also seems to work there.

_MEIPASS2 is found here: bootloader/src/pyi_main.c

I don't know if this is the right 'solution', so I don't think I am qualified to comment on whether this counts as a recipe or not.

@tallforasmurf
Contributor

Yeah, it's confusing. I'm confused because github search still doesn't turn up that pyi_main.c hit in the 41 results for '_MEIPASS2'. But you have fingered the place it matters, here. The bootloader starts, fetches that envar, and at line 122 makes a critical decision.

If _MEIPASS2 was not defined, this is the original bootloader thread and it starts the process of creating the temp folder and unpacking into it. When that completes it will fork itself, set _MEIPASS2 and we come back to line 122 now in a second thread. Because the envar is set, it knows this is the second instance, unpacking is complete, and it can proceed to kick off the unpacked user program.

So by pre-setting the envar and then launching the same executable in a subprocess, you are preventing the bootloader at the head of that executable from going through the unpacking-and-forking business. It just proceeds into the user code in its subprocess thread.

If you made a subprocess out of a different executable it would be a mistake to set that envar because that different executable would not unpack itself, would try to import modules from the parent program's temp folder and would likely fail with an import error.

The multiprocessing recipe you point to seems to be doing this same thing. (The same code appears in this functional test and this one.

However it looks odd to me for a couple of reasons, one is this:

if sys.platform.startswith('win'):
...
            finally:
...
                    # On some platforms (e.g. AIX) 'os.unsetenv()' is not
                    # available. 

Since the code is restricted to Windows, why is it nattering about AIX?

@MerlijnWajer
Contributor

Yes, that is indeed what _MEIPASS2 does. However - your note on 'different executable' is not entirely correct. In this case (MERGE target), only one of the binaries (first one passed to MERGE) contains all of the dependencies. If that one unpacks first, and then the others are started with that _MEIPASS2, it just seems to work.

The multiprocessing recipe mentions Windows specifically because multiprocessing on Windows, using python2, can't do fork+exec (because it is Windows) and you need the hacks that they mention in the recipe. Why they mention AIX is beyond me. It may just be general coding practice / a habbit regarding os.unsetenv. Multiprocessing on python3 will likely require similar hacks/recipes for all platforms.

@xoviat
Contributor
xoviat commented Jan 9, 2017 edited

@MerlijnWajer Some tests fail unexpectedly at the moment because the loader doesn't setup the environment correctly for the parent thread sometimes. You can see a test failure here, but they have also occurred on linux. Can you guess what might be causing this?

@MerlijnWajer
Contributor
MerlijnWajer commented Jan 10, 2017 edited

Are you sure that this is related to the MERGE target? I haven't looked at pyinstaller tests before, so I don't know offhand what is going wrong there. It's also Windows, which I don't usually test pyinstaller on.

@htgoebel - any comments on my previous notes on this? Could this be related to the broken tests? I guess I should perhaps dive into the bootloader code and see if this can be fixed/changed, but I don't know what the intended behaviour is.

@xoviat
Contributor
xoviat commented Jan 11, 2017
@MerlijnWajer
Contributor

@xoviat: Just to check, I don't see how this relates to the MERGE issue?

@xoviat
Contributor
xoviat commented Jan 11, 2017

I don't think it does now. Originally I thought that it was related to the bootloader, but @htgoebel has come up with a more plausible explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment