Repository: denoland/deno
File: libs/npm_installer/bin_entries.rs (Rust)

Code before:
  bin_node_modules_dir_path: &Path,
) -> Result<EntrySetupOutcome<'a>, BinEntriesError> {
  let link = bin_node_modules_dir_path.join(bin_name);
  let original = package_path.join(bin_script);

  fn relative_path(from: &Path, to: &Path) -> Option<PathBuf> {
    pathdiff::diff_paths(to, from)
  }

  let found = make_executable_if_exists(sys, &original).map_err(|source| {
    BinEntriesError::SetUpBin {
      name: bin_name.to_string(),
      path: original.to_path_buf(),
      source: Box::new(source),
    }
  })?;
  if !found {
    return Ok(EntrySetupOutcome::MissingEntrypoint {
      bin_name,
      package_path,
      entrypoint: original,
      package,
      extra,
    });
  }

  let original_relative =
    relative_path(bin_node_modules_dir_path, &original).unwrap_or(original);

  if let Err(err) = sys.fs_symlink_file(&original_relative, &link) {
    if err.kind() == std::io::ErrorKind::AlreadyExists {
      // remove and retry
      sys.fs_remove_file(&link).map_err(|source| {
        BinEntriesError::RemoveBinSymlink {
          path: link.clone(),
          source,
        }
      })?;
      sys
        .fs_symlink_file(&original_relative, &link)
        .map_err(|source| BinEntriesError::SetUpBin {
          name: bin_name.to_string(),
          path: original_relative.to_path_buf(),
          source: Box::new(source.into()),
        })?;
      return Ok(EntrySetupOutcome::Success);
    }
    return Err(BinEntriesError::SetUpBin {
      name: bin_name.to_string(),
      path: original_relative.to_path_buf(),
      source: Box::new(err.into()),

Review comment (performance):
Maybe this should only happen on error? I've found reading symlinks to be really slow compared to just setting up the symlinks.

Code after:

fn symlink_bin_entry<'a>(
  sys: &(impl FsOpen + FsSymlinkFile + FsRemoveFile + FsReadLink),
  package: &'a NpmResolutionPackage,
  extra: &'a NpmPackageExtraInfo,
  bin_name: &'a str,
  bin_script: &str,
  package_path: &'a Path,
  bin_node_modules_dir_path: &Path,
) -> Result<EntrySetupOutcome<'a>, BinEntriesError> {
  let link = bin_node_modules_dir_path.join(bin_name);
  let original = package_path.join(bin_script);

  fn relative_path(from: &Path, to: &Path) -> Option<PathBuf> {
    pathdiff::diff_paths(to, from)
  }

  let original_relative = relative_path(bin_node_modules_dir_path, &original)
    .map(Cow::Owned)
    .unwrap_or_else(|| Cow::Borrowed(&original));

  if let Ok(original_link) = sys.fs_read_link(&link)
    && *original_link == *original_relative
  {
    return Ok(EntrySetupOutcome::Success);
  }

  let found = make_executable_if_exists(sys, &original).map_err(|source| {
    BinEntriesError::SetUpBin {
      name: bin_name.to_string(),
      path: original.to_path_buf(),
      source: Box::new(source),
    }
  })?;
  if !found {
    return Ok(EntrySetupOutcome::MissingEntrypoint {
      bin_name,
      package_path,
      entrypoint: original,
      package,
      extra,
    });
  }

  if let Err(err) = sys.fs_symlink_file(&*original_relative, &link) {
    if err.kind() == std::io::ErrorKind::AlreadyExists {
      // remove and retry
      sys.fs_remove_file(&link).map_err(|source| {
        BinEntriesError::RemoveBinSymlink {
          path: link.clone(),
          source,