Skip to content

Commit 14ea462

Browse files
authored
Add fs::MTime newtype to encourage != instead of > (#20830)
See ["mtime comparison considered harmful"](https://apenwarr.ca/log/20181113) for details of why comparators other than equality/inequality should not be used with mtime. Release Notes: - N/A
1 parent 477c6e6 commit 14ea462

File tree

15 files changed

+155
-112
lines changed

15 files changed

+155
-112
lines changed

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/assistant/src/context_store.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ impl ContextStore {
770770
contexts.push(SavedContextMetadata {
771771
title: title.to_string(),
772772
path,
773-
mtime: metadata.mtime.into(),
773+
mtime: metadata.mtime.timestamp_for_user().into(),
774774
});
775775
}
776776
}

crates/copilot/src/copilot.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1231,7 +1231,7 @@ mod tests {
12311231

12321232
fn disk_state(&self) -> language::DiskState {
12331233
language::DiskState::Present {
1234-
mtime: std::time::UNIX_EPOCH,
1234+
mtime: ::fs::MTime::from_seconds_and_nanos(100, 42),
12351235
}
12361236
}
12371237

crates/editor/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ emojis.workspace = true
4242
file_icons.workspace = true
4343
futures.workspace = true
4444
fuzzy.workspace = true
45+
fs.workspace = true
4546
git.workspace = true
4647
gpui.workspace = true
4748
http_client.workspace = true

crates/editor/src/items.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,15 +1618,14 @@ fn path_for_file<'a>(
16181618
#[cfg(test)]
16191619
mod tests {
16201620
use crate::editor_tests::init_test;
1621+
use fs::Fs;
16211622

16221623
use super::*;
1624+
use fs::MTime;
16231625
use gpui::{AppContext, VisualTestContext};
16241626
use language::{LanguageMatcher, TestFile};
16251627
use project::FakeFs;
1626-
use std::{
1627-
path::{Path, PathBuf},
1628-
time::SystemTime,
1629-
};
1628+
use std::path::{Path, PathBuf};
16301629

16311630
#[gpui::test]
16321631
fn test_path_for_file(cx: &mut AppContext) {
@@ -1679,9 +1678,7 @@ mod tests {
16791678
async fn test_deserialize(cx: &mut gpui::TestAppContext) {
16801679
init_test(cx, |_| {});
16811680

1682-
let now = SystemTime::now();
16831681
let fs = FakeFs::new(cx.executor());
1684-
fs.set_next_mtime(now);
16851682
fs.insert_file("/file.rs", Default::default()).await;
16861683

16871684
// Test case 1: Deserialize with path and contents
@@ -1690,12 +1687,18 @@ mod tests {
16901687
let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project.clone(), cx));
16911688
let workspace_id = workspace::WORKSPACE_DB.next_id().await.unwrap();
16921689
let item_id = 1234 as ItemId;
1690+
let mtime = fs
1691+
.metadata(Path::new("/file.rs"))
1692+
.await
1693+
.unwrap()
1694+
.unwrap()
1695+
.mtime;
16931696

16941697
let serialized_editor = SerializedEditor {
16951698
abs_path: Some(PathBuf::from("/file.rs")),
16961699
contents: Some("fn main() {}".to_string()),
16971700
language: Some("Rust".to_string()),
1698-
mtime: Some(now),
1701+
mtime: Some(mtime),
16991702
};
17001703

17011704
DB.save_serialized_editor(item_id, workspace_id, serialized_editor.clone())
@@ -1792,9 +1795,7 @@ mod tests {
17921795
let workspace_id = workspace::WORKSPACE_DB.next_id().await.unwrap();
17931796

17941797
let item_id = 9345 as ItemId;
1795-
let old_mtime = now
1796-
.checked_sub(std::time::Duration::from_secs(60 * 60 * 24))
1797-
.unwrap();
1798+
let old_mtime = MTime::from_seconds_and_nanos(0, 50);
17981799
let serialized_editor = SerializedEditor {
17991800
abs_path: Some(PathBuf::from("/file.rs")),
18001801
contents: Some("fn main() {}".to_string()),

crates/editor/src/persistence.rs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use anyhow::Result;
22
use db::sqlez::bindable::{Bind, Column, StaticColumnCount};
33
use db::sqlez::statement::Statement;
4+
use fs::MTime;
45
use std::path::PathBuf;
5-
use std::time::{Duration, SystemTime, UNIX_EPOCH};
66

77
use db::sqlez_macros::sql;
88
use db::{define_connection, query};
@@ -14,7 +14,7 @@ pub(crate) struct SerializedEditor {
1414
pub(crate) abs_path: Option<PathBuf>,
1515
pub(crate) contents: Option<String>,
1616
pub(crate) language: Option<String>,
17-
pub(crate) mtime: Option<SystemTime>,
17+
pub(crate) mtime: Option<MTime>,
1818
}
1919

2020
impl StaticColumnCount for SerializedEditor {
@@ -29,16 +29,13 @@ impl Bind for SerializedEditor {
2929
let start_index = statement.bind(&self.contents, start_index)?;
3030
let start_index = statement.bind(&self.language, start_index)?;
3131

32-
let mtime = self.mtime.and_then(|mtime| {
33-
mtime
34-
.duration_since(UNIX_EPOCH)
35-
.ok()
36-
.map(|duration| (duration.as_secs() as i64, duration.subsec_nanos() as i32))
37-
});
38-
let start_index = match mtime {
32+
let start_index = match self
33+
.mtime
34+
.and_then(|mtime| mtime.to_seconds_and_nanos_for_persistence())
35+
{
3936
Some((seconds, nanos)) => {
40-
let start_index = statement.bind(&seconds, start_index)?;
41-
statement.bind(&nanos, start_index)?
37+
let start_index = statement.bind(&(seconds as i64), start_index)?;
38+
statement.bind(&(nanos as i32), start_index)?
4239
}
4340
None => {
4441
let start_index = statement.bind::<Option<i64>>(&None, start_index)?;
@@ -64,7 +61,7 @@ impl Column for SerializedEditor {
6461

6562
let mtime = mtime_seconds
6663
.zip(mtime_nanos)
67-
.map(|(seconds, nanos)| UNIX_EPOCH + Duration::new(seconds as u64, nanos as u32));
64+
.map(|(seconds, nanos)| MTime::from_seconds_and_nanos(seconds as u64, nanos as u32));
6865

6966
let editor = Self {
7067
abs_path,
@@ -280,12 +277,11 @@ mod tests {
280277
assert_eq!(have, serialized_editor);
281278

282279
// Storing and retrieving mtime
283-
let now = SystemTime::now();
284280
let serialized_editor = SerializedEditor {
285281
abs_path: None,
286282
contents: None,
287283
language: None,
288-
mtime: Some(now),
284+
mtime: Some(MTime::from_seconds_and_nanos(100, 42)),
289285
};
290286

291287
DB.save_serialized_editor(1234, workspace_id, serialized_editor.clone())

crates/extension_host/src/extension_host.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,10 @@ impl ExtensionStore {
345345
if let (Ok(Some(index_metadata)), Ok(Some(extensions_metadata))) =
346346
(index_metadata, extensions_metadata)
347347
{
348-
if index_metadata.mtime > extensions_metadata.mtime {
348+
if index_metadata
349+
.mtime
350+
.bad_is_greater_than(extensions_metadata.mtime)
351+
{
349352
extension_index_needs_rebuild = false;
350353
}
351354
}

crates/fs/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ libc.workspace = true
2424
parking_lot.workspace = true
2525
paths.workspace = true
2626
rope.workspace = true
27+
proto.workspace = true
2728
serde.workspace = true
2829
serde_json.workspace = true
2930
smol.workspace = true

0 commit comments

Comments
 (0)