-
-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
recc 1.2.21 #191819
recc 1.2.21 #191819
Conversation
Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. A few suggestions.
bd370df
to
699e2df
Compare
@carlocab, thank you for your detailed review. It really helped to trim down a lot of unnecessary content. I have addressed all the review comments. I included caveat about usage that is specific to installation using Homebrew formula. I noticed a similar caveat is used for ccache. |
RECC is a compiler launcher that caches the results on compilation and link command, and optionally forward them to a remote execution service. Remove running launchctrl command in caveat Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> recc 1.2.21 Address review comments recc 1.2.21 tests Added tests to start recc-server and run recc-cc command twice. The second run should result in a cache hit. Update test case Update test Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> recc 1.2.21: Update user configuration location Use etc as location for system wide recc.conf location. In the absense of the etc/recc.conf, a default configuration installed in prefix/etc/recc/recc.conf will be used. recc 1.2.21 Add caveat about how to invoke compiler using recc. The instructions in caveat is specific to how homebrew is configured Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> Update Formula/r/recc.rb Co-authored-by: Carlo Cabrera <[email protected]> recc 1.2.24 Update version Use wrapper scripts from the upstream project
Dropped the merge commit, this should now be ready for review again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sakeeb, great work (and patience) here. All comments can be addressed in follow-up releases/PRs. You rock!
ENV["RECC_BIN"] = bin/"recc" | ||
system "command envsubst < scripts/wrapper-templates/recc-c++.in > recc-c++" | ||
system "command envsubst < scripts/wrapper-templates/recc-cc.in > recc-cc" | ||
bin.install "recc-c++" | ||
bin.install "recc-cc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A make install
for this would be nice to have.
system "command envsubst < scripts/wrapper-templates/recc.conf.in > recc.conf" | ||
etc.install "recc.conf" | ||
|
||
bin.install "scripts/wrapper-templates/casd-helper" => "recc-server" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this
<<~EOS | ||
To launch a compiler with recc, set the following variables: | ||
CC=#{opt_bin}/recc-cc | ||
CXX=#{opt_bin}/recc-c++ | ||
EOS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these Homebrew specific caveats?
# Override default values of server and log_level | ||
ENV["RECC_SERVER"] = "unix://#{recc_cache_dir}/casd.sock" | ||
ENV["RECC_LOG_LEVEL"] = "info" | ||
recc_test=[bin/"recc-cc", "-c", test_file] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recc_test=[bin/"recc-cc", "-c", test_file] | |
recc_test = [bin/"recc-cc", "-c", test_file] |
RECC is a compiler launcher that caches the results on compilation and link command, and optionally forward them to a remote execution service.
HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?brew audit --strict <formula>
(after doingHOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>
)? If this is a new formula, does it passbrew audit --new <formula>
?