Restore mirror capability #210
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| @dochang Here's the restoration of the original mirror feature. If you are able, we need to first get the tests passing in this branch. I'm 99% sure the code is correct and that it's the tests that aren't correct. PRs welcome (opened against this branch) to fix the tests so that they pass. Once the tests are green, we'll change functionality so that it doesn't rely on ruby's mirror_host/checksum format. Cool? |
| tests green! time to fix formulation of mirror URL |
jasonkarns force-pushed the restore-mirror-capability branch from b1e3b83 to 9c6d847 Compare This reverts commit 22c52e0.
This reverts commit 08b8507.
Some changes were made to the mirror tests in upstream during the time that node-build didn't have mirror support, thus those upstream changes were missed. This replays those changes manually to the mirror tests.
An rbenv mirror has a customized repository layout: ruby tarball's url format is like ``` $RUBY_BUILD_MIRROR_URL/$checksum ``` not like ``` $RUBY_BUILD_MIRROR_URL/2.3/ruby-2.3.1.tar.bz2 ``` Common node mirrors just use the same URL structure as nodejs.org (though typically with the /dist path segment removed). Subsequent PRs will need to make the mirror url formulation more robust. see #208 (comment)
Not all mirrors will use the identical scheme of matching the nodejs.org url structure with `/dist` omitted. Now users may specify `NODE_BUILD_MIRROR_CMD` to be the name of a command or function which constructs the full mirrored package URL when given the original package URL and checksum as arguments. If omitted, the default scheme is used wherein NODE_BUILD_MIRROR_URL is substituted for `https://nodejs.org/dist/` The presence of *either* NODE_BUILD_MIRROR_URL or NODE_BUILD_MIRROR_CMD is sufficient to trigger a mirror attempt.
jasonkarns force-pushed the restore-mirror-capability branch from f46f7e2 to c73fbe2 Compare | @dochang would you be able to verify whether this PR works for you? Also, could you share all of the nodejs mirrors (and their respective URL structures) that you're aware of? |
| It works here, Thank you! Please merge it! Here are all mirrors I found: |
dochang added a commit to dochang/dotfiles that referenced this pull request Dec 13, 2016
Because nodenv supports NODE_BUILD_MIRROR_URL now [1], [2]. [1]: nodenv/node-build#208 [2]: nodenv/node-build#210
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
node-build will not provide a default mirror, but this PR at least restores the ability to select a mirror from which to download packages.
closes #208