Repository: carbon-language/carbon-lang
File: toolchain/install/busybox_info.cpp (C++)

Code before:
    // current binary path. This can help more immediately locate an
    // installation tree, and avoids walking through a final layer of symlinks
    // which may point to content-addressed storage or other parts of a build
    // output tree.
    //
    // We break this into two cases we need to handle:
    // - Carbon's CLI will be: `<prefix>/bin/carbon`
    // - Other tools will be: `<prefix>/lib/carbon/<group>/bin/<tool>`
    //
    // We also check that the current path is within a `bin` directory to
    // provide best-effort checking for accidentally walking up from symlinks
    // that aren't within an installation-shaped tree.
    auto parent_path = info.bin_path.parent_path();
    // Strip any `.` path components at the end to simplify processing.
    while (parent_path.filename() == ".") {
      parent_path = parent_path.parent_path();
    }
    if (parent_path.filename() == "bin") {
      // Note that we walk up using `.parent_path` rather than by appending
      // `../` components. While largely equivalent, this helps keep paths
      // shorter and avoids redundant work. We also don't expect to need to
      // respect _internally_ strange symlinking structures that would need to
      // use appended `../` components.
      auto lib_path = info.bin_path.filename() == "carbon"
                          ? parent_path.parent_path() / "lib" / "carbon"
                          : parent_path.parent_path().parent_path();
      auto busybox_path = lib_path / "carbon-busybox";
      if (auto access = Filesystem::Cwd().Access(busybox_path);
          access.ok() && *access) {
        info.bin_path = busybox_path;
        return info;
      }
    }

    // Try to walk through another layer of symlinks and see if we can find the
    // installation there or are linked directly to the busybox.
    auto readlink = Filesystem::Cwd().Readlink(info.bin_path);
    if (!readlink.ok()) {
      return ErrorBuilder()
             << "expected carbon-busybox symlink at `" << info.bin_path << "`";
    }

    // Do a path join, to handle relative symlinks.
    info.bin_path = parent_path / *readlink;
  }
}

}  // namespace Carbon


Review comment (suggestion):
If somebody does something like:

```
cd /usr/lib/carbon/llvm
./bin/clang --version
```

In this case, `parent_path.filename() == "bin"`, so this code will execute.

I think here that `parent_path.parent_path()` is `./` (fine enough for the first branch which adds extra discerning suffixes), but `parent_path.parent_path().parent_path()` is the empty string.

So what I'm seeing is that if someone runs "./bin/clang", this code will check for "./carbon-busybox" (which won't exist) instead of "../carbon-busybox" (which would be expected to exist). Are you good with that?

(Should this be tested? It looks like tests may not doo much cwd-relative access right now)

Code after:
      // Check for bazel structure. For example, this makes work:
      //   /bin/sh -c "exec -a carbon ./bazel-bin/toolchain/carbon"
      //   /bin/sh -c "exec -a llvm-symbolizer ./bazel-bin/toolchain/carbon"
      //
      // This will never occur in a "bin" subdirectory, so doesn't need to be
      // handled in the other return path.
      std::string busybox_path = info.bin_path.parent_path().string() +
                                 "/prefix/lib/carbon/carbon-busybox";
      if (auto access = Filesystem::Cwd().Access(busybox_path);
          access.ok() && *access) {
        info.bin_path = busybox_path;
      }
      return info;
    }

    // If we've not already reached the busybox, look for it relative to the
    // current binary path. This can help more immediately locate an
    // installation tree, and avoids walking through a final layer of symlinks
    // which may point to content-addressed storage or other parts of a build
    // output tree.
    //
    // We break this into two cases we need to handle:
    // - Carbon's CLI will be: `<prefix>/bin/carbon`
    // - Other tools will be: `<prefix>/lib/carbon/<group>/bin/<tool>`
    //
    // We also check that the current path is within a `bin` directory to
    // provide best-effort checking for accidentally walking up from symlinks
    // that aren't within an installation-shaped tree.
    auto parent_path = info.bin_path.parent_path();
    // Strip any `.` path components at the end to simplify processing.
    while (parent_path.filename() == ".") {
      parent_path = parent_path.parent_path();
    }
    if (parent_path.filename() == "bin") {
      // Note that we use a specialized approach to walking up rather than
      // always appending `../` components. While largely equivalent, this helps
      // keep paths shorter and avoids redundant work. We also don't expect to
      // need to respect _internally_ strange symlinking structures that would
      // need to use appended `../` components.
      auto lib_path = info.bin_path.filename() == "carbon"
                          ? WalkUp(std::move(parent_path)) / "lib" / "carbon"
                          : WalkUp(WalkUp(std::move(parent_path)));
      auto busybox_path = lib_path / "carbon-busybox";
      if (auto access = Filesystem::Cwd().Access(busybox_path);
          access.ok() && *access) {
        info.bin_path = busybox_path;
        return info;
      }
    }

    // Try to walk through another layer of symlinks and see if we can find the