Skip to content
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

man-db: symlink commands without 'g' prefix into libexec/bin #36769

Closed
wants to merge 1 commit into from

Conversation

lembacon
Copy link
Member

@lembacon lembacon commented Feb 6, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Make man-db consistent with other formulae prefixed by g.

@lembacon lembacon requested a review from sjackman February 6, 2019 08:25
Copy link
Member

@sjackman sjackman left a comment

Choose a reason for hiding this comment

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

Thanks, @lembacon!

@sjackman
Copy link
Member

sjackman commented Feb 6, 2019

Ping @maxim-belkin

@maxim-belkin
Copy link
Member

I guess I'm not sure why we need to do this. @lembacon, what is the goal of installing symlinks in libexec?

@sjackman
Copy link
Member

sjackman commented Feb 6, 2019

So people can add libexec/gnubin to the front of their PATH to use man rather than gman.

Copy link
Member

@sjackman sjackman left a comment

Choose a reason for hiding this comment

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

Please add caveats.

def caveats; <<~EOS
   Commands also provided by macOS have been installed with the prefix "g".
   If you need to use these commands with their normal names, you
   can add a "gnubin" directory to your PATH from your bashrc like:
     PATH="#{opt_libexec}/gnubin:$PATH"
 EOS
 end

@maxim-belkin
Copy link
Member

So people can add libexec/gnubin to the front of their PATH to use man rather than gman.

I'm confused. Why are we adding g prefix in the first place?

@sjackman
Copy link
Member

sjackman commented Feb 6, 2019

The default name is gman. People can choose to instead use man to execute gman by either adding an alias or adding libexec/gnubin to their PATH.

@maxim-belkin
Copy link
Member

maxim-belkin commented Feb 6, 2019

The default name is gman. People can choose to instead use man to execute gman by either adding an alias or adding libexec/gnubin to their PATH.

Small correction: the default name is man which we change to gman with the explanation that man is provided by macOS.

  1. What I don't understand is why we first say "it is dangerous to provide man (and similar) executables because they're provided by macOS" and then say "OK, let's allow (advanced) users do that".

  2. The trickery suggested in this PR is outside of the standard procedures for building man-db formula and is Homebrew- and macOS- specific. If there was a decision to follow this path, my comment would be the following: in the current state of Homebrew, this is probably fine. But in the long run there should be a method for doing that semi-automatically does that by simply providing names for binary files that have to be linked/copied/etc. If there was no such decision, I'd suggest rethink the strategy.

# Symlink commands without 'g' prefix into libexec/gnubin and
# man pages into libexec/gnuman
%w[apropos catman lexgrog man mandb manpath whatis].each do |cmd|
(libexec/"gnubin").install_symlink bin/"g#{cmd}" => cmd
Copy link
Member

Choose a reason for hiding this comment

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

man-db is not gnu. Having gnu prefix is deceptive.

(libexec/"gnubin").install_symlink bin/"g#{cmd}" => cmd
end
%w[accessdb].each do |cmd|
(libexec/"gnusbin").install_symlink sbin/"g#{cmd}" => cmd
Copy link
Member

Choose a reason for hiding this comment

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

  1. This is a single file, so no need for 3 lines of code.
  2. Same as above: using gnu is deceptive. How about nongnu?

@maxim-belkin
Copy link
Member

I left couple of comments but they apply to other places in the PR as well -- I just didn't want to flood the PR with repetitive text.

@sjackman
Copy link
Member

sjackman commented Feb 6, 2019

The approach taken by @lembacon here is the same as in the GNU tools.
See

def caveats; <<~EOS
Commands also provided by macOS have been installed with the prefix "g".
If you need to use these commands with their normal names, you
can add a "gnubin" directory to your PATH from your bashrc like:

and also findutils modules grep ed monkeysphere cockroach gawk inetutils gnu-time flex gnu-which emacs make gnu-tar gnu-indent gnu-sed gnu-units

@sjackman
Copy link
Member

sjackman commented Feb 6, 2019

I agree that libexec/gnubin and libexec/gnuman are a bit strange names, since the tool is not GNU. I suggest that the directories be named libexec/bin and libexec/man.

@ericbn
Copy link
Contributor

ericbn commented Feb 6, 2019

I suggest there should be some guidelines about when to symlink binaries:

  • in the path (/usr/local/bin, /usr/local/sbin) with the original name
  • in the path, but adding a prefix to the original name, and then guidelines about the prefixes
  • outside the path (/usr/local/opt/), and then guidelines about the path format
  • when to mix more than one approach above in the same formula

I don't know what else is out there, but we already saw some back and forth in #36494 and I believe that was partially because of the lack of a clear plan.

@sjackman
Copy link
Member

sjackman commented Feb 6, 2019

Anything that doesn't shadow a macOS /usr/bin/ executable is fair game for symlinking.
We've settled on the prefix g as the prefix, understanding that it's a totally arbitrary choice.
Outside the PATH you can call it whatever you want, so use the upstream names.

@lembacon lembacon force-pushed the man-db-gnubin-gnuman branch from abbb77e to b291177 Compare February 7, 2019 07:55
@lembacon lembacon changed the title man-db: add gnubin and gnuman symlink dirs man-db: symlink commands without 'g' prefix into libexec/bin Feb 7, 2019
@lembacon
Copy link
Member Author

lembacon commented Feb 7, 2019

@sjackman @maxim-belkin I've made some changes,

  • Added caveats.
  • Renamed libexec/gnubin and libexec/gnuman to libexec/bin and libexec/man.
  • Codes simplified for directories with single file.

@CL-Jeremy
Copy link
Contributor

Actually, since it's solely a user binary, what could be the advantage of symlinks over an alias in the user's profile? I find aliasing more intuitive and transparent than exploiting the libexec folder for this use,

@sjackman
Copy link
Member

sjackman commented Feb 7, 2019

Thanks, @lembacon !

@CL-Jeremy Most users use only man, but there are in fact seven executables in bin:
apropos catman lexgrog man mandb manpath whatis

@lembacon lembacon closed this in de6cade Feb 8, 2019
@lembacon lembacon deleted the man-db-gnubin-gnuman branch February 8, 2019 06:21
@maxim-belkin
Copy link
Member

Noticed that findutils has bin, libexec/bin, and libexec/gnubin

bin/gupdatedb sets LC_ALL to C and calls libexec/bin/gupdatedb
libexec/gnubin/updatedb is a symlink to libexec/bin/gupdatedb

😮

@lock lock bot added the outdated PR was locked due to age label Mar 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2019
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.

5 participants