-
Notifications
You must be signed in to change notification settings - Fork 6
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
Initial implementation #1
Conversation
7e4c7e0
to
5abb219
Compare
5abb219
to
031348b
Compare
09593d8
to
2bb1c06
Compare
4819fdb
to
b80bdce
Compare
b80bdce
to
8c885da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so far this looks reasonable
Utf8PathBuf::from(&path) | ||
.file_name() | ||
.map(|s| s.strip_suffix(".exe").unwrap_or(s)) | ||
.map(|s| s.strip_suffix("-update").unwrap_or(s)) | ||
.map(|s| s.to_owned()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... this won't behave desirably for apps whose name doesn't match their primary binary's name, and which want to use this as a library.
For instance turso has a repo name "libsql" which ships an app named "libsql-server" whose only binary is "sqld", for legacy backcompat reasons.
https://github.com/tursodatabase/libsql/blob/main/libsql-server/Cargo.toml#L8
https://github.com/tursodatabase/libsql/releases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context, the "guess from filename" is intended for the standalone updater. That's why it's trying to strip -update
from the end of the filename. End consumers of this as a library shouldn't use this method.
src/lib.rs
Outdated
if cfg!(debug_assertions) { | ||
Ok(Utf8PathBuf::try_from(current_dir()?)?) | ||
} else { | ||
let Some(home) = homedir::get_my_home()? else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw this is the hardcore library for this stuff
it depends on a few os crates like redox_users and windows_sys, idk how much bloat that entails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started with dirs
, but had to back out because I couldn't write the receipts using it. dirs only lets you access dir info for the platform it's running on; you can't, say, ask for what Windows dirs should look like when you're running on Linux. As a result, I could only really assemble actual receipt paths in bash/powershell, where I didn't have access to dirs. Here I'm matching exactly the logic I used there, rather than dirs's special logic that might not match it 1:1.
} | ||
|
||
fn load_receipt_from_path(install_receipt_path: &Utf8PathBuf) -> AxoupdateResult<InstallReceipt> { | ||
Ok(SourceFile::load_local(install_receipt_path)?.deserialize_json()?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh nice, ok so we aren't being Super precious about deps (for now)
|
||
impl Release { | ||
pub fn version(&self) -> String { | ||
if let Some(stripped) = self.tag_name.strip_prefix('v') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh geez can we get both of these into the manifest. we specifically send both to axoreleases because getting one from the other is a mess (your code will die on my-app-v1.0.0
, my-app/v1.0.0
, releases/1.0.0
, ....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(really cargo-dist should be told what the schema is for tags so the updater can construct them... sigh)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it honestly would be a lot easier than going off the tag here.
Instead of branching behaviour based on debug assertions, check for environment variables and use those instead.
c38c2f5
to
e67a961
Compare
e43c1ea
to
0697862
Compare
Merging the impl for now; further changes will be smaller PRs from this. |
This features the initial implementation of axoupdater. This version is mainly targeted for use as a standalone executable, but it has most of the work necessary for a library that can be embedded in into another program.
The updater executable is designed to be installed with a filename which indicates what it's meant to update; it then infers what program to update from its filename. For example, if it's installed as
cargo-dist-update
, it'll attempt to update cargo-dist. The update process looks like this:This version contains testing debug features; it tries to load install receipts from the current directory instead of the standard config path, and hardcodes the name of the program to update as "cargo-dist". These testing features are disabled in release builds.
Some work that isn't done yet: