bazel-contrib / bazel-contrib/toolchains_llvm

cc_wrapper.sh.tpl has comment referring to old approach that has long been replaced

Open
#476 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.