Skip to content

Consider using symlinks for rustup proxies #4022

Description

@ChrisDenton

This was brought up elsewhere so I'm writing an issue here to note the conversation. I'm happy to implement this if it's acceptable (though I don't mind if someone else wants to claim it).

Currently rustup proxies are created using hard links with a fallback to soft links if creating hard links aren't supported by the filesystem or the platform (e.g. android). This proposal is to swap the order so that proxies are created using soft links with a fallback to hard links if creating soft links aren't supported by the filesystem or platform (e.g. Windows in the default configuration).

Hard links have the issue that they are, for most purposes, normal files. So many copy utilities (including cp) will just naively copy the file. This can be a problem when copying the rustup directory as all the hard linked proxies will be turned into literal copies of rustup.

Using symlinks for this use case is common in Unix-like environments. E.g. busybox is a canonical example. I have not been able to find any real downside to this approach (though I may have missed something).

Activity

  1. rami3l commented on Sep 16, 2024

    @rami3l
    Member

    @ChrisDenton Thanks! I personally have no objection to this proposal, and would love to see experiments regrading switching to soft links.

    cc @rbtcollins Just in case, do you have any historical context in terms of why we are doing hard links today?

  2. rami3l commented on Sep 17, 2024

    @rami3l
    Member

    @ChrisDenton I've just been pointed out the original rationale:

    Use hardlinks instead of softlinks for self installation / --link-local

    Softlinks need admin access on windows, which is a pain. I think
    hardlinks should be fine since we are in control of all the sources
    and destinations.
    #88

    This reminds me again of the fact that I'm still quite new to Windows development. Maybe things have changed since then, or you've correctly handled this case in #4023 already?

  3. ChrisDenton commented on Sep 17, 2024

    @ChrisDenton
    MemberAuthor

    Creation of symlinks can be enabled for all users by turning on developer mode in settings. Which does require someone with admin access to do (though most users do have admin access on their own machine).

    image

    Alternatively an admin can assign the SeCreateSymbolicLinkPrivilege permission to a specific user or group.


    From the above you can infer that this is not enabled by default but that's ok because if creating symlinks fails we fallback to using hardlinks. Also all users can still use or delete symlinks so it doesn't matter if another user creates the symlink (although running the same rustup for multiple users isn't a supported workflow in any case),

  4. added this to the 1.28.0 milestone on Sep 17, 2024
  5. rbtcollins commented on Sep 19, 2024

    @rbtcollins
    Contributor

    From memory we also have some logic about detecting alterations that looks for the hardlink status of a proxy. So this may cause errors or failure to delete proxies on uninstall. I have no fundamental objection here, just will need some digging to be sure everything is caught.

  6. ChrisDenton commented on Sep 19, 2024

    @ChrisDenton
    MemberAuthor

    Hm, I did see some code like that but it opens files by following symlinks and then uses same-file crate to compare with rustup. So I think that's unaffected. But I'll look into uninstall too.

    Also do note that the symlink code path already existed (albeit as a fallback) so hopefully any such issues have already been addressed.

  7. rami3l commented on Sep 20, 2024

    @rami3l
    Member

    @ChrisDenton #4023 has unfortunately caused regressions on FreeBSD:

    > rustup self uninstall -y
      out.ok: false
      out.stdout (0 lines):
          
      out.stderr (4 lines):
          info: removing rustup home
          info: removing cargo home
          info: removing rustup binaries
          error: could not remove 'cargo_home' directory: '/home/runner/work/rustup/rustup/target/x86_64-unknown-freebsd/tests/running-test-xrCTlH/rustup-cargoDGW09M/ch': Too many links (os error 31)

    https://github.com/rust-lang/rustup/actions/runs/10901896415/job/30252621940

    Could this be a restriction with UFS2? 1

    Footnotes

    1. See https://stackoverflow.com/a/22604322 for a similar syndrome. ↩

  8. reopened this on Sep 20, 2024
  9. ChrisDenton commented on Sep 20, 2024

    @ChrisDenton
    MemberAuthor

    Ooh interesting, "too many links" seems a bit weird considering we're now prioritising symlinks.

    31 EMLINK Too many links. The number of hard links to a single file has exceeded the maximum. (The system-wide maximum number of hard links is 32767. Each file system may impose a lower limit for files contained within it).

    I wonder why we're making so many hard links... I will investigate tomorrow.

  10. ChrisDenton commented on Sep 20, 2024

    @ChrisDenton
    MemberAuthor

    Ah just saw your edit. Yeah that seems worth looking at.

  11. ChrisDenton commented on Sep 20, 2024

    @ChrisDenton
    MemberAuthor

    Hm, so I'm confused why this is failing at removing directories, not creating proxies. As far as I can tell (I could be wrong, please correct me if so), removing the directory ultimately lands on

    remove_dir_all::remove_dir_all(path)
    which is the remove_dir_all crate. I'm not sure why that would be failing on FreeBSD when there are symlinks in the directory. cc @rbtcollins

    I think this might be one of those cases where an errno is overloaded so it's error string is a bit misleading. The obvious workaround is to just always use hard links on FreeBSD for now.

  12. ChrisDenton commented on Sep 20, 2024

    @ChrisDenton
    MemberAuthor

    Ooh, this might be relevant: https://man.freebsd.org/cgi/man.cgi?query=open&sektion=2&n=1#end. There's a mention that FreeBSD returns EMLINK instead of ELOOP. Which might be relevant here: https://github.com/XAMPPRocky/remove_dir_all/blob/4306a5f6b9f5a3ecd784349b333c33db7e1686b1/src/_impl.rs#L153

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions