Skip to content

Update manpage with information about alternative compilers#4398

Merged
MikeMcQuaid merged 10 commits intoHomebrew:masterfrom
afnanenayet:update-compiler-docs
Jul 7, 2018
Merged

Update manpage with information about alternative compilers#4398
MikeMcQuaid merged 10 commits intoHomebrew:masterfrom
afnanenayet:update-compiler-docs

Conversation

@afnanenayet
Copy link
Copy Markdown
Contributor

@afnanenayet afnanenayet commented Jun 30, 2018

This change references #4387. While I understand alternative compilers aren't officially supported, it is worth documenting Homebrew's behavior with the options that do allow users to compile with something besides Apple's bundled clang.

  • added a detail to the --cc param, stating how to use the Homebrew provided clang, also noting that issues should not be filed if this flag is used
  • removed trailing whitespace in the manpage's markdown source.

Note: I did not write any additional tests, because I did not change any functionality.

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

* Update the manpage source file with additional information
    * document the `HOMEBREW_CC` variable with usage information
    * update usage information for `--cc`, specifying how to use llvm's clang
@afnanenayet
Copy link
Copy Markdown
Contributor Author

afnanenayet commented Jun 30, 2018

I'm not sure what the proper way is to go about updating the documentation, it says that travis failed because the man pages changed.

@jonchang
Copy link
Copy Markdown
Contributor

jonchang commented Jul 1, 2018

You'll need to run brew man to update the man pages and commit it with your pull request.

@afnanenayet
Copy link
Copy Markdown
Contributor Author

Ah gotcha, thank you!

Use brew man to generate manpages, and then copy them to the repo.
@jonchang
Copy link
Copy Markdown
Contributor

jonchang commented Jul 1, 2018

@afnanenayet my apologies for not being clearer. The manpages are automatically generated from documentation embedded in the Homebrew source files. So you need to update the documentation comments in the relevant files, and then run brew man to regenerate the manpages and commit it in the same pull request. This process could probably be better documented.

For the changes that you want, you'll need to update cmd/install.rb and manpages/brew.1.md.erb.

@afnanenayet
Copy link
Copy Markdown
Contributor Author

Haha yeah I was very confused. Thanks for the pointer, maybe we should add this to the documentation as well.

Properly update man pages using the local brew executable (from the repo),
and copy the man pages to the repo.
@afnanenayet
Copy link
Copy Markdown
Contributor Author

afnanenayet commented Jul 2, 2018

@jonchang is there anything we can do about the code coverage?

Edit: JK, I thought that the codecov regression was blocking the merge

Comment thread Library/Homebrew/cmd/install.rb Outdated
#: <compiler> should be the name of the compiler's executable, for instance
#: `gcc-4.2` for Apple's GCC 4.2, or `gcc-4.9` for a Homebrew-provided GCC
#: 4.9.
#: 4.9. In order to use LLVM's clang, use `llvm_clang`. To specify the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to reference current versions e.g. gcc, gcc-6 or gcc-4.9? Could you also add Please do not file issues if you encounter errors when using this flag.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes for sure, I used those examples because that's what was documented previously, and I wasn't sure if I should change them

Comment thread Library/Homebrew/manpages/brew.1.md.erb Outdated

*Default:* `~/Library/Caches/Homebrew`.

* `HOMEBREW_CC`:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably rather we left this undocumented as it's very likely to break things if you set it in your environment.

Comment thread docs/Custom-GCC-and-cross-compilers.md Outdated
* Homebrew provides a `gcc` formula for use with Xcode 4.2+ or when needing
C++11 support on earlier versions.
C++11 support on earlier versions.
* Homebrew provides older GCC formulae, e.g. `gcc@4.8` and `gcc@6`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gcc@4.9 and gcc@6?

@MikeMcQuaid
Copy link
Copy Markdown
Member

@jonchang is there anything we can do about the code coverage?

Edit: JK, I thought that the codecov regression was blocking the merge

Yep, can just ignore code coverage on this PR 👍

* update references in `--cc` flag, using modern gcc examples
* note that issues should not be filed if `--cc` flag is used
* remove reference to `HOMEBREW_CC` as it should not be exposed to users
* generate manpages using `brew man` with these changes
This fixes the rubocop offences, which was blocking Travis
Generate new man pages so that the docs and manpages match up
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple more tiny tweaks then good to 🚢! Thanks again.

Comment thread Library/Homebrew/cmd/install.rb Outdated
#: Homebrew-provided GCC 4.9. In order to use LLVM's clang, use
#: `llvm_clang`. To specify the Apple-provided clang, use `clang`. This
#: parameter will only accept compilers that are provided by Homebrew or
#: bundled with MacOS. Please do not file issues if you encounter errors
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macOS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been spelling it wrong all this time...O.o

* change `MacOS` -> `macOS`
* regenerate manpages
@MikeMcQuaid
Copy link
Copy Markdown
Member

Thanks so much for your first contribution! Without people like you submitting PRs we couldn't run this project. You rock, @afnanenayet!

@MikeMcQuaid MikeMcQuaid merged commit b350fe0 into Homebrew:master Jul 7, 2018
@afnanenayet
Copy link
Copy Markdown
Contributor Author

@MikeMcQuaid @jonchang I really appreciate how you guys helped me out despite me not knowing anything about how homebrew is structured and literally taking 10 commits to update some doc -- really appreciate the encouragement :)

@afnanenayet afnanenayet deleted the update-compiler-docs branch July 7, 2018 13:58
@lock lock bot added the outdated PR was locked due to age label Aug 6, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants