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

[1.1] fix: Broken cache on Windows #4549

Merged

Conversation

serverwentdown
Copy link
Contributor

@serverwentdown serverwentdown commented Sep 23, 2021

(Backport to 1.1) (Original PR: #4531)

The previous implementation would fail to install packages on Windows
because it creates a Path starting with a slash. Such a Path is
invalid on Windows. Instead, use the utility function url_to_path.

Pull Request Check List

Resolves: #4479 #4535

  • Added tests for changed code
  • Updated documentation for changed code. (N/A)

@serverwentdown serverwentdown force-pushed the 1.1-fix-cache-on-windows branch 2 times, most recently from f14ccf3 to 58375e2 Compare September 23, 2021 00:19
@serverwentdown serverwentdown changed the title fix: cache URLs on Windows [1.1] fix: cache URLs on Windows Sep 23, 2021
@GoodiesHQ
Copy link

I can at least confirm that this solves my issue. All I did was install poetry using install-poetry.py and manually change %APPDATA%\pypoetry\venv\Lib\site-packages\poetry\installation\executor.py to match the pull request. I am now able to successfully install packages on windows. It's bizarre, I would imagine this would be a rampant issue with more people on Windows...

@barskern
Copy link

Also having this issue. Hopefully it gets merged soon, seems like an uncontroversial change.

@serverwentdown
Copy link
Contributor Author

Ping @sdispater

@ditwrd
Copy link

ditwrd commented Sep 30, 2021

Can't wait for this merge, my current workaround is just to delete the cache folder and running poetry install again, well it work but doin it and waiting for the file to be deleted is a pain in the noggin :' @sdispater

@Greatness7
Copy link

Can confirm that the current release of poetry was totally broken on my windows system, and this PR fixed it. Seems like something that should be merged ASAP!

@finswimmer finswimmer requested a review from a team October 9, 2021 18:29
@serverwentdown serverwentdown changed the title [1.1] fix: cache URLs on Windows [1.1] fix: Broken cache on Windows Oct 15, 2021
@f0ff886f
Copy link

For tracking, this also closes #4163.

@Ivoz
Copy link

Ivoz commented Oct 20, 2021

Pasting this version of executor.py ontop of a fresh 1.1.11 installation made it work for me

@Luttik
Copy link

Luttik commented Oct 28, 2021

After these latest changes, I also tried replacing executor.py and everything worked fine for me too.

@geodesyMUC
Copy link

(Poetry version 1.1.11) Just wanted to add that I also experienced this issue after doing a mistake when adding and then removing a package. From then on I was unable to add any packages, resulting in the abovementioned "File does not exist" error due to the "/" character at the beginning of the path. This is on Windows 10.

Taking the executor.py from this pull request fixed my issue.

@MadJlzz
Copy link

MadJlzz commented Oct 28, 2021

Thanks a lot for the PR! I will check with the teams we have there as this brings a lot of pain to everyone!

@f0ff886f
Copy link

Thanks a lot for the PR! I will check with the teams we have there as this brings a lot of pain to everyone!

@MadJlzz Is there any way people outside of poetry core can help?

Poetry has been broken without manual intervention since June 9 (150+ days) for Windows users, is there anything we can do to get a fix rolled out?

@Luttik
Copy link

Luttik commented Nov 12, 2021

@f0ff886f I've tried to get a fork from poetry with @serverwentdown deployed but that turned out to be more than a 30-minute job 😔. I think that we need to at least get the get-poetry.py script to work with the fork for it to be usable as an alternative... I just don't have the time at this point to figure out everything that needs to change to get both the release workflow to work aswell as the get-poetry.py. It seems like we at least need an alternative to https://pypi.org/pypi/poetry/json but I've gotten no clue how that one is generated.

I did reserve poetry-windows-fix on pypi with the @serverwentdown 's fix online already. If anybody wants to continue the work I can transfer it or add you as a contibruter to both the github and pypi repo.

@serverwentdown
Copy link
Contributor Author

serverwentdown commented Nov 12, 2021

@Luttik if you'd like to install my branch of poetry, you can run the following pip command (note that this install method is not recommended by the documentation due to the potential for dependency conflicts, but if you don't have any other tools installed globally, this should be fine):

pip install git+https://github.com/serverwentdown/[email protected]
# Or, if you don't have Git installed:
pip install https://github.com/serverwentdown/poetry/archive/refs/heads/1.1-fix-cache-on-windows.zip

@neersighted
Copy link
Member

neersighted commented Nov 12, 2021

@Luttik if you'd like to install my branch of poetry, you can run the following pip command (note that this install method is not recommended by the documentation due to the potential for dependency conflicts, but if you don't have any other tools installed globally, this should be fine):

git+https://github.com/serverwentdown/[email protected]

You can also use install-poetry.py with the --git option to do it the recommended way e.g. curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/install-poetry.py | python - --git https://github.com/serverwentdown/[email protected]

@Luttik
Copy link

Luttik commented Nov 12, 2021

@serverwentdown I realise that this is possible (I assume that it is even better to do this with pipx). I am however very aware of my lack of awareness regarding the downsides and why the get-poetry script is what it is...

@neersighted that seems like an even better solution

Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

Looks good to merge once the PR is rebased, the format complies with our lints, and the checks pass.

@neersighted neersighted merged commit 3dcbb76 into python-poetry:1.1 Nov 12, 2021
@MadJlzz
Copy link

MadJlzz commented Nov 12, 2021

Thanks a lot for the PR! I will check with the teams we have there as this brings a lot of pain to everyone!

@MadJlzz Is there any way people outside of poetry core can help?

Poetry has been broken without manual intervention since June 9 (150+ days) for Windows users, is there anything we can do to get a fix rolled out?

Sorry for the misunderstanding ; I am not from the Poetry core team. I meant "check with the team at my company" to help them out. 😢

Thanks @neersighted for merging this out and @serverwentdown for the fix!

@Luttik
Copy link

Luttik commented Nov 18, 2021

FYI @neersighted the command should be without git+ it seems like the script prepends the url with git+ already.

so it bocomes: curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/install-poetry.py | python - --git https://github.com/serverwentdown/[email protected]

Could you edit your comment since I believe many use it as a reference.

@neersighted
Copy link
Member

FYI @neersighted the command should be without git+ it seems like the script prepends the url with git+ already.

so it bocomes: curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/install-poetry.py | python - --git https://github.com/serverwentdown/[email protected]

Could you edit your comment since I believe many use it as a reference.

My bad. Thanks!

@damarvin
Copy link

just to mention, I tried this curl+install as given, but still see the leading backslash → ValueError File \C:\Users...

@serverwentdown
Copy link
Contributor Author

serverwentdown commented Nov 25, 2021

@damarvin That's the command for Linux. For Windows:

(Invoke-WebRequest -Uri https://raw.githubusercontent.com/python-poetry/poetry/master/install-poetry.py -UseBasicParsing).Content | python - --git https://github.com/python-poetry/[email protected]

@damarvin
Copy link

Thanks for the hint! (but...)
I really prefer the cmd.exe over powershell.
What is wrong with curl? The given
curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/install-poetry.py | python - --git https://github.com/serverwentdown/[email protected] works for me, Your line (started in powershell) left me with
ERROR: Invalid requirement: 'git+git+https://github.com/python-poetry/[email protected]'

The point is, [email protected] seemingly does not fix the "leading backslash" issue.
I'm trying this in a venv, so sure it is newly installed (and for this issue I accustomed to clean the poetry caches).

@serverwentdown
Copy link
Contributor Author

serverwentdown commented Nov 26, 2021

@damarvin I believe the curl command wouldn't work on Windows. That's why you're not getting the updated version.

I have updated my comment to fix the PowerShell command.

@damarvin
Copy link

damarvin commented Nov 26, 2021

SIDE NOTE:
curl/wget is fine (like via cygwin), powershell is not the daily terminal for many developers under Windows—I stay with cmd.exe.
May I propose, you also provide a command line w/o PowerShell at https://python-poetry.org/docs/master/#installation?
And %APPDATA%\Python\Scripts should read %APPDATA%\Roaming\Python\Scripts.
And you should mention the parallel %APPDATA%\Roaming\pypoetry.
And you should mention that poetry.exe under former is only a symbolic link to latter.

poetry install from [email protected] tells now

[SolverProblemError]
Because <project> depends on pyproject-flake8 (*) which doesn't match any versions, version solving failed.

which used to work before. While going back to

pip install poetry

I see now

Attempting uninstall: poetry
  Found existing installation: poetry 1.1.11
  Uninstalling poetry-1.1.11:
    Successfully uninstalled poetry-1.1.11
ERROR: Could not install packages due to an OSError: [WinError 5] Access is denied: 'C:\\Users\\<user>\\AppData\\Local\\Temp\\pip-uninstall-km8mnbvv\\poetry.exe'

I mean you should rely on introduced ways, not fiddling in the user's system.
I really much prefer calling in a venv pip install poetry over the over-complicated new install-poetry.py.
I like the ideas of poetry over setuptools, but please keep it simple and do not waste the users' time.

...but don't mind, just take it as input, no answer needed, I'll fix it and decide my own on poetry vs. setuptools.

@finswimmer finswimmer mentioned this pull request Nov 27, 2021
@damarvin
Copy link

#4836 fixes it.
Thanks a lot, @finswimmer !

@Red-Eyed
Copy link

Just wonder, couldn't Path().as_posix() be used?

@serverwentdown
Copy link
Contributor Author

@Red-Eyed It wouldn't directly address the mistake made by the previous author.

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.