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
Eliminate symbolic links #20
Conversation
Nice work @benrudolph, this would be great to have |
old_dir = os.path.normpath(os.path.abspath(old_dir)) | ||
new_dir = os.path.normpath(os.path.abspath(new_dir)) | ||
old_dir = os.path.realpath(os.path.normpath(os.path.abspath(old_dir))) | ||
new_dir = os.path.realpath(os.path.normpath(os.path.abspath(new_dir))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems that os.path.realpath
is all that is actually needed as it expands to the full, canonical path and the usage of both normpath
and abspath
can be removed.
In [10]: x
Out[10]: 'foodir2'
In [11]: os.path.islink(x)
Out[11]: True
In [12]: os.path.realpath(x)
Out[12]: '/Users/xxxxxx/Workspace/projects/xxxxxxxxxxx/foodir'
so this could just be:
old_dir = os.path.realpath(old_dir)
new_dir = os.path.realpath(new_dir)
do you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed. i'll update thanks for looking into it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benrudolph can you update this to remove the unnecessary path code here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I was looking at just the first commit, and you changed this in a second one. Merged, thanks @benrudolph!
thanks @benrudolph. let me see if i can get travis or circleci set up with this repo and then i'll merge |
👍 sounds good |
Thanks tim! Happy to see this merged
…On Sat, Sep 16, 2017 at 12:37 PM Tim Abbott ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In clonevirtualenv.py
<#20 (comment)>
:
> @@ -291,8 +291,8 @@ def main():
except ValueError:
print("virtualenv-clone {}".format(__version__))
parser.error("not enough arguments given.")
- old_dir = os.path.normpath(os.path.abspath(old_dir))
- new_dir = os.path.normpath(os.path.abspath(new_dir))
+ old_dir = os.path.realpath(os.path.normpath(os.path.abspath(old_dir)))
+ new_dir = os.path.realpath(os.path.normpath(os.path.abspath(new_dir)))
Oops, I was looking at just the first commit, and you changed this in a
second one. Merged, thanks @benrudolph <https://github.com/benrudolph>!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA4D8hf67X33xjxth_AuxuUItz-7JqrYks5si_k6gaJpZM4FwbLM>
.
|
This removes any references to symbolic links when cloning. We were seeing a bug due to this in our deploy process. For example, suppose
current
is the symlink to the current release deployed.old_dir
will becurrent/python_env
but in actuality the proper old path isrelease1/python_env
and thus the find and replace fails