bazel-contrib / bazel-contrib/toolchains_llvm
cc_wrapper.sh.tpl has comment referring to old approach that has long been replaced
- Dominant language
- Starlark
- Stars
- 371
- Forks
- 283
- Avg merge
- 1d 55m
- Merged PRs (30d)
- 25
Description
I happened to be reading the `cc_wrapper.sh.tpl` file, which has the following comment at the top:
```
# OS X relpath is not really working. This is a wrapper script around gcc
# to simulate relpath behavior.
#
# This wrapper uses install_name_tool to replace all paths in the binary
# (bazel-out/.../path/to/original/library.so) by the paths relative to
# the binary. It parses the command line to behave as rpath is supposed
# to work.
#
# See https://blogs.oracle.com/dipol/entry/dynamic_libraries_rpath_and_mac
# on how to set those paths for Mach-O binaries.
```
This comment appears to be referring to the solution implemented in the initial version of `cc_wrapper.sh.tpl` in [this commit](https://github.com/bazel-contrib/toolchains_llvm/commit/408474baa30039b6c5ba35c9545e2edcf644bb3d#diff-d0bdf5b1cdb738b12a358c7a5bf3b7f70be7678bdc84cd21fbc09d7b42193419). That solution was completely replaced with a different approach in [this commit](https://github.com/bazel-contrib/toolchains_llvm/commit/e4b62c9a4f123cf25701ce0760fe050fc4e293b3#diff-d0bdf5b1cdb738b12a358c7a5bf3b7f70be7678bdc84cd21fbc09d7b42193419), however. The comment still refers to the old approach, which can be confusing when trying to understand what the wrapper script is doing.
Contributor guide
Research direction
Start in cc_wrapper.sh.tpl and compare its top comment with the current implementation and the two referenced commits. Update the comment so it describes the approach actually used by the wrapper; the work is done when no text refers to the replaced implementation and the file remains consistent with its behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- shell
- Domain
- documentation
- Issue type
- Documentation
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100