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

Added --root command line option to give functionality equivalent to distutils --root #693

Merged
merged 4 commits into from Oct 29, 2012

Conversation

Projects
None yet
3 participants
@hetmankp

hetmankp commented Oct 2, 2012

This is a clean up of the patch initially suggested for resolving issue #253. I've also added a simple test to make sure the option behaves as expected.

@pnasrat

This comment has been minimized.

Show comment
Hide comment
@pnasrat

pnasrat Oct 3, 2012

Contributor

Thanks for the patch - it looks like your attempt to fix pypy is still causing failing tests

https://travis-ci.org/#!/pypa/pip/builds/2647084

Does this pass for you locally?

Contributor

pnasrat commented Oct 3, 2012

Thanks for the patch - it looks like your attempt to fix pypy is still causing failing tests

https://travis-ci.org/#!/pypa/pip/builds/2647084

Does this pass for you locally?

@qwcode

This comment has been minimized.

Show comment
Hide comment
@qwcode

qwcode Oct 3, 2012

Contributor

that 1 pypy test has been failing periodically in the develop branch as well.
trying to sort out here: #698

Contributor

qwcode commented Oct 3, 2012

that 1 pypy test has been failing periodically in the develop branch as well.
trying to sort out here: #698

@hetmankp

This comment has been minimized.

Show comment
Hide comment
@hetmankp

hetmankp Oct 5, 2012

Yep, this doesn't seem to be related to my changes. Judging by the activity on pull request #698 this seems to have been resolved now?

Is there any way to tell Travis to re-run the test on my pull request changes to confirm it's all working now? Thanks.

hetmankp commented Oct 5, 2012

Yep, this doesn't seem to be related to my changes. Judging by the activity on pull request #698 this seems to have been resolved now?

Is there any way to tell Travis to re-run the test on my pull request changes to confirm it's all working now? Thanks.

@qwcode

This comment has been minimized.

Show comment
Hide comment
@qwcode

qwcode Oct 5, 2012

Contributor

you'd need to merge or rebase on develop to get that test fix I made in #698
afaik, you can only trigger travis based on branch changes. a "redo" button would be nice.

Contributor

qwcode commented Oct 5, 2012

you'd need to merge or rebase on develop to get that test fix I made in #698
afaik, you can only trigger travis based on branch changes. a "redo" button would be nice.

@hetmankp

This comment has been minimized.

Show comment
Hide comment
@hetmankp

hetmankp Oct 8, 2012

Ok, thanks, rebased. The pull request should be good to go now.

hetmankp commented Oct 8, 2012

Ok, thanks, rebased. The pull request should be good to go now.

@pnasrat

This comment has been minimized.

Show comment
Hide comment
@pnasrat

pnasrat Oct 23, 2012

Contributor

Looks great - do you want to add yourself to AUTHORS.txt if not present and update docs/news.txt and I'll merge.

Contributor

pnasrat commented Oct 23, 2012

Looks great - do you want to add yourself to AUTHORS.txt if not present and update docs/news.txt and I'll merge.

@hetmankp

This comment has been minimized.

Show comment
Hide comment
@hetmankp

hetmankp Oct 28, 2012

Sorry about the delayed reply, I hadn't realised where the message notification had been moved to on Github most recently.

No problems I'll do that. One question because I'm not sure how this is usually done. Would you prefer if I squash all the changes in my pull request into a single commit? Or do you prefer to keep a more fine grained history? Or is all the squashing something that happens automatically when you do the merging anyway?

Cheers.

hetmankp commented Oct 28, 2012

Sorry about the delayed reply, I hadn't realised where the message notification had been moved to on Github most recently.

No problems I'll do that. One question because I'm not sure how this is usually done. Would you prefer if I squash all the changes in my pull request into a single commit? Or do you prefer to keep a more fine grained history? Or is all the squashing something that happens automatically when you do the merging anyway?

Cheers.

@qwcode

This comment has been minimized.

Show comment
Hide comment
@qwcode

qwcode Oct 28, 2012

Contributor

we don't require the commits to be squashed. you can just leave it as is. github merging doesn't squash.

Contributor

qwcode commented Oct 28, 2012

we don't require the commits to be squashed. you can just leave it as is. github merging doesn't squash.

@hetmankp

This comment has been minimized.

Show comment
Hide comment
@hetmankp

hetmankp Oct 29, 2012

Alright I've updated those two files so it should be ready to go.

hetmankp commented Oct 29, 2012

Alright I've updated those two files so it should be ready to go.

pnasrat added a commit that referenced this pull request Oct 29, 2012

Merge pull request #693 from hetmankp/develop
Added --root command line option to give functionality equivalent to distutils --root

@pnasrat pnasrat merged commit fad6532 into pypa:develop Oct 29, 2012

1 check passed

default The Travis build passed
Details
@qwcode

This comment has been minimized.

Show comment
Hide comment
@qwcode

qwcode Jan 22, 2013

Contributor

the test for this is failing on windows:
http://jenkins.qwcode.com/job/pip_win_27/10/console

Contributor

qwcode commented Jan 22, 2013

the test for this is failing on windows:
http://jenkins.qwcode.com/job/pip_win_27/10/console

@hetmankp

This comment has been minimized.

Show comment
Hide comment
@hetmankp

hetmankp Jan 30, 2013

Hi, is this still an issue? The link you gave doesn't seem to go anywhere.

hetmankp commented Jan 30, 2013

Hi, is this still an issue? The link you gave doesn't seem to go anywhere.

@qwcode

This comment has been minimized.

Show comment
Hide comment
@qwcode

qwcode Jan 30, 2013

Contributor

the test is working now on linux and windows with a minor change.
can you confirm the test still looks valid to you?
the change is in this diff: 6b5fd90

Contributor

qwcode commented Jan 30, 2013

the test is working now on linux and windows with a minor change.
can you confirm the test still looks valid to you?
the change is in this diff: 6b5fd90

@hetmankp

This comment has been minimized.

Show comment
Hide comment
@hetmankp

hetmankp Feb 1, 2013

Hi, I've made the test a little better so that it works correctly for the special case where it is most useful. It's in pull request #787. Turns out Python's distutils simply doesn't allow unix style root paths to be specified under windows for an installation.

hetmankp commented Feb 1, 2013

Hi, I've made the test a little better so that it works correctly for the special case where it is most useful. It's in pull request #787. Turns out Python's distutils simply doesn't allow unix style root paths to be specified under windows for an installation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment