r-lib / pak

A fresh approach to package installation
https://pak.r-lib.org
682 stars 62 forks source link

pak local install overwrites newer package version #662

Open strengejacke opened 4 months ago

strengejacke commented 4 months ago

Steps to reproduce the issue:

  1. Install latest package version from a (local) repository (in this case, the performance package)
  2. You have a dependency (package insight), which is installed from a pull request and has the "latest" version number.
  3. Run "Ctrl+Shift+I" in Positron. insight is overwritten by an older version (i.e. an older version is being installed)

What did you expect to happen?

a newer installed package is overwritten by an older version by pak:

insight     0.20.1.14 → 0.20.1.13 [bld][cmp] (GitHub: c4a33df)

I would expect that a package is not installed when it has an "older" version number.

gaborcsardi commented 4 months ago

Run "Ctrl+Shift+I" in Positron

That does not do anything for me.

I cannot reproduce this:

❯ pak::pkg_install("r-lib/cli")
✔ Loading metadata database ... done

→ Will update 1 package.
→ The package (0 B) is cached.
+ cli 3.6.3 → 3.6.3.9000 👷🏿‍♀️🔧 (GitHub: d9febb5)
? Do you want to continue (Y/n)
ℹ No downloads are needed, 1 pkg is cached
✔ Got cli 3.6.3.9000 (source) (768.96 kB)
ℹ Packaging cli 3.6.3.9000
✔ Packaged cli 3.6.3.9000 (2.6s)
ℹ Building cli 3.6.3.9000
✔ Built cli 3.6.3.9000 (5.8s)
✔ Installed cli 3.6.3.9000 (github::r-lib/cli@d9febb5) (53ms)
✔ 1 pkg: upd 1, dld 1 (NA B) [16.1s]
❯ pak::pkg_install("local::.")

→ Will update 1 package.
→ The package (0 B) is cached.
+ rcmdcheck 1.4.0 → 1.4.0.9000 👷🏿‍♀️
? Do you want to continue (Y/n)
ℹ No downloads are needed, 1 pkg is cached
✔ Got rcmdcheck 1.4.0.9000 (source) (96 B)
ℹ Packaging rcmdcheck 1.4.0.9000
✔ Packaged rcmdcheck 1.4.0.9000 (540ms)
ℹ Building rcmdcheck 1.4.0.9000
✔ Built rcmdcheck 1.4.0.9000 (1.4s)
✔ Installed rcmdcheck 1.4.0.9000 (local) (24ms)
✔ 1 pkg + 14 deps: kept 14, upd 1, dld 1 (NA B) [4.5s]
gaborcsardi commented 4 months ago

Is this what you are installing locally? https://github.com/easystats/performance/blob/4b39cbf2723d8bcfdd1ccc3629ff7773843357cc/DESCRIPTION#L157

Remotes: easystats/insight

means that you want insight from GitHub, that last commit of the main branch. So that's what pak is installing.

I understand that this is not ideal for local development, maybe pak should have another mode where local installs are accepted (although it is not easy to tell what is a local install).

strengejacke commented 4 months ago

Ah, ok. That could explain it.

Yes, I had this version installed, where I was locally working on: https://github.com/easystats/insight/pull/901

Than I wanted to install performance locally (the version from main, https://github.com/easystats/performance/), which downloaded and installed insight from main, despite having a lower version number.

Maybe for pak::local_install(), you could also cross-check for version numbers, if a REMOTES field is present?

gaborcsardi commented 4 months ago

Version numbers do not matter for a package from GitHub. Most people don't increase the version number for every commit, and it is impossible to have linear version numbers for multiple branches.

You can remove the Remotes field for local development, after all it is not accurate in your case.

strengejacke commented 4 months ago

after all it is not accurate in your case.

What do you mean by this?

gaborcsardi commented 4 months ago

You have

Remotes: easystats/insight

in DESCRIPTION, meaning that you want insight from GitHub. But you don't actually want to install insight from GitHub, do you?

strengejacke commented 4 months ago

Yes, that's intended. insight is imported by performance and includes new features that I want to test for performance on GitHub