xpack / xpm-js

The xPack Project Manager command line tool
https://xpack.github.io/xpm-js/
MIT License
31 stars 7 forks source link

`xpm uninstall --global` fails if package version is `@latest` #202

Open TommyMurphyTM1234 opened 3 months ago

TommyMurphyTM1234 commented 3 months ago

Describe the bug

xpm uninstall --global fails with @latest even when the specified package is installed.

To Reproduce

I did this on Windows 10:

mkdir test
cd test

xpm init

xpm install @xpack-dev-tools/riscv-none-elf-gcc@latest --verbose
...

xpm uninstall @xpack-dev-tools/riscv-none-elf-gcc@latest --verbose
xPack manager - uninstall package(s)

Symlink 'xpacks/@xpack-dev-tools/riscv-none-elf-gcc' removed
package.json devDependencies['@xpack-dev-tools/riscv-none-elf-gcc'] removed

'xpm uninstall' completed in 336 ms.

xpm list --global
- @xpack-dev-tools/riscv-none-elf-gcc
  A binary xPack with the GNU RISC-V Embedded GCC executables
  - 14.1.0-1.1

xpm uninstall @xpack-dev-tools/riscv-none-elf-gcc@latest --verbose --global
xPack manager - uninstall package(s)

error: global package '@xpack-dev-tools/riscv-none-elf-gcc@latest' not installed
exit(2)

xpm uninstall @xpack-dev-tools/riscv-none-elf-gcc@14.1.0-1.1 --verbose --global
xPack manager - uninstall package(s)

Changing permissions to read-write...
'C:\Users\Tommy Murphy\AppData\Roaming\xPacks\@xpack-dev-tools\riscv-none-elf-gcc\14.1.0-1.1' removed

'xpm uninstall' completed in 6.274 sec.

Expected behaviour

I would have expected the xpm uninstall using @latest as the version to work.

Actual behaviour

xpm uninstall with a package version of @latest fails but it works if I give it a specific version number.

Context

npm doctor
Connecting to the registry
Ok

Checking npm version
Not ok
Use npm v10.8.2

Checking node version
Ok
current: v20.16.0, recommended: v20.16.0

Checking configured npm registry
Ok
using default registry (https://registry.npmjs.org/)

Checking for git executable in PATH
Ok
C:\Program Files\Git\cmd\git.EXE

Checking for global bin folder in PATH
Ok
C:\Users\Tommy Murphy\AppData\Roaming\npm

npm error Some problems found. See above for recommendations.
npm error A complete log of this run can be found in: C:\Users\Tommy Murphy\AppData\Local\npm-cache\_logs\2024-07-30T20_44_41_562Z-debug-0.log

Comments

None.

ilg-ul commented 3 months ago

Although it seems obvious that the install and uninstall must be consistent, under the bonnet things are a bit more complicated.

xpm has no understanding of what latest means. xpm (as npm does too) asks the server to get the version tagged with latest (or any tag in fact), and the server returns the actual version.

At uninstall time, which may be long after install, this information not only that it is no longer available, but asking the server for the meaning of latest is wrong, since in the mean time a different version may have been tagged as latest.

So, strictly speaking, uninstall @latest cannot be implemented.

What can be done is to cheat and compute latest at uninstall time as the greatest version, according to semver.

For normal npm packages which use a strict semver string, this should work.

Unfortunately binary xpacks use an extended semver, with additional numbers defined as pre-releases. If the package that provides semver services is able to sort such strings, we probably have an acceptable solution.

TommyMurphyTM1234 commented 3 months ago

What can be done is to cheat and compute latest at uninstall time as the greatest version, according to semver.

Wouldn't that make sense given that that seems to be what happens when working within an xPack project directory using xpm uninstall ... @latest without --global?

Or else just make xpm uninstall require an explicit (fully qualified?) version number and reject @latest altogether?

Either way, what happens within a project directory and what happens when using --global should ideally be consistent.

ilg-ul commented 3 months ago

I guess that xpm uninstall in a project completely ignores the version, since there can be no multiple versions of the same package installed in a project.

At the global level, if multiple versions are installed, the version is mandatory. Or should be, I don't know how the code behaves now.

TommyMurphyTM1234 commented 3 months ago

I guess that xpm uninstall in a project completely ignores the version, since there can be no multiple versions of the same package installed in a project.

Ah - I didn't realise that even though it makes sense! :-)

In that case maybe it would make more sense if...

  1. "Local" xpm uninstall errors out if any version, including @latest, is specified?
  2. "Global" xpm uninstall ... --global errors out if a version is not specified or @latest is specified?
ilg-ul commented 3 months ago

It makes sense. I'll improve the validation and error processing.

TommyMurphyTM1234 commented 3 months ago

I also noticed this - is it deliberate?


xpm list --global
- @xpack-dev-tools/riscv-none-elf-gcc
  A binary xPack with the GNU RISC-V Embedded GCC executables
  - 13.3.0-1.1
  - 14.1.0-1.1

mkdir test
cd test

xpm init

# No version specified but works the same as if @latest or @14.1.0-1.1 were specified
xpm install @xpack-dev-tools/riscv-none-elf-gcc --verbose

xpm list

- @xpack-dev-tools/riscv-none-elf-gcc@14.1.0-1.1
  A binary xPack with the GNU RISC-V Embedded GCC executables
...
ilg-ul commented 3 months ago

On 31 Jul 2024, at 00:46, Tommy Murphy @.***> wrote:

No version specified but works the same as if @latest or @14.1.0-1.1 were specified

xpm install @xpack-dev-tools/riscv-none-elf-gcc --verbose

that's correct, the default is @latest.

however I prefer to add it explicitly in the documentation.

TommyMurphyTM1234 commented 3 months ago

On 31 Jul 2024, at 00:46, Tommy Murphy @.***> wrote: # No version specified but works the same as if @latest or @14.1.0-1.1 were specified xpm install @xpack-dev-tools/riscv-none-elf-gcc --verbose that's correct, the default is @latest.

But not when I do xpm uninstall @xpack-dev-tools/riscv-none-elf-gcc --verbose --global. In that case it removes ALL versions of @xpack-dev-tools/riscv-none-elf-gcc from the central store.

I'm my opinion the handling of version specifiers could/should ideally be more consistent.

ilg-ul commented 3 months ago

You're right. What would be the ideal consistent behaviour for both install and uninstall?

TommyMurphyTM1234 commented 3 months ago

You're right. What would be the ideal consistent behaviour for both install and uninstall?

I need to review the current behaviour of project based and global package installs/uninstalls more systematically and then think about this.

TommyMurphyTM1234 commented 3 months ago

You're right. What would be the ideal consistent behaviour for both install and uninstall?

I need to review the current behaviour of project based and global package installs/uninstalls more systematically and then think about this.

As far as I can see...

(An "xPack enabled project directory" is one in which xpm init was previously run).

Global

  1. xpm install ... --global with no version specifier is equivalent to @latest.
  2. xpm uninstall ... --global with no version specifier uninstalls all versions of the specified package.
  3. xpm uninstall ... --global with @latest gives an error (e.g. error: global package '@xpack-dev-tools/openocd@latest' not installed).
  4. So, xpm uninstall ... --global needs either no version specifier or a specific version specifier but not @latest.

xPack enabled project directory

  1. xpm install without --global in an xPack enabled project directory with no version installs the latest version available in the central store.
  2. xpm uninstall without --global in an xPack enabled project directory ignores any version specifier.

Am I missing anything here? Are there also version pattern matching options that can be used?

ilg-ul commented 3 months ago

Am I missing anything here?

For completeness you can add the cases with @latest in xPack enabled project, but they are obvious.

Are there also version pattern matching options that can be used?

Not for the moment.

Anyways, the xpm behaviour should be consistent to npm, if possible.

(BTW, you can install xPacks with npm too, but the binary archive is not automatically unpacked in the .content folder, it needs to be done by a separate mechanism).