Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add options to keep the build directory around #143

Merged
merged 5 commits into from

3 participants

@raggi

This is useful when using gdb and memprof on OSX.

@raggi

Refrences #108

@trans trans referenced this pull request
Closed

Where's the source? #142

@jeremy
Collaborator

I like being able to specify & keep the build dir, but not sure about making it such a first-class feature: third arg to ruby-build (rather than an opt or env var) and prominent --keep option on rbenv install itself.

@raggi

Really, you're so strong on keeping features down, so I wouldn't expect it to clobber much.

I'll happily shave it down to just the env var if you prefer.

@raggi

@jeremy as per your request, let me know if you'd like that squashed too. I also rebased on top of current master.

See summary diff, it's super tiny. If you want a squash, I'll fix the line end change too (sorry, vimism).

@sstephenson
Owner

@raggi, thanks for your work here so far.

I spoke with @jeremy about this patch and we both have a couple of reservations.

First, it feels awkward to couple preservation of the build directory with the option to place the build directory in a location of your choosing. What I'm thinking is that we should offer an environment variable (name TBD) to override the default build directory, and a command-line flag (-k/--keep) to keep the build directory around after installation. These are separate concerns.

Second, it does make sense to distill the common use case into a single command-line flag. The problem with putting this flag in the ruby-build command is that ruby-build makes no assumptions about the organization of your filesystem. But as an rbenv user, if I want to keep the source around, it makes sense to do so in a directory tree parallel to ~/.rbenv/versions. So perhaps rbenv-install can have a separate option that invokes ruby-build with --keep and sets the build directory environment variable to a well-chosen value.

@raggi

Interestingly, this was basically how it was implemented the first time. I'll rewrite it again.

@raggi

Branch rewritten following the above. I have split each of the two use cases from each of the two paragraphs above into individual commits. The summary diff is still relatively short, but the addition of options to ruby-build did make it slightly longer than the prior two variations.

If there are further changes required, I would be happy to make them.

@raggi

Is this likely to get merged, or are you unhappy with the size now?

@jeremy jeremy commented on the diff
bin/ruby-build
((5 lines not shown))
RUBY_BIN="${PREFIX_PATH}/bin/ruby"
CWD="$(pwd)"
+if [ -z $RUBY_BUILD_BUILD_PATH ]; then
+ BUILD_PATH="${TMP}/ruby-build.${SEED}"
+else
+ BUILD_PATH=$RUBY_BUILD_BUILD_PATH
+fi
@jeremy Collaborator
jeremy added a note

Can use a default var here:
BUILD_PATH=${RUBY_BUILD_BUILD_PATH:-${TMP}/ruby-build.${SEED}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeremy
Collaborator

Looking good to me. /cc @sstephenson

@sstephenson
Owner

Beautiful patch @raggi (and sorry I missed this earlier). Thank you!

@sstephenson sstephenson merged commit f68c8f5 into from
@raggi

Thanks guys, and sorry if I was pushy! :)

@jeremy
Collaborator

Patience incarnate ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 40 additions and 8 deletions.
  1. +8 −0 README.md
  2. +12 −2 bin/rbenv-install
  3. +20 −6 bin/ruby-build
View
8 README.md
@@ -53,6 +53,14 @@ ruby-build provides an `rbenv-install` command that shortens this to:
$ rbenv install 1.9.2-p290
+ruby-build supports $RUBY_BUILD_BUILD_PATH to override the location in which
+sources are downloaded and built. The -k/--keep flags will preserve this path
+after the build is complete.
+
+rbenv-install also supports the -k/--keep flag, and additionally supports an
+environment variable option $RBENV_BUILD_ROOT that when set, will always build
+sources under that location, and keep the sources after build completion.
+
### Version History
#### 20120423
View
14 bin/rbenv-install
@@ -23,10 +23,20 @@ case "$DEFINITION" in
} >&2
exit 1
;;
+"-k" | "--keep" )
+ [ -z "${RBENV_BUILD_ROOT}" ] && RBENV_BUILD_ROOT="${RBENV_ROOT}/srcs"
+ RUBY_BUILD_OPTIONS+=" -k"
+ ;;
esac
VERSION_NAME="${DEFINITION##*/}"
PREFIX="${RBENV_ROOT}/versions/${VERSION_NAME}"
-ruby-build "$DEFINITION" "$PREFIX"
-rbenv rehash
+# If RBENV_BUILD_ROOT is set, then always pass keep options to ruby-build
+if [ -n "${RBENV_BUILD_ROOT}" ]; then
+ export RUBY_BUILD_BUILD_PATH="${RBENV_BUILD_ROOT}/${VERSION_NAME}"
+ RUBY_BUILD_OPTIONS+=" -k"
+fi
+
+ruby-build "$DEFINITION" "$PREFIX" "$RUBY_BUILD_OPTIONS"
+rbenv rehash
View
26 bin/ruby-build
@@ -28,8 +28,8 @@ build_failed() {
echo "BUILD FAILED"
echo
- if ! rmdir "${TEMP_PATH}" 2>/dev/null; then
- echo "Inspect or clean up the working tree at ${TEMP_PATH}"
+ if ! rmdir "${BUILD_PATH}" 2>/dev/null; then
+ echo "Inspect or clean up the working tree at ${BUILD_PATH}"
if file_is_not_empty "$LOG_PATH"; then
echo "Results logged to ${LOG_PATH}"
@@ -68,7 +68,7 @@ install_package_using() {
local package_name="$3"
shift 3
- pushd "$TEMP_PATH" >&4
+ pushd "$BUILD_PATH" >&4
"fetch_${package_type}" "$package_name" $*
shift $(($package_type_nargs))
make_package "$package_name" $*
@@ -396,6 +396,15 @@ if [ -z "$PREFIX_PATH" ]; then
usage
fi
+OPTIONS="$3"
+for option in $OPTIONS; do
+ case "$option" in
+ "-k" | "--keep" )
+ KEEP_BUILD_PATH="y"
+ ;;
+ esac
+done
+
if [ -z "$TMPDIR" ]; then
TMP="/tmp"
else
@@ -404,10 +413,15 @@ fi
SEED="$(date "+%Y%m%d%H%M%S").$$"
LOG_PATH="${TMP}/ruby-build.${SEED}.log"
-TEMP_PATH="${TMP}/ruby-build.${SEED}"
RUBY_BIN="${PREFIX_PATH}/bin/ruby"
CWD="$(pwd)"
+if [ -z $RUBY_BUILD_BUILD_PATH ]; then
+ BUILD_PATH="${TMP}/ruby-build.${SEED}"
+else
+ BUILD_PATH=$RUBY_BUILD_BUILD_PATH
+fi
@jeremy Collaborator
jeremy added a note

Can use a default var here:
BUILD_PATH=${RUBY_BUILD_BUILD_PATH:-${TMP}/ruby-build.${SEED}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
exec 4<> "$LOG_PATH" # open the log file at fd 4
if [ -n "$VERBOSE" ]; then
tail -f "$LOG_PATH" &
@@ -421,7 +435,7 @@ unset RUBYOPT
unset RUBYLIB
trap build_failed ERR
-mkdir -p "$TEMP_PATH"
+mkdir -p "$BUILD_PATH"
source "$DEFINITION_PATH"
-rm -fr "$TEMP_PATH"
+[ -z "${KEEP_BUILD_PATH}" ] && rm -fr "$BUILD_PATH"
trap - ERR
Something went wrong with that request. Please try again.