volta icon indicating copy to clipboard operation
volta copied to clipboard

`volta list` output is wrong for locally-installed tools

Open chriskrycho opened this issue 5 years ago • 5 comments

In a project with a given tool installed locally (in this case, ember-cli)—

  • Expected:

    $ volta list
    
    ⚡️ Currently active tools:
    
        Node: v12.16.2 (current @ /Users/chris/dev/test/foo/package.json)
        Yarn: v1.22.4 (current @ /Users/chris/dev/test/foo/package.json)
        Tool binaries available:
            ember (current @ /Users/chris/dev/test/foo/package.json)
    
  • Actual:

    $ volta list
    
    ⚡️ Currently active tools:
    
        Node: v12.16.2 (current @ /Users/chris/dev/test/foo/package.json)
        Yarn: v1.22.4 (current @ /Users/chris/dev/test/foo/package.json)
        Tool binaries available:
            ember (default)
    

Additionally, we currently report the wrong platform values for these. In the extended form (volta list all):

This correctly reports the Node version (though without the current @ ... from the design), but not the correct package manager version.

This affects both the --format=plain and the --format=human variants, which is expected: the underlying logic for both is identical.

chriskrycho avatar Apr 16 '20 14:04 chriskrycho

The 2nd part of this is potentially tricky for a couple of reasons: First, we don't have an obvious way to determine the version of a local binary: It would be equally wrong to show [email protected] (current @ ...) when the local project is actually using [email protected]. The only way I can think of to determine that would be to dig into node_modules/ember-cli/package.json and read the version from there (we can't use the project's package.json because it could be a semver range).

Additionally, the logic for determining the platform that a binary will use is currently bespoke logic inside of volta_core::run::binary, so in order to duplicate it we would probably want to refactor it out into somewhere reusable. That would actually simplify a few other things (like the volta run implementation), so it's worth doing, but it's not a super simple change.

charlespierce avatar Apr 17 '20 19:04 charlespierce

I was thinking about that first part, too, but hadn't worked out the details on the second part yet. That's tricky indeed. 🤔

Right now, we're printing something that's actually out-and-out wrong, so I'm wondering if we shouldn't tackle this in two phases:

  1. Don't print the platform or version info for it at all for local binaries; just print ember-cli (current @ ...) or something. That gives the user enough info to introspect the version manually, while not printing erroneous information.

  2. Design a solution which correctly handles the bits to go ahead and supply that data correctly after landing volta run.

Thoughts? cc. @rwjblue @dherman

chriskrycho avatar Apr 20 '20 14:04 chriskrycho

I missed this comment when the issue first came up, but I agree. Since we don't have a way forward yet on showing the correct version, we should at the very least prevent showing incorrect information for now.

charlespierce avatar May 11 '20 23:05 charlespierce

I had a thought on how to determine the version: We could display the value from package.json directly, including any semver ranges. So if they have "typescript": "^3.0.0", we would show typescript@^3.0.0, which is valid npm-style syntax and is accurate (though not specific). For determining the platform we can definitely do that, it'll just take some refactoring on the run::binary module to abstract out that logic.

charlespierce avatar May 20 '20 00:05 charlespierce

Out of curiosity, why isn't it okay to simply rummage through to node_modules/ember-cli/package.json to grab the version? Afai can see, that's likely the only method that's feasible...

TheBrenny avatar Aug 05 '24 00:08 TheBrenny