Skip to content

Commit

Permalink
merge: #2656
Browse files Browse the repository at this point in the history
2656: fix(si): ensure new binary is in same directory as old before rename r=fnichol a=fnichol

This change fixes an issue which will arise when a user's tempdir is on a different volume mount than the filesystem mount that contains the current binary. An attempt to use `fs:rename` (i.e. an atomic move) will span a filesystem boundary and is often not successful.

A different strategy moves/copies the new binary into the same directory as the current one, applies the same permissions as the current binary, and then calls `fs:rename` to move the new binary and replace the old one. As both files are on the same filesystem, this results in the desired atomic move.

Thankfully, the [self-replace
crate](https://crates.io/crates/self-replace) performs these exact activities, so we'll use this!

Closes #2651

Co-authored-by: Fletcher Nichol <[email protected]>
  • Loading branch information
si-bors-ng[bot] and fnichol authored Aug 19, 2023
2 parents 120e4a2 + 0f788b9 commit ac833bf
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 46 deletions.
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ diff = "0.1.13"
directories = "5.0.1"
docker-api = "0.14"
dyn-clone = "1.0.11"
flate2 = "1.0.26"
futures = "0.3.28"
futures-lite = "1.13.0"
hex = "0.4.3"
Expand All @@ -84,8 +85,8 @@ nats = { version = "0.24.0" }
nix = "0.26.2"
nkeys = "0.2.0"
num_cpus = "1.15.0"
open = "5.0.0"
once_cell = "1.17.1"
open = "5.0.0"
opentelemetry = { version = "~0.18.0", features = ["rt-tokio", "trace"] } # pinned, pending new release of tracing-opentelemetry, 0.18
opentelemetry-otlp = "~0.11.0" # pinned, pending new release of tracing-opentelemetry, post 0.18
opentelemetry-semantic-conventions = "~0.10.0" # pinned, pending new release of tracing-opentelemetry, post 0.18
Expand All @@ -105,10 +106,10 @@ remain = "0.2.8"
reqwest = { version = "0.11.17", default-features = false, features = ["rustls-tls", "json", "multipart"] }
rust-s3 = { version = "0.33.0", default-features = false, features = ["tokio-rustls-tls"] }
sea-orm = { version = "0.11", features = ["sqlx-postgres", "runtime-tokio-rustls", "macros", "with-chrono", "debug-print"]}
self-replace = "1.3.5"
serde = { version = "1.0.160", features = ["derive", "rc"] }
serde-aux = "4.2.0"
serde_json = { version = "1.0.96", features = ["preserve_order"] }
toml = { version = "0.7.6" }
serde_url_params = "0.2.1"
serde_with = "3.0.0"
serde_yaml = "0.9.21"
Expand All @@ -127,6 +128,7 @@ tokio-stream = "0.1.14"
tokio-test = "0.4.2"
tokio-tungstenite = "0.18.0"
tokio-util = { version = "0.7.8", features = ["codec"] }
toml = { version = "0.7.6" }
tower = "0.4.13"
tower-http = { version = "0.4.0", features = ["cors", "trace"] }
tracing = { version = "0.1" }
Expand All @@ -137,7 +139,6 @@ url = { version = "2.3.1", features = ["serde"] }
uuid = { version = "1.3.2", features = ["serde", "v4"] }
vfs = "0.9.0"
vfs-tar = { version = "0.4.0", features = ["mmap"] }
flate2 = "1.0.26"

[patch.crates-io]
# pending a potential merge and release of
Expand Down
2 changes: 1 addition & 1 deletion bin/si/src/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20230731231120
20190703.164251.0-sha.021b4563a
4 changes: 2 additions & 2 deletions lib/dal/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,13 @@ impl Func {
let mut new_unique_name;
loop {
new_unique_name = format!("{}{}", self.name(), generate_unique_id(4));
if Self::find_by_name(&ctx, &new_unique_name).await?.is_none() {
if Self::find_by_name(ctx, &new_unique_name).await?.is_none() {
break;
};
}

let mut new_func = Self::new(
&ctx,
ctx,
new_unique_name,
*self.backend_kind(),
*self.backend_response_type(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ use dal::{
SchemaVariantDefinition, SchemaVariantDefinitionError as DalSchemaVariantDefinitionError,
SchemaVariantDefinitionId,
},
DalContext, Func, FuncBackendKind, FuncBackendResponseType, FuncError, FuncId, Schema,
SchemaError, StandardModel, Visibility, WsEvent,
Func, Schema, SchemaError, StandardModel, Visibility, WsEvent,
};
use serde::{Deserialize, Serialize};
use std::env::var;

#[derive(Deserialize, Serialize, Debug)]
#[serde(rename_all = "camelCase")]
Expand Down
19 changes: 10 additions & 9 deletions lib/si-cli/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ load("@prelude-si//:macros.bzl", "rust_library")
rust_library(
name = "si-cli",
deps = [
"//lib/si-posthog-rs:si-posthog",
"//lib/telemetry-rs:telemetry",
"//third-party/rust:axum",
"//third-party/rust:base64",
"//third-party/rust:color-eyre",
Expand All @@ -11,24 +13,23 @@ rust_library(
"//third-party/rust:console",
"//third-party/rust:directories",
"//third-party/rust:docker-api",
"//third-party/rust:flate2",
"//third-party/rust:futures",
"//third-party/rust:indicatif",
"//third-party/rust:inquire",
"//third-party/rust:open",
"//third-party/rust:rand",
"//third-party/rust:remain",
"//third-party/rust:tar",
"//third-party/rust:flate2",
"//third-party/rust:serde",
"//third-party/rust:tokio",
"//third-party/rust:toml",
"//third-party/rust:reqwest",
"//third-party/rust:self-replace",
"//third-party/rust:serde",
"//third-party/rust:serde_json",
"//third-party/rust:sodiumoxide",
"//third-party/rust:thiserror",
"//third-party/rust:tar",
"//third-party/rust:tempfile",
"//lib/si-posthog-rs:si-posthog",
"//lib/telemetry-rs:telemetry",
"//third-party/rust:serde_json",
"//third-party/rust:thiserror",
"//third-party/rust:tokio",
"//third-party/rust:toml",
],
srcs = glob([
"src/**/*.rs",
Expand Down
17 changes: 9 additions & 8 deletions lib/si-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,27 @@ publish = false
axum = { workspace = true }
base64 = { workspace = true }
color-eyre = { workspace = true }
colored = { workspace = true }
comfy-table = { workspace = true }
console = { workspace = true }
directories = { workspace = true }
docker-api = { workspace = true }
flate2 = { workspace = true }
futures = { workspace = true }
indicatif = { workspace = true }
inquire = { workspace = true }
open = { workspace = true }
rand = { workspace = true }
remain = { workspace = true }
reqwest = { workspace = true }
self-replace = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
si-posthog = { path = "../../lib/si-posthog-rs" }
sodiumoxide = { workspace = true }
tar = { workspace = true }
telemetry = { path = "../../lib/telemetry-rs" }
tempfile = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true }
toml = { workspace = true }
serde_json = { workspace = true }
serde = { workspace = true }
sodiumoxide = { workspace = true }
reqwest = { workspace = true }
tempfile = { workspace = true }
colored = { workspace = true }
tar = { workspace = true }
flate2 = { workspace = true }
34 changes: 17 additions & 17 deletions lib/si-cli/src/cmd/update.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use crate::containers::DockerClient;
use crate::key_management::get_user_email;
use crate::state::AppState;
use crate::{CliResult, SiCliError};
use colored::Colorize;
use flate2::read::GzDecoder;
use inquire::Confirm;
use serde::{Deserialize, Serialize};
use std::fs;
use std::io::Cursor;
use std::{env, io::Cursor};
use telemetry::prelude::*;

use crate::{
containers::DockerClient, key_management::get_user_email, state::AppState, CliResult,
SiCliError,
};

#[derive(Deserialize, Debug, Clone)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -48,14 +49,6 @@ pub struct Update {
static HOST: &str = "https://auth-api.systeminit.com";

async fn update_current_binary(url: &str) -> CliResult<()> {
let current_exe = std::env::current_exe()?;

let exe_path = if current_exe.is_symlink() {
fs::read_link(current_exe)?
} else {
current_exe
};

let req = reqwest::get(url).await?;
if req.status().as_u16() != 200 {
println!(
Expand All @@ -80,11 +73,18 @@ async fn update_current_binary(url: &str) -> CliResult<()> {
})
.await??;

println!("Overwriting current binary");
tokio::fs::rename(tempdir.path().join("si"), exe_path).await?;
let current_exe = env::current_exe()?;
let new_binary = tempdir.path().join("si");

println!("Replacing '{}' with new binary", current_exe.display());
tokio::task::spawn_blocking(move || self_replace::self_replace(new_binary)).await??;

println!("Binary updated!");

if let Err(err) = tokio::task::spawn_blocking(move || tempdir.close()).await? {
debug!(err = ?err, "tempdir failed to successfully delete");
}

Ok(())
}

Expand Down Expand Up @@ -192,7 +192,7 @@ async fn invoke(
}

if let Some(update) = &update.si {
let version = &update.version;
let version = update.version.split('/').last().unwrap_or(&update.version);
println!("Launcher update found: from {current_version} to {version}",);
}

Expand Down
42 changes: 42 additions & 0 deletions third-party/rust/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -11633,6 +11633,44 @@ cargo.rust_library(
],
)

alias(
name = "self-replace",
actual = ":self-replace-1.3.5",
visibility = ["PUBLIC"],
)

http_archive(
name = "self-replace-1.3.5.crate",
sha256 = "a0e7c919783db74b5995f13506069227e4721d388bea4a8ac3055acac864ac16",
strip_prefix = "self-replace-1.3.5",
urls = ["https://crates.io/api/v1/crates/self-replace/1.3.5/download"],
visibility = [],
)

cargo.rust_library(
name = "self-replace-1.3.5",
srcs = [":self-replace-1.3.5.crate"],
crate = "self_replace",
crate_root = "self-replace-1.3.5.crate/src/lib.rs",
edition = "2018",
platform = {
"windows-gnu": dict(
deps = [
":fastrand-1.9.0",
":windows-sys-0.48.0",
],
),
"windows-msvc": dict(
deps = [
":fastrand-1.9.0",
":windows-sys-0.48.0",
],
),
},
visibility = [],
deps = [":tempfile-3.6.0"],
)

alias(
name = "serde",
actual = ":serde-1.0.164",
Expand Down Expand Up @@ -13436,6 +13474,7 @@ cargo.rust_binary(
":reqwest-0.11.18",
":rust-s3-0.33.0",
":sea-orm-0.11.3",
":self-replace-1.3.5",
":serde-1.0.164",
":serde-aux-4.2.0",
":serde_json-1.0.97",
Expand Down Expand Up @@ -15694,7 +15733,10 @@ cargo.rust_library(
"Win32_System_Console",
"Win32_System_Diagnostics",
"Win32_System_Diagnostics_Debug",
"Win32_System_Environment",
"Win32_System_IO",
"Win32_System_LibraryLoader",
"Win32_System_Memory",
"Win32_System_Pipes",
"Win32_System_SystemServices",
"Win32_System_Threading",
Expand Down
12 changes: 12 additions & 0 deletions third-party/rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions third-party/rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ diff = "0.1.13"
directories = "5.0.1"
docker-api = "0.14"
dyn-clone = "1.0.11"
flate2 = "1.0.26"
futures = "0.3.28"
futures-lite = "1.13.0"
futures-util = "0.3"
Expand All @@ -61,8 +62,8 @@ nats = { version = "0.24.0" }
nix = "0.26.2"
nkeys = "0.2.0"
num_cpus = "1.15.0"
open = "5.0.0"
once_cell = "1.17.1"
open = "5.0.0"
opentelemetry = { version = "~0.18.0", features = ["rt-tokio", "trace"] } # pinned, pending new release of tracing-opentelemetry, 0.18
opentelemetry-otlp = "~0.11.0" # pinned, pending new release of tracing-opentelemetry, post 0.18
opentelemetry-semantic-conventions = "~0.10.0" # pinned, pending new release of tracing-opentelemetry, post 0.18
Expand All @@ -82,10 +83,10 @@ remain = "0.2.8"
reqwest = { version = "0.11.17", default-features = false, features = ["rustls-tls", "json", "multipart"] }
rust-s3 = { version = "0.33.0", default-features = false, features = ["tokio-rustls-tls"] }
sea-orm = { version = "0.11", features = ["sqlx-postgres", "runtime-tokio-rustls", "macros", "with-chrono", "debug-print"]}
self-replace = "1.3.5"
serde = { version = "1.0.160", features = ["derive", "rc"] }
serde-aux = "4.2.0"
serde_json = { version = "1.0.96", features = ["preserve_order"] }
toml = { version = "0.7.6" }
serde_url_params = "0.2.1"
serde_with = "3.0.0"
serde_yaml = "0.9.21"
Expand All @@ -104,6 +105,7 @@ tokio-stream = "0.1.14"
tokio-test = "0.4.2"
tokio-tungstenite = "0.18.0"
tokio-util = { version = "0.7.8", features = ["codec"] }
toml = { version = "0.7.6" }
tower = "0.4.13"
tower-http = { version = "0.4.0", features = ["cors", "trace"] }
tracing = { version = "0.1" }
Expand All @@ -114,7 +116,6 @@ url = { version = "2.3.1", features = ["serde"] }
uuid = { version = "1.3.2", features = ["serde", "v4"] }
vfs = "0.9.0"
vfs-tar = { version = "0.4.0", features = ["mmap"] }
flate2 = "1.0.26"

# Local patches - typically Git references
[patch.crates-io]
Expand Down

0 comments on commit ac833bf

Please sign in to comment.