JonathonReinhart / staticx

Create static executable from dynamic executable
https://staticx.readthedocs.io/
Other
345 stars 37 forks source link

Add an option to pass the original environment (ED: or just the current LD_LIBRARY_PATH ) to the ldd subprocess #184

Closed m0nzderr closed 3 years ago

m0nzderr commented 3 years ago

In my docker build scenario LD_LIBRARY_PATH is being changed just in the shell environment in order to include some additional libraries. My executable works and ldd output looks just fine from the shell.

However, when running the staticx from the same shell it fails with the error like staticx: Unexpected line in ldd output: myLibrary.so => not found. This is clearly due to env={} (elf.py:142) instead of using env=dict(os.environ) as suggested in the comments:

    # TODO: Should we use dict(os.environ) instead?
    #       For now, make sure we always pass a clean environment.
    env = {}

    if libpath:
        # Prepend to LD_LIBRARY_PATH
        assert isinstance(libpath, list)
        old_libpath = env.get('LD_LIBRARY_PATH', '')
        env['LD_LIBRARY_PATH'] = ':'.join(libpath + [old_libpath])

    rc, output = tool_ldd.run(path, env=env)

Should we use dict(os.environ) instead?

-My answer is "yes, perhaps if user wants it". It would be nice to have an option like "--inherit-env" or "--clear-env" to switch between these different behaviors.

Could you please consider adding this feature? Let me know if you prefer a PR.

m0nzderr commented 3 years ago

UPD: A moment later I've noticed that LD_LIBRARY_PATH was already supposed to be preserved in case of non-empty libpath argument. Why not just keep the original os.environ['LD_LIBRARY_PATH'] always? I.e., something like that should be fine:

    env = {'LD_LIBRARY_PATH':os.environ['LD_LIBRARY_PATH']}
    if libpath:
        # Prepend to LD_LIBRARY_PATH
    ...

Is there any reason for not doing so?

JonathonReinhart commented 3 years ago

Hi @m0nzderr. You're right, the current code (added in 9cabf0045b0bb7761d87d31ae4343264fe854255) makes no sense. Why would I bother to call env.get('LD_LIBRARY_PATH') if env is always an empty dict?

I opened PR #185 to always path LD_LIBRARY_PATH like you suggest (but done a bit differently).

Please try that out and let me know! I can merge it and cut a release asap.

m0nzderr commented 3 years ago

Thanks @JonathonReinhart, I checked the branch 184-persist-LD_LIBRARY_PATH from PR #185 and it worked as expected. A new release would be greatly appreciated!

JonathonReinhart commented 3 years ago

When I merge #185, this issue will auto-close.

JonathonReinhart commented 3 years ago

@m0nzderr Staticx v0.12.3 is released. Enjoy! Thanks for pointing out this issue.