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

creating softlink (or provide the option to choose) #336

Closed
denisguzeyev opened this issue Aug 24, 2021 · 7 comments
Closed

creating softlink (or provide the option to choose) #336

denisguzeyev opened this issue Aug 24, 2021 · 7 comments

Comments

@denisguzeyev
Copy link

now:

  # make a backup (once)
  test -f "$interpreter_path,orig" || ln  "$interpreter_path" "$interpreter_path,orig"

wouldn't it be better to use softlink:

  # make a backup (once)
  test -f "$interpreter_path,orig" || ln -s  "$interpreter_path" "$interpreter_path,orig"

currently getting the following error (not critical):

cp: cannot create hard link '{path}/bin/python2.7,new' to '/usr/bin/python2.7': Invalid cross-device link

@nailor
Copy link
Collaborator

nailor commented Aug 24, 2021

Hi,

unless I’m misreading the issue the link is created by virtualenv, and as such it is not something we can affect. Please reopen the issue if this is not the case

@nailor nailor closed this as completed Aug 24, 2021
@drexljo
Copy link

drexljo commented Aug 24, 2021

Generally it's better to use symlinks and avoid hardlinks as much as possible, as
a) developers don't know where SysOps decide to mount different file systems which break hardlinks and
b) symlinks are visible to the user, thus functions and problems are more transparent

@denisguzeyev
Copy link
Author

denisguzeyev commented Aug 24, 2021

Hi @nailor, thank you for the fast answer. Could you please reopen the issue

probably I provided a wrong part of source code.
This part exacrly causes mentioned above error:

hardlink or copy new interpreter

    cp -fpl "/usr/bin/$pythonX_Y" "$interpreter_path,new" \
        || cp -fp "/usr/bin/$pythonX_Y" "$interpreter_path,new" \
        || rm -f "$interpreter_path,new" \
        || true

cp: cannot create hard link '{path}/bin/python2.7,new' to '/usr/bin/python2.7': Invalid cross-device link

@nailor
Copy link
Collaborator

nailor commented Aug 24, 2021

Sorry, my bad! Yes this indeed is in the upgrade script designed to catch cases where the Python interpreter updates on system level and to avoid breakage caused by it.

It should already have the or clause and fall back to regular copy if hard link fails. Unsure why that does not happen in your case.

for context we are defaulting to hard link as that is what virtualenv also sets up

@nailor nailor reopened this Aug 24, 2021
@jhermann
Copy link
Contributor

a) "backup" and "symlink" don't mix
b) the error msg can be ignored, if anything can be improved, then it'd be "2>/dev/null" in the 1st command
c) that code shouldn't trigger anymore anyway if you use venv, since stuff is not copied anymore. so another improvement would be to simply disable it then, since it server no purpose with venv (namely survive host python updates).

@drexljo
Copy link

drexljo commented Aug 30, 2021

Hard links are no backup. So for most/all purposes they can be replaced by symlinks, which can cross filesystem boundaries.

@nailor
Copy link
Collaborator

nailor commented Sep 12, 2021

Thanks @jhermann pointing out this is not an error but just the stderr output which is not redirected.

I think the error message being visible is fine, so since there isn’t anything really broken, closing the issue

@nailor nailor closed this as completed Sep 12, 2021
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

No branches or pull requests

4 participants