-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[New] Parallel script download jobs in install script #1479
Conversation
2b174cb
to
681e412
Compare
} & | ||
for job in $(jobs -p | sort) | ||
do | ||
wait "$job" || return $? |
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've not had good luck with wait
reliably waiting on all parallel commands :-/
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.
Really? How do you say that?
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 tried to use it to speed up the installation jobs, so travis wouldn't time out, and it never seemed to have the desired effect.
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.
Do you have any example? I can take a look at it, I would say that the various on Travis CI is too high, much more higher than the time we can save here, but for an end-user, it's pretty good.
install.sh
Outdated
chmod a+x "$INSTALL_DIR/nvm-exec" || { | ||
echo >&2 "Failed to mark '$INSTALL_DIR/nvm-exec' as executable" | ||
return 3 | ||
} | ||
} & |
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.
changing privs on nvm-exec
must not happen until nvm-exec
is downloaded - so this can't be parallelized; in other words, the wait
would need to happen before the chmod
.
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.
Oops, my bad here.
681e412
to
a678d52
Compare
Updated! |
nvm_download -s "$NVM_EXEC_SOURCE" -o "$INSTALL_DIR/nvm-exec" || { | ||
echo >&2 "Failed to download '$NVM_EXEC_SOURCE'" | ||
return 2 | ||
} | ||
} & | ||
for job in $(jobs -p | sort) |
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.
will this include only the two previous jobs? or could user jobs show up here too?
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.
@ljharb since install.sh
won't be executed by source but a single bash process, so it'll only show the process we forked to background only, right?
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.
That sounds reasonable; that's what I'm asking :-)
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 don't think installation really needs to be fast, but this seems good.
a678d52
to
ec3ba19
Compare
Faster is better 👍 |
No description provided.