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

Isolate apps from system site packages #65

Open
ipmb opened this issue Sep 22, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@ipmb
Copy link
Contributor

commented Sep 22, 2018

Shiv includes system site packages in the Python path. Roughly the equivalent of virtualenv's --system-site-packages. In general (or at least for my uses) this is not desirable. Would you consider changing it to not include other site-packages or adding a flag to toggle the behavior at build time?

@lorencarvalho

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

Hm, given that we ensure that our site-packages comes first there shouldn't be any benefit to removing the system site-packages (at least, nothing that immediately comes to mind). Any duplicates will favor the first site-packages dir found on the path (which is the one placed by our bootstrapping code).

Can you describe specific issues you've encountered that this problem would fix? Just want to make sure I fully understand :)

@ipmb

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2018

The specific issue I saw would be resolved by #64, but here's a couple scenarios where you could get bitten by this:

It's not uncommon to see optional imports in Python projects:

try:
    import thing
except ImportError:
    # do something different here

You could unexpectedly pull in a system package this way.

Another variation of this is a version mismatch where imports have changed. Say v1 of package had package.old_module and now you've upgraded to v2 and you should be using package.new_module instead. Well, if you have v1 in your system and a bug in your code that still imports old_module, it's going to do some weird stuff.

They are edge cases, but they happen. I think most people would expect/prefer isolation as the default. And if you don't want isolation, you can flip the switch for it.

@lorencarvalho

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

@ipmb gotya, thanks for the added clarity,

I am personally satisfied with what #64 solves. I'm wary to get shiv into the business of handling too much in this space.

Ultimately I think the sort of interpreter-level isolation you are talking about is better handled by system automation (for example, at LinkedIn we compile & distribute our python interpreters via automation and ensure nothing is installed into their site-packages).

All that said, happy to be convinced otherwise :)

@ipmb

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2018

I hear where you're coming from, that's a fair stance on the issue. If we changed the default so it retained the current functionality, would you consider adding it behind a flag?

I'm running a much smaller shop than LinkedIn and compiling/distributing our own Python distributions is overkill for our needs. Generally I have an Ubuntu system running SaltStack at the system level and I'd like to run a bunch of isolated zipapps as well. Installing SaltStack from their repo is quick and convenient, but pollutes the global namespace. There may be other system tools that use Python too, so I can't guarantee a perfectly clean system Python.

There are a number of ways around this issue, but since shiv was already asserting what is/is not a site package and the meat of this change is effectively a one-liner, it seemed like the easiest place to ensure the zipapps are isolated while still maintaining a mostly "stock" system otherwise.

@anthrotype

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

I would also like to use an —no-system-site-packages option for the same reasons that @ipmb explained.

@lorencarvalho

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

If we changed the default so it retained the current functionality, would you consider adding it behind a flag?

Eh, I'm just hesitant to make such a fundamental change in behavior - but, at the same time I recognize that I'm just not feeling the pain due to the environment I work in.

It sounds like what we should really be emulating here is what pex does with it's --inherit-path argument. I'll try and get a review out soon

@ipmb

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2019

Documenting for anybody who comes across this issue. @lorencarvalho tipped me off to python -S today.

-S : don't imply 'import site' on initialization

So you can either run your zipapp with python -S app.pyz or add it to your shebang during shiv creation with --python. There is one caveat, however. #!/usr/bin/env python3.7 -S will not work due to an idiosyncrasy with how the shebang arguments are processed. The following are valid alternatives:

  1. Use a direct path to Python: #!/usr/local/bin/python3.7 -S
  2. If you have a very recent version of coreutils (8.30+), use the -S flag for env
    #!/usr/bin/env -S python3.7 -S

The shebang parsing is an annoyance, but this generally solves the issue for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.