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 poetry with git dependencies #362

Closed
wants to merge 8 commits into from

Conversation

jaysonsantos
Copy link
Contributor

Hey there, this PR is to fix problems when you use poetry and git dependencies.
By default, poetry will generate a requirements.txt with -e/--editable flag which does not work well with -t (--target). This PR will read that file and rewrite the lines to make sure pip won't try to install in a wrong way.

@jaysonsantos jaysonsantos changed the title Fix poetry with git dendencies Fix poetry with git dependencies May 17, 2019
@jaysonsantos
Copy link
Contributor Author

The tests break because they try to clone the public repository using ssh and there is no key there.
What do you think is the best way to fix it? The fix I sent is actually to fix that problem.

@dschep
Copy link
Contributor

dschep commented May 24, 2019

@jaysonsantos any reason to not clone the public repository via https?

@jaysonsantos
Copy link
Contributor Author

@dschep There is when the bug happens, which is a common way when using private repositories to avoid having passwords on the lock.

@tpansino
Copy link
Contributor

tpansino commented Jul 25, 2019

@jaysonsantos This also fails if you connect over https as @dschep suggests, and wouldn't require an SSH key to be set up.

What do you think of using this instead?

bottle = {git = "https://github.com/bottlepy/bottle.git", tag = "0.12.16"}

Looking at the poetry export code, this should test out the same pathway you want.

I would very much like to see this PR merged, it's very annoying to try to use poetry and this plugin with any private or GitHub-hosted packages currently.

@dschep
Copy link
Contributor

dschep commented Jul 25, 2019

could you also resolve the conflicts please? This seems like a good PR to merge before i release next week :)

@tpansino
Copy link
Contributor

@dschep @jaysonsantos If it's ok with you I'd be happy to make the requested changes and fix the merge conflict today. I just need to be granted contributor access.

@dschep
Copy link
Contributor

dschep commented Jul 25, 2019

@tpansino, that actually won't do it since this PR is from a fork. I can't actually commit my suggestions either. Feel free to make a new PR that's based on this one instead.

@jaysonsantos jaysonsantos force-pushed the poetry-fixes branch 2 times, most recently from 52b8980 to 29cab16 Compare August 5, 2019 13:25
Co-Authored-By: Daniel Schep <dschep@gmail.com>
@jaysonsantos
Copy link
Contributor Author

Closing this as #390 got merged

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

Successfully merging this pull request may close these issues.

3 participants