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

Overwriting sys.argv[0] leaves no remaining methods to identify the actual sys.argv[0] #2006

Closed
jdm-twtr opened this issue Dec 8, 2022 · 11 comments · Fixed by #2007
Closed
Assignees
Labels

Comments

@jdm-twtr
Copy link

jdm-twtr commented Dec 8, 2022

As of the 2.1.91 release, sys.argv[0] is no longer a reliable option for identifying the executed script as called.

For example, creating a symlink to a pex, then calling that symlink yields a sys.argv[0] that does not point to the symlink. The PEX environment variable also does not point to the symlink.

Are there any known remaining ways to obtain argv[0] as provided by the calling binary?

@jsirois jsirois added the bug label Dec 8, 2022
@jsirois
Copy link
Member

jsirois commented Dec 8, 2022

There is probably not. Let me dig up why that changed. My policy is to not break folks until the 3.x transition so this is a bug unless the change was intentional and fixed something else and that something else trumps this after debate.

You'll make my day if this broke your conscript BusyBox.

@jsirois
Copy link
Member

jsirois commented Dec 8, 2022

Yeah. so #1785 was 1st released in 2.1.91; so that's likely the root cause of the regression. I'll see what I can do today and get out a ... pretty luke warm hotfix at this point if possible.

@jsirois jsirois self-assigned this Dec 8, 2022
@jsirois
Copy link
Member

jsirois commented Dec 8, 2022

Well, that was too easy. Removing the os.realpath added to the generated __main__.py by pex_builder left all unit tests and ITs passing. I think that means my old enemy macOS is involved, we'll see in CI. Its probably the @#$! weird stuff they do with the user tmp dir here in which case I'll need to fix the test instead of the production code - which was hacky if that's what I did.

jsirois added a commit to jsirois/pex that referenced this issue Dec 8, 2022
@jsirois
Copy link
Member

jsirois commented Dec 8, 2022

@jamesmeador it would be great if you can chime in on #2007. Namely what is your use case and does this fix it? I assume a conscript / BusyBox use case and I fix that, but it would be helpful to know exactly what you were previously doing successfully with PEX using sys.argv[0] that was broken in 2.1.91 to make sure I fix the correct thing.

@jsirois
Copy link
Member

jsirois commented Dec 9, 2022

@jamesmeador I'm going ahead with this fix since it definitely fixes the broken BusyBox case. If this doesn't solve your git hook case - please report back with details.

@jsirois
Copy link
Member

jsirois commented Dec 9, 2022

Alright @jamesmeador a fresh cut with this fix is out in 2.1.118: https://pypi.org/project/pex/2.1.118/.

@jdm-twtr
Copy link
Author

@jsirois if only I had been able to test this sufficiently for you... maybe the issue is still on my end. I'm still seeing this, and I made a test pex to illuminate the issue:

test.py consists of:

import sys

if __name__ == '__main__':
  print(sys.argv[0])

Executing the pex:

$ dist/test.pex
/Users/jmeador/.pex/unzipped_pexes/55019cf537ede1f571e75182536f8b617127c2b0/twitter/git_review/bin/test.py

In theory, this should have output dist/test.pex.

Here's some more info that may shed light on something else that could be going wrong?

$ unzip -p dist/test.pex PEX-INFO | jq
{
  "bootstrap_hash": "738140a8be7a33833bd4a5cc3748af7a3aee1dfe",
  "build_properties": {
    "pex_version": "2.1.121"
  },
  "code_hash": "80ae6d781659617bbbc97ac5b79d1c940bef0be0",
  "distributions": {},
  "emit_warnings": false,
  "entry_point": "twitter.git_review.bin.test",
  "inherit_path": "false",
  "interpreter_constraints": [
    "CPython<3.8,>=3.7"
  ],
  "pex_hash": "55019cf537ede1f571e75182536f8b617127c2b0",
  "pex_root": "~/.pex",
  "requirements": [],
  "strip_pex_env": true
}

@jsirois
Copy link
Member

jsirois commented Jan 23, 2023

@jamesmeador I'm AFK, but can you try a function entry point instead and see what happens?

@jdm-twtr
Copy link
Author

That works! Thanks for the reply while AFK.

@jsirois
Copy link
Member

jsirois commented Jan 24, 2023

Ok. I'll take a look at your case, which is the runpy.run_module case. I suspect that may be a sticky case. The PEX invocation uses alter_sys=True, and, although my memory is foggy, I think this solves some other bug but I expect it leads to this bug :/

@jsirois
Copy link
Member

jsirois commented Jan 25, 2023

Yeah, so its either make -m work to preserve symlinks in sys.argv[0] or make -m work with pickle: #1018

@jamesmeador as far as I can tell there is simply no way to make -m satisfy both pickle and the preservation of symlinks case. Are you OK with the function entrypoint workaround?

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

Successfully merging a pull request may close this issue.

2 participants