-
-
Notifications
You must be signed in to change notification settings - Fork 11.3k
Change brew linkapps
to use the global /Applications folder
#22378
Conversation
TARGET_DIR = ARGV.include?("--system") ? "/Applications" : File.expand_path("~/Applications") | ||
|
||
unless File.exist? TARGET_DIR | ||
opoo "#{TARGET_DIR} does not exist, stopping." |
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.
For a local link, this existence check still needs to be done.
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.
I'll fix that up and push a new commit.
I'm going to push the commit(s) from #22380 here, and leave some time for them to be reviewed in-full; then I'll squash and rebase on top of the latest master after a sign-off from one of you maintainers, so it's a one-click merge. |
On Mavericks we need to explicitly include AvailabilityMacros.h to have the version detection macros defined. Fix 10.9 ruby framework detection and compilation Mavericks ships with version 1.8 and 2.0 of the Ruby.framework, so we must directly link the framework version matching the ruby-command. This also means that ruby.h must no longer included via the framework name, which has the nice side effect of allowing you to compile with non-framework rubies, if you remove the formula's hardcoded RUBY_PATH.
@BrewTestBot test this please |
Looks fine; go ahead and add another commit that moves this from contributions to the main cmd folder too. |
@adamv is there any other changes necessary in the codebase to support that? I'm not familiar with how these are loaded. Is it entirely dynamic (i.e. just a I've pushed that change, let me know if I missed something. Edit: Yeah, there's clearly more required. Maybe it's better than somebody else make this change, as I'm fairly swamped (unless it's something simple that I'm missing here):
|
Ah, right, there's a difference in form. If you squash the original 4 commits into 1 or 2 commits, I can pull those then move it myself. Otherwise, look at the files in |
This reverts commit d42410c97d91487ba2230952f3731c3ee48446a5.
(as per discussion on Homebrew#22378.)
This reverts commit d366942.
Okay, @adamv. I've rebased everything on top of the current master, and then squashed the relevant commits into a single one for you to cherry-pick: |
Also adds support for a new `--local` flag, and documentation for the whole command. Closes Homebrew#22378. Signed-off-by: Adam Vandenberg <flangy@gmail.com>
(As per @adamv's request, re-requesting this pull, here. I'll keep this rebased as necessary as the discussion continues.)
This has been heavily discussed elsewhere. I'll simply re-list the reasons for this change, here:
Also, it seems to be the majority request, here. At an informal reading of #8699 and #17643 (and please, forgive me, or let me know, if I mis-read your intentions, posters), the following are for the global
/Applications
being the default, with the user-~/Applications
being an optional flag: @hced, @dysinger, @zhouyan, @mxcl, @thecontrarian, @iainbeeston, @demure, and @vilhalmer. As far as I can tell, the only person categorically against it, is @adamv.)'=
Other relevant Issues / Pull Requests: #12619, #18196.
Further commentary welcome. Also, if any of my changes to the code are unacceptable, please let me know;
CONTRIBUTING.md
was rather unhelpful to someone modifying Homebrew itself, ironically. (=(I'll be forthcoming with additional patches, not dependent on this one, that respectively add documentation of this feature, since there seems to be none [#22380]; and another controversial one wherein I suggest that we move apps, not symlink them, into whichever folder we decide upon here [#22379].)