SpacetimeDB icon indicating copy to clipboard operation
SpacetimeDB copied to clipboard

Fix `spacetime version list` not showing current version

Open bfops opened this issue 7 months ago • 3 comments

Description of Changes

See https://github.com/clockworklabs/SpacetimeDB/issues/2679.

At some point, this code was changed so that current contains an entire path, rather than just the version string itself, so the comparison between ver and current was always failing.

I'm not strongly attached to this particular approach to fixing, if there's a more principled way or a better function I should be using.

API and ABI breaking changes

Not a breaking change.

Expected complexity level and risk

1

Testing

  • [x] It works now!
$ cargo run -pspacetimedb-update -- version list
   Compiling spacetimedb-update v1.1.1 (/home/lead/work/clockwork-localhd/SpacetimeDBPrivate/public/crates/update)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.63s
     Running `target/debug/spacetimedb-update version list`
1.1.1
1.1.0 (current)

I could be convinced to add a smoketest for this.

bfops avatar Apr 29 '25 17:04 bfops

Oh, whoops. I think the actual root cause is in a different function - this should be a thorough fix. It does mean that the current version will have to be re-set for it to work.

diff --git a/crates/paths/src/cli.rs b/crates/paths/src/cli.rs
index 28d26fb81..f39761c09 100644
--- a/crates/paths/src/cli.rs
+++ b/crates/paths/src/cli.rs
@@ -85,7 +85,7 @@ impl VersionBinDir {
     }
 
     fn link_to(&self, path: &Path) -> anyhow::Result<()> {
-        let rel_path = path.strip_prefix(self).unwrap_or(path);
+        let rel_path = path.strip_prefix(self.0.parent().unwrap()).unwrap_or(path);
         #[cfg(unix)]
         {
             // remove the link if it already exists

coolreader18 avatar Apr 29 '25 17:04 coolreader18

Oh, whoops. I think the actual root cause is in a different function - this should be a thorough fix. It does mean that the current version will have to be re-set for it to work.

diff --git a/crates/paths/src/cli.rs b/crates/paths/src/cli.rs
index 28d26fb81..f39761c09 100644
--- a/crates/paths/src/cli.rs
+++ b/crates/paths/src/cli.rs
@@ -85,7 +85,7 @@ impl VersionBinDir {
     }
 
     fn link_to(&self, path: &Path) -> anyhow::Result<()> {
-        let rel_path = path.strip_prefix(self).unwrap_or(path);
+        let rel_path = path.strip_prefix(self.0.parent().unwrap()).unwrap_or(path);
         #[cfg(unix)]
         {
             // remove the link if it already exists

Ah okay! thanks for the better fix. Do you have any suggestions for how we fix this for people who already have SpacetimeDB installed? I guess it would just get quietly fixed the next time they upgrade?

bfops avatar Apr 30 '25 20:04 bfops

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 03 '25 18:05 CLAassistant

I guess it would just get quietly fixed the next time they upgrade?

Yeah, that's the idea.

coolreader18 avatar May 16 '25 18:05 coolreader18