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

Version is not always updated when re-building from source #10855

Closed
rcorre opened this issue May 30, 2024 · 3 comments · Fixed by #10896
Closed

Version is not always updated when re-building from source #10855

rcorre opened this issue May 30, 2024 · 3 comments · Fixed by #10896
Labels
C-bug Category: This is a bug

Comments

@rcorre
Copy link
Contributor

rcorre commented May 30, 2024

Summary

I'm building helix from source, and noticed that the version reported by --version does not quite match the commit I'm building from.

[rrc@rrc-dev helix]$ git rev-parse HEAD
179673568df2a519fae2537fbc0053a64ecf3d8b
[rrc@rrc-dev helix]$ cargo install --path helix-term --locked
  Installing helix-term v24.3.0 (/home/rrc/src/helix/helix-term)
    Updating crates.io index
warning: package `libc v0.2.154` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked
    Finished release [optimized] target(s) in 0.49s
   Replacing /home/rrc/.cargo/bin/hx
    Replaced package `helix-term v24.3.0 (/home/rrc/src/helix/helix-term)` with `helix-term v24.3.0 (/home/rrc/src/helix/helix-term)` (executable `hx`)
[rrc@rrc-dev helix]$ which hx
/home/rrc/.cargo/bin/hx
[rrc@rrc-dev helix]$ hx --version
helix 24.3 (f656b4f3)

I do see some files for helix-loader referencing that hash:

[rrc@rrc-dev helix]$ rg f656b4f3 -uuu
target/release/hx: binary file matches (found "\0" byte around offset 7)

target/release/deps/helix_loader-a08374f82e769eec.d
14:# env-dep:VERSION_AND_GIT_HASH=24.3 (f656b4f3)

I believe what's happening is that helix-loader is what defines the version that helix-term imports, so if you pull changes that don't include helix-loader and rebuild, the version remains unchanged.

[rrc@rrc-dev helix]$ rg VERSION_AND_GIT_HASH
helix-loader/build.rs
36:    println!("cargo:rustc-env=VERSION_AND_GIT_HASH={}", version);

helix-loader/src/lib.rs
9:pub const VERSION_AND_GIT_HASH: &str = env!("VERSION_AND_GIT_HASH");

helix-term/src/main.rs
3:use helix_loader::VERSION_AND_GIT_HASH;
73:        VERSION_AND_GIT_HASH,
91:        println!("helix {}", VERSION_AND_GIT_HASH);

helix-lsp/src/client.rs
9:use helix_loader::VERSION_AND_GIT_HASH;
678:                version: Some(String::from(VERSION_AND_GIT_HASH)),

Reproduction Steps

I tried this:

  1. hx

I expected this to happen:

Instead, this happened:

Helix log

~/.cache/helix/helix.log
please provide a copy of `~/.cache/helix/helix.log` here if possible, you may need to redact some of the lines

Platform

Linux

Terminal Emulator

N/A

Installation Method

source

Helix Version

1796735

@rcorre rcorre added the C-bug Category: This is a bug label May 30, 2024
@dead10ck
Copy link
Member

dead10ck commented Jun 1, 2024

I'm not sure this matters very much. This should only happen on local cached builds, and if we "fixed" this so the commit hash was always accurate, then it would cause a ton of unnecessary compile times, since helix-loader sits at the top of the crate dependency tree, so everything underneath it has to get rebuilt too

@rcorre
Copy link
Contributor Author

rcorre commented Jun 1, 2024

That's fair. My thinking was that "local cached builds" would be common for those who regularly stay up to date with main, and would also represent helix's most active user base in terms of trying new features and reporting bugs, which could lead to confusing bug reports.

It's also a bit confusing as a user, I spent a while installing and uninstalling helix, thinking cargo install wasn't actually updating my binary.

@pascalkuthe
Copy link
Member

we already instruct cargo to rebuild if the HEAD commit changes but I think there is a bug in the logic

rcorre added a commit to rcorre/helix that referenced this issue Jun 6, 2024
Fixes helix-editor#10855.

The loader build.rs tries to determine when HEAD changes,
so it can re-run and update the embedded version.

It was shelling out to do this, and capturing a trailing newline,
so it always thought HEAD did not exist.

I confirmed that if I update helix-term, helix-loader rebuilds and updates the commit hash:

```
[rcorre@midgar helix]$ git rev-parse --short HEAD
e24eed2c
[rcorre@midgar helix]$ hx --version
helix 24.3 (e24eed2c)
[rcorre@midgar helix]$ hx helix-term/src/main.rs
[rcorre@midgar helix]$ git cma "test changing helix-term"
[fix-version cbb68bab] test changing helix-term
 1 file changed, 1 insertion(+)
[rcorre@midgar helix]$ cargo install --path helix-term --locked
  Installing helix-term v24.3.0 (/home/rcorre/src/helix/helix-term)
    Updating crates.io index
   Compiling helix-loader v24.3.0 (/home/rcorre/src/helix/helix-loader)
   Compiling helix-term v24.3.0 (/home/rcorre/src/helix/helix-term)
   ... snip ...
[rcorre@midgar helix]$ git rev-parse --short HEAD
cbb68bab
[rcorre@midgar helix]$ hx --version
helix 24.3 (cbb68bab)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants