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

fix: RunCommand calling scripts with incorrect executable path #1001

Closed
wants to merge 4 commits into from

Conversation

chdsbd
Copy link
Contributor

@chdsbd chdsbd commented Mar 31, 2019

Fixes GH-965

Calling sys.argv should run the same program as the currently running program. To make calling Poetry scripts through RunCommand match this behavior, we must set sys.argv[0] to be the full path of the executable.

This change makes the behavior of calling a script through poetry run the same as calling a script directly from the .venv/bin.

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

  • Added tests for changed code.
  • Updated documentation for changed code.

Note: If your Pull Request introduces a new feature or changes the current behavior, it should be based
on the develop branch. If it's a bug fix or only a documentation update, it should be based on the master branch.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Fixes python-poetryGH-965

Calling `sys.argv` should run the same program as the currently
running program. To make calling Poetry scripts through RunCommand
match this behavior, we must set `sys.argv[0]` to be the full path of
the executable.

This change makes the behavior of calling a script through `poetry run`
the same as calling a script directly from the .venv/bin.
@chdsbd chdsbd changed the title WIP: fix: RunCommand calling scripts with incorrect executable path fix: RunCommand calling scripts with incorrect executable path Mar 31, 2019
There are probably cleaner ways to test the CLI, so let me know.
@chdsbd
Copy link
Contributor Author

chdsbd commented Mar 31, 2019

My test is hacky. Any tips on cleaning it up would be appreciated.

@chdsbd
Copy link
Contributor Author

chdsbd commented Mar 31, 2019

@sdispater This is ready for review

@brycedrennan brycedrennan added the kind/bug Something isn't working as expected label Aug 17, 2019
@stale
Copy link

stale bot commented Nov 13, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 13, 2019
@chdsbd
Copy link
Contributor Author

chdsbd commented Nov 13, 2019

This is still something I'd like to fix in poetry. I'll fix the conflicts when I get a commitment that this will be merged.

@stale stale bot removed the stale label Nov 13, 2019
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your contribution! 👍

Could you please rebase on the current master branch?

Comment on lines +27 to +28
full_path_args = list(args)
full_path_args[0] = self.env._bin(full_path_args[0])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we simplify it with

args[0] = self.env._bin(args[0])

instead?

@arbaes
Copy link

arbaes commented May 12, 2022

Hello !

I just ran into this issue, is there still no plan to merge this ?

@Secrus
Copy link
Member

Secrus commented May 21, 2022

@chdsbd are you still interested in bringing this to Poetry? if so, please merge the master branch to your code and resolve potential issues. This would allow for rereview.

@qexat
Copy link

qexat commented Aug 24, 2022

Any updates? This would be very nice if this issue was fixed..

@wagnerluis1982
Copy link
Contributor

If desired, I can work on resolve the conflicts and address the issues.

@chdsbd are you okay if I rebase your work on another branch? (I will keep the authorship of your commits)

@chdsbd
Copy link
Contributor Author

chdsbd commented Oct 5, 2022

@wagnerluis1982 Thank you. That would be great!

@wagnerluis1982
Copy link
Contributor

@finswimmer could you close this PR in favor of #6737?

@radoering
Copy link
Member

Superseded by #6737

@radoering radoering closed this Jan 16, 2023
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RunCommand incorrectly sets sys.argv for Poetry scripts
8 participants