Skip to content
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

Working with nvm, nvmw and similar installation managers #40

Closed
smikes opened this issue Dec 3, 2014 · 44 comments
Closed

Working with nvm, nvmw and similar installation managers #40

smikes opened this issue Dec 3, 2014 · 44 comments
Milestone

Comments

@smikes
Copy link
Contributor

smikes commented Dec 3, 2014

This is intended as a placeholder and heads-up.

In the transitional period (while io.js is gaining acceptance, node has a huge install base, and people are sorting out which binary they are going to use), package authors will need to test against both io.js and node. Making io.js easy to install via tools like nvm would ease that.

nvm maintainer @ljharb says this here ( nvm-sh/nvm#590 (comment) )

If the community starts moving in a major way towards any given fork of node, and if that fork provides the same installation methods that node does (binaries and source), then this project should be able to easily support it, and I'd intend to do so.

So that's a good goal.

@jaydson
Copy link

jaydson commented Dec 3, 2014

👍 for this one.

@arb
Copy link

arb commented Dec 3, 2014

To that same effect, I would like to see version switching (of io.js) build directly into the run time. Needing a separate tool to version switch is just added overhead. I'm pretty sure anyone doing work on node/io.js has either nvm or n already installed so making one pack-in solution seems like a good idea to me.

@gkatsev
Copy link

gkatsev commented Dec 3, 2014

Not sure we'd want to have such a thing built into core. Perhaps the binaries can ship with a specific tool, a kin to how npm ships with it.

@Fishrock123
Copy link
Contributor

See also: node-forward/discussions#6

@therebelrobot
Copy link

👍 to this entire discussion

@ljharb
Copy link
Member

ljharb commented Dec 3, 2014

Please note that this may apply to environment variables such as $NODE_PATH, although that's an easier thing to handle, as well as installation methods and relative file paths within the installation directory.

For another +1, if nvm adds iojs support, then travis-ci gets it too with no extra work on their end.

@kenperkins
Copy link
Contributor

@arb What's the motivation for adding versioning complexity directly to core itself? I find nvm works great.

@kenperkins
Copy link
Contributor

Also +1 to nvm, n, etc support.

@arb
Copy link

arb commented Dec 3, 2014

If the versioning is built in, deployments and authoring would probably become easier you would have consistency with how to change versions across different environments.

My main thought was that it's just a tedious chore to ALWAYS have to set up some kind of versioning manager. We all basically need it anyway, why not build it in/ship the binary with it already included like npm.

@sergiolepore
Copy link

Awesome! 👍

@smikes
Copy link
Contributor Author

smikes commented Dec 3, 2014

There are two topics here:

  1. making io.js work with nvm, nvmw, etc..
  2. including one (or more) of nvm, n etc. with official distribution packages of io.js.

I am in favor of both of those, but I think we should talk about them separately. Item 1 will be useful for testing early releases -- such as io.js@1.0.0-a1. Item 2 is more like a post-1.0.0 feature from what I understand of the io.js roadmap. (Item 2 also offers more opportunity for bikeshedding...)

I propose that discussion of item 1 stays here and discussion of item 2 goes to node-forward/discussions#6 (which I was not aware of when I created this issue, even though I commented on that issue a month ago...)

@ljharb Can I assume you don't mind me starting to dig around in nvm to put together a PR for the io.js repositories, once they are established?

@ljharb
Copy link
Member

ljharb commented Dec 3, 2014

@smikes Not at all, it's appreciated! Multiple smaller PRs preferred, and I'll want to follow the cowpath I've built for node 0.12 and later (the "versions" subdirectory) to make sure that iojs versions end up in a subdirectory as well - but we can bikeshed these things on the nvm repo.

Currently nvm lists versions by scraping the nodejs.org website, which is also where it downloads binaries. Will there be a reliable source for listing iojs versions, and also downloading binaries?

@kenperkins
Copy link
Contributor

Maybe we can expose an api as a function of the build system on iojs.org?

@gkatsev
Copy link

gkatsev commented Dec 3, 2014

@ljharb I think there will be a better listing, eventually: https://github.com/iojs/build

@smikes
Copy link
Contributor Author

smikes commented Dec 3, 2014

Will there be a reliable source for listing iojs versions, and also downloading binaries?

This is exactly the sort of thing I want to discuss here. No idea if planned, sounds like a good idea though. ;-)

Since iojs is planning to follow semver, there's that, at least.

@bnoordhuis
Copy link
Member

Will there be a reliable source for listing iojs versions, and also downloading binaries?

I think that can be arranged. Do you need something like a .txt with file paths or would a base URL + predictable path work (e.g. https://iojs.org/download/x.y.z/iojs-x.y.z.tar.gz)? I guess you'd still need a list of released versions.

@smikes
Copy link
Contributor Author

smikes commented Dec 3, 2014

I would like to have two resources at static locations --

  1. a text file which documents all available releases
  2. a text file which identifies the latest version, or (if there are multiple release streams eg stable, canary) the latest in each release stream

Item 2 is useful in a brew doctor type context, where I want to do a quick request and let the user know if an upgrade is available, while Item 1 is what a fully-capable tool like nvm wants.

Since nvm is a shell script, simpler is better. A parseable list of each tar.gz would be ideal. Notionally:

$ curl https://iojs.org/download/all-versions.txt
https://iojs.org/download/1.0.0-a1/iojs-1.0.0-a1.tar.gz
https://iojs.org/download/1.0.0-a2/iojs-1.0.0-a2.tar.gz
https://iojs.org/download/1.0.0/iojs-1.0.0.tar.gz
$ curl https://iojs.org/download/latest-versions.txt
stable: https://iojs.org/download/1.0.0/iojs-1.0.0.tar.gz
latest: https://iojs.org/download/1.0.0/iojs-1.0.0.tar.gz
canary: https://iojs.org/download/1.0.0/iojs-1.1.0.tar.gz

Do those sound like reasonable URLs? https://iojs.org/download/all-versions.txt and https://iojs.org/download/latest-versions.txt

@ljharb
Copy link
Member

ljharb commented Dec 3, 2014

What @smikes is suggesting would work perfectly, especially because it's easily curlable and parseable.

@alexgorbatchev
Copy link

may I suggest the following:

$ curl https://iojs.org/download/all-versions.txt
1.0.0-a1: https://iojs.org/download/1.0.0-a1/iojs-1.0.0-a1.tar.gz
1.0.0-a2: https://iojs.org/download/1.0.0-a2/iojs-1.0.0-a2.tar.gz
1.0.0: https://iojs.org/download/1.0.0/iojs-1.0.0.tar.gz

@SomeoneWeird
Copy link
Member

What about a json blob with a latest key?

@keithamus
Copy link

What about replicating/emulating NPM's JSON structure? Seems to work well for them. Something like:

{
    "dist-tags": {
        "latest": "1.0.0",
        "beta": "1.1.0-beta"
    },
    "versions": {
        "1.0.0": {
            "dist": {
                "shasum": "2f246aeae428f07feaceb8d39a2e3870a978c9f8",
                "tarball": "http://iojs.io/download/1.0.0.tgz"
            }
        },
        "1.1.0-beta": {
            "dist": {
                "shasum": "910e639e7f523e8fa28047307aecfd89fe6a55a7",
                "tarball": "http://iojs.io/download/1.1.0-beta.tgz"
            }
        }
    }
}

@ljharb
Copy link
Member

ljharb commented Dec 3, 2014

JSON is much nicer but much more difficult to parse on a shell. I'm all for a JSON option but please let's also provide a straight plain text option?

@alexgorbatchev's modification to all-versions.txt is better 👍

@smikes
Copy link
Contributor Author

smikes commented Dec 3, 2014

JSON is much nicer but much more difficult to parse on a shell. I'm all for a JSON option but please let's also provide a straight plain text option?

The challenge of a version manager is it has to work when there is no node installed. That's bad enough with nvm running under a POSIX-y shell, but think how hard it must be for nvmw who have to do it under CMD.EXE or (if lucky) powershell.

Plain text for now. JSON for 1.1 or 2.0

@alexgorbatchev's modification to all-versions.txt is better 👍

Also 👍 to version: url

@bnoordhuis bnoordhuis added this to the v1.0.0 milestone Dec 6, 2014
@bnoordhuis
Copy link
Member

Nominating for the v1.0.0 milestone. I suggest a simple line-based $version $url because it's easy to parse.

/cc @Fishrock123 - I expect that the list will have to be updated manually for now. Ideally, it's auto-generated but it's unclear to me how that would work. A cron job running on dist.iojs.org? Generated by the CI when releasing? TBI.

@cjihrig cjihrig mentioned this issue Dec 6, 2014
@naholyr
Copy link
Contributor

naholyr commented Dec 6, 2014

How does nvm work for node already? Would not the best way to make life easier for @ljharb be to just provide the exact same files, same paths, same formats, with just a different domain name?

@ljharb
Copy link
Member

ljharb commented Dec 6, 2014

That would indeed make it insanely trivial for me to add iojs support to nvm. However, the current method for nvm ls remote of scraping a webpage and parsing stuff out of it is atrocious, and I'd love to move away from that.

@naholyr
Copy link
Contributor

naholyr commented Dec 6, 2014

Oh OK didn't know that ;) then 👍 for @alexgorbatchev 's version: url very easy to parse. May I propose = instead of : as separator? It would then become simple ini data, which is fast & safe & easy to parse in any language.

elprans pushed a commit to elprans/node that referenced this issue Apr 13, 2019
targos added a commit to targos/node that referenced this issue Apr 17, 2021
Original commit message:

    Merged: [codegen] Skip invalid optimization in tail calls

    Preparing for tail call is usually done by emitting the gap moves and
    then moving the stack pointer to its new position. An optimization
    consists in moving the stack pointer first and transforming some of the
    moves into pushes. In the attached case it looks like this (arm):

    138  add sp, sp, nodejs#40
    13c  str r6, [sp, #-4]!
    140  str r6, [sp, #-4]!
    144  str r6, [sp, #-4]!
    148  str r6, [sp, #-4]!
    14c  str r6, [sp, #-4]!
    ...
    160  vldr d1, [sp - 4*3]

    The last line is a gap reload, but because the stack pointer was already
    moved, the slot is now below the stack pointer. This is invalid and
    triggers this DCHECK:

    Fatal error in ../../v8/src/codegen/arm/assembler-arm.cc, line 402
    Debug check failed: 0 <= offset (0 vs. -12).

    A comment already explains that we skip the optimization if the gap
    contains stack moves to prevent this, but the code only checks for
    non-FP slots. This is fixed by replacing "source.IsStackSlot()" with
    "source.IsAnyStackSlot()":

    108  vldr d1, [sp + 4*2]
    ...
    118  str r0, [sp, #+36]
    11c  str r0, [sp, #+32]
    120  str r0, [sp, #+28]
    124  str r0, [sp, #+24]
    128  str r0, [sp, #+20]
    ...
    134  add sp, sp, nodejs#20

    TBR=​jgruber@chromium.org

    (cherry picked from commit 7506e063d0d7fb00e4b9c06735c91e1953296867)

    Change-Id: I66ed6187755af956e245207e940c83ea0697a5e6
    Bug: chromium:1137608
    No-Try: true
    No-Presubmit: true
    No-Tree-Checks: true
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2505976
    Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
    Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
    Cr-Commit-Position: refs/branch-heads/8.6@{nodejs#42}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@8c725f7
targos added a commit to targos/node that referenced this issue Apr 17, 2021
Original commit message:

    Merged: [runtime] Fix sorted order of DescriptorArray entries

    Revision: 518d67ad652fc24b7eb03e48bb342f952d4ccf74

    This is a reland of the previous merge which addresses the cctest link
    failure in component build mode.

    BUG=chromium:1133527
    NOTRY=true
    NOPRESUBMIT=true
    NOTREECHECKS=true
    R=verwaest@chromium.org

    Change-Id: Icbbc69fd5403fd0c2ab6d07d4340292b2b8c72b9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504264
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/branch-heads/8.6@{nodejs#40}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@1a7d55a
targos added a commit to targos/node that referenced this issue Apr 27, 2021
Original commit message:

    Merged: [codegen] Skip invalid optimization in tail calls

    Preparing for tail call is usually done by emitting the gap moves and
    then moving the stack pointer to its new position. An optimization
    consists in moving the stack pointer first and transforming some of the
    moves into pushes. In the attached case it looks like this (arm):

    138  add sp, sp, nodejs#40
    13c  str r6, [sp, #-4]!
    140  str r6, [sp, #-4]!
    144  str r6, [sp, #-4]!
    148  str r6, [sp, #-4]!
    14c  str r6, [sp, #-4]!
    ...
    160  vldr d1, [sp - 4*3]

    The last line is a gap reload, but because the stack pointer was already
    moved, the slot is now below the stack pointer. This is invalid and
    triggers this DCHECK:

    Fatal error in ../../v8/src/codegen/arm/assembler-arm.cc, line 402
    Debug check failed: 0 <= offset (0 vs. -12).

    A comment already explains that we skip the optimization if the gap
    contains stack moves to prevent this, but the code only checks for
    non-FP slots. This is fixed by replacing "source.IsStackSlot()" with
    "source.IsAnyStackSlot()":

    108  vldr d1, [sp + 4*2]
    ...
    118  str r0, [sp, #+36]
    11c  str r0, [sp, #+32]
    120  str r0, [sp, #+28]
    124  str r0, [sp, #+24]
    128  str r0, [sp, #+20]
    ...
    134  add sp, sp, nodejs#20

    TBR=​jgruber@chromium.org

    (cherry picked from commit 7506e063d0d7fb00e4b9c06735c91e1953296867)

    Change-Id: I66ed6187755af956e245207e940c83ea0697a5e6
    Bug: chromium:1137608
    No-Try: true
    No-Presubmit: true
    No-Tree-Checks: true
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2505976
    Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
    Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
    Cr-Commit-Position: refs/branch-heads/8.6@{nodejs#42}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@8c725f7
targos added a commit to targos/node that referenced this issue Apr 27, 2021
Original commit message:

    Merged: [runtime] Fix sorted order of DescriptorArray entries

    Revision: 518d67ad652fc24b7eb03e48bb342f952d4ccf74

    This is a reland of the previous merge which addresses the cctest link
    failure in component build mode.

    BUG=chromium:1133527
    NOTRY=true
    NOPRESUBMIT=true
    NOTREECHECKS=true
    R=verwaest@chromium.org

    Change-Id: Icbbc69fd5403fd0c2ab6d07d4340292b2b8c72b9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504264
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/branch-heads/8.6@{nodejs#40}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@1a7d55a
targos added a commit that referenced this issue Apr 30, 2021
Original commit message:

    Merged: [codegen] Skip invalid optimization in tail calls

    Preparing for tail call is usually done by emitting the gap moves and
    then moving the stack pointer to its new position. An optimization
    consists in moving the stack pointer first and transforming some of the
    moves into pushes. In the attached case it looks like this (arm):

    138  add sp, sp, #40
    13c  str r6, [sp, #-4]!
    140  str r6, [sp, #-4]!
    144  str r6, [sp, #-4]!
    148  str r6, [sp, #-4]!
    14c  str r6, [sp, #-4]!
    ...
    160  vldr d1, [sp - 4*3]

    The last line is a gap reload, but because the stack pointer was already
    moved, the slot is now below the stack pointer. This is invalid and
    triggers this DCHECK:

    Fatal error in ../../v8/src/codegen/arm/assembler-arm.cc, line 402
    Debug check failed: 0 <= offset (0 vs. -12).

    A comment already explains that we skip the optimization if the gap
    contains stack moves to prevent this, but the code only checks for
    non-FP slots. This is fixed by replacing "source.IsStackSlot()" with
    "source.IsAnyStackSlot()":

    108  vldr d1, [sp + 4*2]
    ...
    118  str r0, [sp, #+36]
    11c  str r0, [sp, #+32]
    120  str r0, [sp, #+28]
    124  str r0, [sp, #+24]
    128  str r0, [sp, #+20]
    ...
    134  add sp, sp, #20

    TBR=​jgruber@chromium.org

    (cherry picked from commit 7506e063d0d7fb00e4b9c06735c91e1953296867)

    Change-Id: I66ed6187755af956e245207e940c83ea0697a5e6
    Bug: chromium:1137608
    No-Try: true
    No-Presubmit: true
    No-Tree-Checks: true
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2505976
    Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
    Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
    Cr-Commit-Position: refs/branch-heads/8.6@{#42}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@8c725f7

PR-URL: #38275
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
targos added a commit that referenced this issue Apr 30, 2021
Original commit message:

    Merged: [runtime] Fix sorted order of DescriptorArray entries

    Revision: 518d67ad652fc24b7eb03e48bb342f952d4ccf74

    This is a reland of the previous merge which addresses the cctest link
    failure in component build mode.

    BUG=chromium:1133527
    NOTRY=true
    NOPRESUBMIT=true
    NOTREECHECKS=true
    R=verwaest@chromium.org

    Change-Id: Icbbc69fd5403fd0c2ab6d07d4340292b2b8c72b9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504264
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/branch-heads/8.6@{#40}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@1a7d55a

PR-URL: #38275
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests