Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions nvm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1231,6 +1231,32 @@ nvm_install_node_source() {
make='gmake'
MAKE_CXX="CXX=c++"
fi

if [ -z "$NVM_MAKE_JOBS" ]; then
if [ "_$NVM_OS" = "_linux" ]; then
NVM_CPU_THREADS="$(grep -c 'core id' /proc/cpuinfo)"
Copy link
Member

Choose a reason for hiding this comment

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

command grep, in case it's aliased?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I'm just letting it pass the test first 😄

elif [ "_$NVM_OS" = "_freebsd" ] || [ "_$NVM_OS" = "_darwin" ]; then
NVM_CPU_THREADS="$(sysctl -n hw.ncpu)"
elif [ "_$NVM_OS" = "_sunos" ]; then
NVM_CPU_THREADS="$(psrinfo | wc -l)"
fi
local NVM_CPU_THREAD_VALID
NVM_CPU_THREAD_VALID=$(nvm_is_natural_num $NVM_CPU_THREADS)
if [ -z "$NVM_CPU_THREADS" ] || [ "$NVM_CPU_THREAD_VALID" != "true" ] ; then
echo "Can not determine how many thread(s) we can use, set to only 1 now." 1>&2
echo "Please report an issue on GitHub to help us make it better and run it faster on your computer!" 1>&2
Copy link
Member

Choose a reason for hiding this comment

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

If you want to encourage people to include nvm debug output in this error message (and then update nvm debug to output NVM_OS and NVM_ARCH), that would help the error reports be more useful :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No problem.

NVM_MAKE_JOBS="1"
else
echo "Detected that you have $NVM_CPU_THREADS CPU thread(s)"
if [ $NVM_CPU_THREADS -gt 2 ]; then
NVM_MAKE_JOBS=$(($NVM_CPU_THREADS - 1))
echo "Set the number of jobs to $NVM_CPU_THREADS - 1 = $NVM_MAKE_JOBS jobs to speed up the build"
else
NVM_MAKE_JOBS=1
echo "Number of CPU thread(s) less or equal to 2 will have only one job a time for 'make'"
fi
fi
fi
local tmpdir
tmpdir="$NVM_DIR/src"
local tmptarball
Expand All @@ -1252,9 +1278,9 @@ nvm_install_node_source() {
command tar -xzf "$tmptarball" -C "$tmpdir" && \
cd "$tmpdir/node-$VERSION" && \
./configure --prefix="$VERSION_PATH" $ADDITIONAL_PARAMETERS && \
$make $MAKE_CXX && \
$make -j $NVM_MAKE_JOBS $MAKE_CXX && \
command rm -f "$VERSION_PATH" 2>/dev/null && \
$make $MAKE_CXX install
$make -j $NVM_MAKE_JOBS $MAKE_CXX install
)
then
if ! nvm_has "npm" ; then
Expand Down Expand Up @@ -1430,6 +1456,16 @@ nvm_sanitize_path() {
echo "$SANITIZED_PATH" | command sed "s#$HOME#\$HOME#g"
}

nvm_is_natural_num() {
echo $1 | command egrep -q '^[0-9]{1,}$' &> /dev/null
Copy link
Member

Choose a reason for hiding this comment

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

this incorrectly returns "true" for an input of 0 (i'll fix in a followup)

local IS_NATURAL_NUM=$?
if [ "$IS_NATURAL_NUM" = "0" ]; then
echo true
else
echo false
fi
}

nvm() {
if [ $# -lt 1 ]; then
nvm help
Expand Down Expand Up @@ -1545,6 +1581,21 @@ nvm() {
if [ "_$1" = "_-s" ]; then
nobinary=1
shift
if [ "_$1" = "_-j" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

however, this -j only is processed under -s - shouldn't it be parsed regardless, even if it's ignored when installing from a binary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So do you need me to detect -j somewhere else?

Copy link
Member

Choose a reason for hiding this comment

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

-j -s should probably error out, as should -j $not_a_number - i think it should be checked along with -s, and ideally in any order.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh okay

shift
Copy link
Member

Choose a reason for hiding this comment

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

why the double shift?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The second is for the comming provided_version="$1"

Copy link
Member

Choose a reason for hiding this comment

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

right but the first one takes care of that, doesn't it?

at any rate, tests would make this clear one way or the other :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Of course, you are correct, I'm starting to write test :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

-j has a following parameter, so it has two shift, -s doesn't, isn't it correct?

Copy link
Member

Choose a reason for hiding this comment

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

aha, that is correct! good call

local NVM_CPU_THREAD_VALID
NVM_CPU_THREAD_VALID=$(nvm_is_natural_num $1)
if [ "$NVM_CPU_THREAD_VALID" = "true" ]; then
NVM_MAKE_JOBS=$1
Copy link
Member

Choose a reason for hiding this comment

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

can we output something to stdout here indicating what the number of threads will be (for the user to confirm they entered it right)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure

echo "Set number of jobs to $MAKE_JOBS for 'make' utility"
Copy link
Member

Choose a reason for hiding this comment

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

this var name is incorrect (i'll fix in a followup)

else
unset NVM_MAKE_JOBS
echo >&2 "$1 is invalid for CPU threads, should be a natural number"
fi
shift
else
unset NVM_MAKE_JOBS
fi
fi

provided_version="$1"
Expand Down
33 changes: 33 additions & 0 deletions test/installation/node/install from source with thread parameter
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/bin/sh

die () { echo $@ ; exit 1; }

. ../../../nvm.sh

NVM_TEST_VERSION=v0.10.41

# STAGE 1 #

# Remove the stuff we're clobbering.
[ -e ../../../$NVM_TEST_VERSION ] && rm -R ../../../$NVM_TEST_VERSION

# Install from source with 1 CPU thread parameter
nvm install -s -j 1 $NVM_TEST_VERSION || die "'nvm install -s $NVM_TEST_VERSION' failed"

# Check
[ -d ../../../$NVM_TEST_VERSION ]
nvm run $NVM_TEST_VERSION --version | grep $NVM_TEST_VERSION || "'nvm run $NVM_TEST_VERSION --version | grep $NVM_TEST_VERSION' failed"



# STAGE 2 #

# Remove the stuff we're clobbering.
[ -e ../../../$NVM_TEST_VERSION ] && rm -R ../../../$NVM_TEST_VERSION

# Install from source with 2 CPU threads parameter
nvm install -s -j 2 $NVM_TEST_VERSION || die "'nvm install -s $NVM_TEST_VERSION' failed"

# Check
[ -d ../../../$NVM_TEST_VERSION ]
nvm run $NVM_TEST_VERSION --version | grep $NVM_TEST_VERSION || "'nvm run $NVM_TEST_VERSION --version | grep $NVM_TEST_VERSION' failed"