Skip to content

--global-options Issue #845 Candidate#2017

Closed
reedjosh wants to merge 13 commits intopython-poetry:masterfrom
reedjosh:master
Closed

--global-options Issue #845 Candidate#2017
reedjosh wants to merge 13 commits intopython-poetry:masterfrom
reedjosh:master

Conversation

@reedjosh
Copy link
Copy Markdown

@reedjosh reedjosh commented Feb 10, 2020

Pull Request Check List

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

Regarding:
Note: If your Pull Request introduces a new feature or changes the current behavior, it should be based
on the develop

If the methods used in this pull request are satisfactory, I will rebase upon the develop branch.

I added an argument to the add command and updated through to the schema and pip install.

I've personally used this code to install a package that wouldn't be possible otherwise. I added a test in the add command and ran the pre-commit hook.

I have not full addressed issue #845 as I did not provide --build-options or --install-options. If these are desired I can add them too, I just wanted to get feedback on my methods first.

Thank you!

@finswimmer finswimmer added the kind/feature Feature requests/implementations label Feb 11, 2020
@reedjosh
Copy link
Copy Markdown
Author

Tests all fixed up. Please let me know your thoughts. I'm not certain the class usage model I've used jives with the intent of the objects. It does generate a nice readable toml file though.

pyodbc = {version = "4.0.26", global-options = ["build_ext", "--include-dirs", ".../joshuare/pkgs/include"]}

@reedjosh reedjosh requested a review from finswimmer February 12, 2020 17:47
@clintlombard
Copy link
Copy Markdown

clintlombard commented May 15, 2020

This would be super useful, for example installing tensorflow-graphics with GPU support requires passing a flag during installation. This fix would make it as simple as:

[tool.poetry.dependencies]
...
tensorflow-graphics-gpu = { git = "ssh://git@github.com:tensorflow/graphics.git", branch="master", global-option="--compute_platform gpu" }

@reedjosh
Copy link
Copy Markdown
Author

@clintlombard Yup, we've been keeping a local copy of this branch at work until it gets merged. Haven't heard anything from the maintainers though. : /

I'm still happy to work this further once I do hear back.

@thrix
Copy link
Copy Markdown

thrix commented May 29, 2020

I have not full addressed issue #845 as I did not provide --build-options or --install-options. If these are desired I can add them too, I just wanted to get feedback on my methods first.

Including them would be awesome, we now have to manually workaround this common problem on Fedora:

pycurl: libcurl link-time ssl backend (openssl) is different from compile-time ssl backend (none/other)

by manually reinstallaing the package in venv

@reedjosh
Copy link
Copy Markdown
Author

@finswimmer Hey, I see people are still asking for this feature. Just bumping to see if you have time to review.

Thank you!

@finswimmer finswimmer requested review from a team and removed request for finswimmer November 27, 2020 05:38
@jkklapp
Copy link
Copy Markdown

jkklapp commented Apr 8, 2021

This PR needs to be merged in 🙏

@mvtm-dn
Copy link
Copy Markdown

mvtm-dn commented Apr 22, 2021

I apologize, but does anybody work to merge this request?

@gjoseph92
Copy link
Copy Markdown

@reedjosh if this ever does get reviewed, here's a vote for --install-options; it would help me a lot. And here's a bump hoping this does get reviewed someday.

@ali5h
Copy link
Copy Markdown

ali5h commented Jun 7, 2021

another bump

@p-i-
Copy link
Copy Markdown

p-i- commented Jun 29, 2021

Including them would be awesome, we now have to manually workaround this common problem on Fedora:

pycurl: libcurl link-time ssl backend (openssl) is different from compile-time ssl backend (none/other)

by manually reinstallaing the package in venv

Bump! Are we nearly there yet? 🙈

@phyrwork
Copy link
Copy Markdown

phyrwork commented Nov 5, 2021

Bumpity bump. What's left to be done here?

@mvtm-dn
Copy link
Copy Markdown

mvtm-dn commented Apr 29, 2022

Bump

@toughrogrammer
Copy link
Copy Markdown

same here

@abn
Copy link
Copy Markdown
Member

abn commented May 18, 2022

This is probably not the right level to solve this issue. For example --include-dirs might vary depending on the user environment. This is more an installation target environmet configuration rather than a dependency configuration. While there might be some configuration that makes sense for everyone, this might not be the case for all configurations. Additioanally, these configurations only apply if a bdist is not already available for the dependency.

Additionally, it is important to note that in a future poetry release pip usage for installing packages will be removed.

@neersighted
Copy link
Copy Markdown
Member

Closing as pip is very much an internal implementation detail of Poetry these days, and we are moving to a custom installer and will drop our dependence on pip.

@dbischof90
Copy link
Copy Markdown

Closing as pip is very much an internal implementation detail of Poetry these days, and we are moving to a custom installer and will drop our dependence on pip.

As an addendum to this, do you have tracking issues for the development of the internal custom installer?

@neersighted
Copy link
Copy Markdown
Member

#6205

@github-actions
Copy link
Copy Markdown

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/feature Feature requests/implementations

Projects

None yet

Development

Successfully merging this pull request may close these issues.