This repository has been archived by the owner. It is now read-only.

Migrate external commands to internal #29326

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

adamv commented May 16, 2014

No description provided.

Contributor

jacknagel commented May 17, 2014

I'm going to migrate brew unpack to core, and possibly port brew man to ruby.

Contributor

adamv commented May 20, 2014

Something needs to be done about the git/svn wrappers that live in Contributions/cmd and are symlinked to from ENV.

Contributor

jacknagel commented May 20, 2014

I don't particularly care where they live, but they need to be on PATH during normal execution.

Contributor

adamv commented May 20, 2014

Definitely don't want to put all of ENV/version in the path, so this will require a some thought.

@adamv adamv added the contrib label May 20, 2014

Contributor

jacknagel commented May 20, 2014

For some background, these were originally added to support Xcode-only installs where the Xcode provided git might not be on PATH.

Later links were added to superenv because of Homebrew#15783 (tl;dr older git might choke on config settings that newer git supports, so try to use the git that the user expects).

If the idea is to eventually remove Contributions/cmd altogether, then the easiest solution would be to add a new directory somewhere that we always put on PATH and stick these scripts in it.

A slightly more involved solution would be to move them to superenv and solve the original problem with internal code.

Contributor

adamv commented May 21, 2014

Any thoughts on which and switch? From bug reports I know they get used, but they're in the same category as versions.

Contributor

jacknagel commented May 21, 2014

switch should be cleaned up and made internal, which can probably go though.

Contributor

adamv commented May 27, 2014

Moved the wrappers; open to suggestion there.

Contributor

jacknagel commented May 28, 2014

Extracted the bits of brew versions that we use for bottling, so that command is ready to be moved to contrib or whatever the plan is.

Contributor

jacknagel commented Jun 20, 2014

I don't actually know what brew which does.

Owner

MikeMcQuaid commented Jun 20, 2014

Looks good to me.

Owner

MikeMcQuaid commented Jun 20, 2014

Moved versions in Homebrew#30298.

Contributor

adamv commented Jun 20, 2014

which looks at what is symlinked to the prefix and figures out what the current version is. This was well before opt and was probably mostly useful for working around bugs in the linking process.

Recommend removal at this point.

Contributor

adamv commented Jul 11, 2014

gist-logs probably needs to go internal

@adamv adamv commented on the diff Jul 11, 2014

Library/brew.rb
@@ -107,8 +114,10 @@ def require? path
end
end
- # Add contributed commands to PATH before checking.
- ENV['PATH'] += "#{File::PATH_SEPARATOR}#{HOMEBREW_CONTRIB}/cmd"
+ # Add the wrapper directory to PATH
+ ENV["PATH"] += "#{File::PATH_SEPARATOR}#{HOMEBREW_REPOSITORY}/Library/ENV/wrapper"
+ # Add contributed commands to PATH
+ ENV["PATH"] += "#{File::PATH_SEPARATOR}#{HOMEBREW_CONTRIB}/cmd"
internal_cmd = require? HOMEBREW_LIBRARY_PATH.join("cmd", cmd) if cmd
@adamv

adamv Jul 11, 2014

Contributor

Need to extract a variable for HOMEBREW_LIBRARY_PATH.join("cmd", cmd) rather than do it 3 times.

@adamv adamv commented on an outdated diff Jul 11, 2014

Library/Homebrew/cmd/pull.rb
@@ -0,0 +1,131 @@
+# Gets a patch from a GitHub commit or pull request and applies it to Homebrew.
+# Optionally, installs it too.
+
+require 'utils'
+require 'formula'
+
+module Homebrew extend self
@adamv

adamv Jul 11, 2014

Contributor

Remove extend self.

@adamv adamv commented on an outdated diff Jul 11, 2014

Library/Homebrew/cmd/pull.rb
@@ -0,0 +1,131 @@
+# Gets a patch from a GitHub commit or pull request and applies it to Homebrew.
@adamv

adamv Jul 11, 2014

Contributor

Need to redo this move against master, since the original brew-pull was changed.

Owner

MikeMcQuaid commented Jul 11, 2014

Great work here so far 👍

@adamv adamv closed this Sep 21, 2014

@adamv adamv deleted the unknown repository branch Sep 21, 2014

Owner

MikeMcQuaid commented Sep 21, 2014

@adamv are you ok with my version in #32472?

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 17, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.