Skip to content

Commit 19aea3a

Browse files
authored
fix(update): Implement proper "once" behavior for check_updates setting (#160)
When check_updates is set to "once", the extension now correctly checks for updates only once per component, preventing repeated GitHub API calls on every Zed restart. Previously, the "once" mode would check GitHub API on every restart when no local installation existed, making it functionally identical to "always" mode until a download completed. Changes: - Add persistence mechanism using .update_checked marker files - Store downloaded version in marker files for future reference - Implement has_checked_once() and mark_checked_once() helpers - Update should_use_local_or_download() to check marker before allowing download - Apply fix to all components: JDTLS, Lombok, Debugger, and JDK Behavior after fix: - First run: GitHub API called → Download → Create marker with version - Subsequent runs: No GitHub API call (uses local or returns error) Marker files created: - jdtls/.update_checked - lombok/.update_checked - debugger/.update_checked - jdk/.update_checked
1 parent 11d843a commit 19aea3a

File tree

4 files changed

+141
-24
lines changed

4 files changed

+141
-24
lines changed

src/debugger.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ use zed_extension_api::{
1212
use crate::{
1313
config::get_java_debug_jar,
1414
lsp::LspWrapper,
15-
util::{create_path_if_not_exists, get_curr_dir, path_to_string, should_use_local_or_download},
15+
util::{
16+
create_path_if_not_exists, get_curr_dir, mark_checked_once, path_to_string,
17+
should_use_local_or_download,
18+
},
1619
};
1720

1821
#[derive(Serialize, Deserialize, Debug)]
@@ -121,9 +124,11 @@ impl Debugger {
121124
}
122125

123126
// Use local installation if update mode requires it
124-
if let Some(path) =
125-
should_use_local_or_download(configuration, find_latest_local_debugger(), "debugger")?
126-
{
127+
if let Some(path) = should_use_local_or_download(
128+
configuration,
129+
find_latest_local_debugger(),
130+
DEBUGGER_INSTALL_PATH,
131+
)? {
127132
self.plugin_path = Some(path.clone());
128133
return Ok(path);
129134
}
@@ -159,6 +164,9 @@ impl Debugger {
159164
format!("Failed to download java-debug fork from {JAVA_DEBUG_PLUGIN_FORK_URL}: {err}")
160165
})?;
161166

167+
// Mark the downloaded version for "Once" mode tracking
168+
let _ = mark_checked_once(DEBUGGER_INSTALL_PATH, latest_version);
169+
162170
self.plugin_path = Some(jar_path.clone());
163171
Ok(jar_path)
164172
}
@@ -260,6 +268,9 @@ impl Debugger {
260268
DownloadedFileType::Uncompressed,
261269
)
262270
.map_err(|err| format!("Failed to download {url} {err}"))?;
271+
272+
// Mark the downloaded version for "Once" mode tracking
273+
let _ = mark_checked_once(DEBUGGER_INSTALL_PATH, latest_version);
263274
}
264275

265276
self.plugin_path = Some(jar_path.clone());

src/jdk.rs

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ use std::path::{Path, PathBuf};
22

33
use zed_extension_api::{
44
self as zed, Architecture, DownloadedFileType, LanguageServerId,
5-
LanguageServerInstallationStatus, Os, current_platform, download_file,
5+
LanguageServerInstallationStatus, Os, current_platform, download_file, serde_json::Value,
66
set_language_server_installation_status,
77
};
88

9-
use crate::util::{get_curr_dir, path_to_string, remove_all_files_except};
9+
use crate::util::{
10+
get_curr_dir, mark_checked_once, path_to_string, remove_all_files_except,
11+
should_use_local_or_download,
12+
};
1013

1114
// Errors
1215
const JDK_DIR_ERROR: &str = "Failed to read into JDK install directory";
@@ -15,6 +18,7 @@ const NO_JDK_DIR_ERROR: &str = "No match for jdk or corretto in the extracted di
1518
const CORRETTO_REPO: &str = "corretto/corretto-25";
1619
const CORRETTO_UNIX_URL_TEMPLATE: &str = "https://corretto.aws/downloads/resources/{version}/amazon-corretto-{version}-{platform}-{arch}.tar.gz";
1720
const CORRETTO_WINDOWS_URL_TEMPLATE: &str = "https://corretto.aws/downloads/resources/{version}/amazon-corretto-{version}-{platform}-{arch}-jdk.zip";
21+
const JDK_INSTALL_PATH: &str = "jdk";
1822

1923
fn build_corretto_url(version: &str, platform: &str, arch: &str) -> String {
2024
let template = match zed::current_platform().0 {
@@ -46,9 +50,42 @@ fn get_platform() -> zed::Result<String> {
4650
}
4751
}
4852

53+
fn find_latest_local_jdk() -> Option<PathBuf> {
54+
let jdk_path = get_curr_dir().ok()?.join(JDK_INSTALL_PATH);
55+
std::fs::read_dir(&jdk_path)
56+
.ok()?
57+
.filter_map(Result::ok)
58+
.map(|entry| entry.path())
59+
.filter(|path| path.is_dir())
60+
.filter_map(|path| {
61+
let created_time = std::fs::metadata(&path)
62+
.and_then(|meta| meta.created())
63+
.ok()?;
64+
Some((path, created_time))
65+
})
66+
.max_by_key(|&(_, time)| time)
67+
.map(|(path, _)| path)
68+
}
69+
4970
pub fn try_to_fetch_and_install_latest_jdk(
5071
language_server_id: &LanguageServerId,
72+
configuration: &Option<Value>,
5173
) -> zed::Result<PathBuf> {
74+
let jdk_path = get_curr_dir()?.join(JDK_INSTALL_PATH);
75+
76+
// Check if we should use local installation based on update mode
77+
if let Some(path) =
78+
should_use_local_or_download(configuration, find_latest_local_jdk(), JDK_INSTALL_PATH)?
79+
{
80+
return get_jdk_bin_path(&path);
81+
}
82+
83+
// Check for updates, if same version is already downloaded skip download
84+
set_language_server_installation_status(
85+
language_server_id,
86+
&LanguageServerInstallationStatus::CheckingForUpdate,
87+
);
88+
5289
let version = zed::latest_github_release(
5390
CORRETTO_REPO,
5491
zed_extension_api::GithubReleaseOptions {
@@ -58,16 +95,8 @@ pub fn try_to_fetch_and_install_latest_jdk(
5895
)?
5996
.version;
6097

61-
let jdk_path = get_curr_dir()?.join("jdk");
6298
let install_path = jdk_path.join(&version);
6399

64-
// Check for updates, if same version is already downloaded skip download
65-
66-
set_language_server_installation_status(
67-
language_server_id,
68-
&LanguageServerInstallationStatus::CheckingForUpdate,
69-
);
70-
71100
if !install_path.exists() {
72101
set_language_server_installation_status(
73102
language_server_id,
@@ -87,12 +116,19 @@ pub fn try_to_fetch_and_install_latest_jdk(
87116
)?;
88117

89118
// Remove older versions
90-
let _ = remove_all_files_except(jdk_path, version.as_str());
119+
let _ = remove_all_files_except(&jdk_path, version.as_str());
120+
121+
// Mark the downloaded version for "Once" mode tracking
122+
let _ = mark_checked_once(JDK_INSTALL_PATH, &version);
91123
}
92124

125+
get_jdk_bin_path(&install_path)
126+
}
127+
128+
fn get_jdk_bin_path(install_path: &Path) -> zed::Result<PathBuf> {
93129
// Depending on the platform the name of the extracted dir might differ
94130
// Rather than hard coding, extract it dynamically
95-
let extracted_dir = get_extracted_dir(&install_path)?;
131+
let extracted_dir = get_extracted_dir(install_path)?;
96132

97133
Ok(install_path
98134
.join(extracted_dir)

src/jdtls.rs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::{
1919
jdk::try_to_fetch_and_install_latest_jdk,
2020
util::{
2121
create_path_if_not_exists, get_curr_dir, get_java_exec_name, get_java_executable,
22-
get_java_major_version, get_latest_versions_from_tag, path_to_string,
22+
get_java_major_version, get_latest_versions_from_tag, mark_checked_once, path_to_string,
2323
remove_all_files_except, should_use_local_or_download,
2424
},
2525
};
@@ -50,7 +50,8 @@ pub fn build_jdtls_launch_args(
5050
if java_major_version < 21 {
5151
if is_java_autodownload(configuration) {
5252
java_executable =
53-
try_to_fetch_and_install_latest_jdk(language_server_id)?.join(get_java_exec_name());
53+
try_to_fetch_and_install_latest_jdk(language_server_id, configuration)?
54+
.join(get_java_exec_name());
5455
} else {
5556
return Err(JAVA_VERSION_ERROR.to_string());
5657
}
@@ -157,12 +158,17 @@ pub fn try_to_fetch_and_install_latest_jdtls(
157158
) -> zed::Result<PathBuf> {
158159
// Use local installation if update mode requires it
159160
if let Some(path) =
160-
should_use_local_or_download(configuration, find_latest_local_jdtls(), "jdtls")?
161+
should_use_local_or_download(configuration, find_latest_local_jdtls(), JDTLS_INSTALL_PATH)?
161162
{
162163
return Ok(path);
163164
}
164165

165166
// Download latest version
167+
set_language_server_installation_status(
168+
language_server_id,
169+
&LanguageServerInstallationStatus::CheckingForUpdate,
170+
);
171+
166172
let (last, second_last) = get_latest_versions_from_tag(JDTLS_REPO)?;
167173

168174
let (latest_version, latest_version_build) = download_jdtls_milestone(last.as_ref())
@@ -202,6 +208,9 @@ pub fn try_to_fetch_and_install_latest_jdtls(
202208

203209
// ...and delete other versions
204210
let _ = remove_all_files_except(prefix, build_directory.as_str());
211+
212+
// Mark the downloaded version for "Once" mode tracking
213+
let _ = mark_checked_once(JDTLS_INSTALL_PATH, &latest_version);
205214
}
206215

207216
// return jdtls base path
@@ -213,9 +222,11 @@ pub fn try_to_fetch_and_install_latest_lombok(
213222
configuration: &Option<Value>,
214223
) -> zed::Result<PathBuf> {
215224
// Use local installation if update mode requires it
216-
if let Some(path) =
217-
should_use_local_or_download(configuration, find_latest_local_lombok(), "lombok")?
218-
{
225+
if let Some(path) = should_use_local_or_download(
226+
configuration,
227+
find_latest_local_lombok(),
228+
LOMBOK_INSTALL_PATH,
229+
)? {
219230
return Ok(path);
220231
}
221232

@@ -248,6 +259,9 @@ pub fn try_to_fetch_and_install_latest_lombok(
248259
// ...and delete other versions
249260

250261
let _ = remove_all_files_except(prefix, jar_name.as_str());
262+
263+
// Mark the downloaded version for "Once" mode tracking
264+
let _ = mark_checked_once(LOMBOK_INSTALL_PATH, &latest_version);
251265
}
252266

253267
// else use it

src/util.rs

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ const TAG_UNEXPECTED_FORMAT_ERROR: &str = "Malformed GitHub tags response";
3232
const PATH_IS_NOT_DIR: &str = "File exists but is not a path";
3333
const NO_LOCAL_INSTALL_NEVER_ERROR: &str =
3434
"Update checks disabled (never) and no local installation found";
35+
const NO_LOCAL_INSTALL_ONCE_ERROR: &str =
36+
"Update check already performed once and no local installation found";
37+
38+
const ONCE_CHECK_MARKER: &str = ".update_checked";
3539

3640
/// Create a Path if it does not exist
3741
///
@@ -60,6 +64,41 @@ pub fn create_path_if_not_exists<P: AsRef<Path>>(path: P) -> zed::Result<()> {
6064
}
6165
}
6266

67+
/// Check if update check has been performed once for a component
68+
///
69+
/// # Arguments
70+
///
71+
/// * [`component_name`] - The component directory name (e.g., "jdtls", "lombok")
72+
///
73+
/// # Returns
74+
///
75+
/// Returns true if the marker file exists, indicating a check was already performed
76+
pub fn has_checked_once(component_name: &str) -> bool {
77+
PathBuf::from(component_name)
78+
.join(ONCE_CHECK_MARKER)
79+
.exists()
80+
}
81+
82+
/// Mark that an update check has been performed for a component
83+
///
84+
/// # Arguments
85+
///
86+
/// * [`component_name`] - The component directory name (e.g., "jdtls", "lombok")
87+
/// * [`version`] - The version that was downloaded
88+
///
89+
/// # Returns
90+
///
91+
/// Returns Ok(()) if the marker was created successfully
92+
///
93+
/// # Errors
94+
///
95+
/// Returns an error if the directory or marker file could not be created
96+
pub fn mark_checked_once(component_name: &str, version: &str) -> zed::Result<()> {
97+
let marker_path = PathBuf::from(component_name).join(ONCE_CHECK_MARKER);
98+
create_path_if_not_exists(PathBuf::from(component_name))?;
99+
fs::write(marker_path, version).map_err(|e| e.to_string())
100+
}
101+
63102
/// Expand ~ on Unix-like systems
64103
///
65104
/// # Arguments
@@ -140,7 +179,8 @@ pub fn get_java_executable(
140179
// If the user has set the option, retrieve the latest version of Corretto (OpenJDK)
141180
if is_java_autodownload(configuration) {
142181
return Ok(
143-
try_to_fetch_and_install_latest_jdk(language_server_id)?.join(java_executable_filename)
182+
try_to_fetch_and_install_latest_jdk(language_server_id, configuration)?
183+
.join(java_executable_filename),
144184
);
145185
}
146186

@@ -329,6 +369,7 @@ pub fn remove_all_files_except<P: AsRef<Path>>(prefix: P, filename: &str) -> zed
329369
///
330370
/// # Errors
331371
/// - Update mode is Never but no local installation found
372+
/// - Update mode is Once and already checked but no local installation found
332373
pub fn should_use_local_or_download(
333374
configuration: &Option<Value>,
334375
local: Option<PathBuf>,
@@ -341,7 +382,22 @@ pub fn should_use_local_or_download(
341382
"{NO_LOCAL_INSTALL_NEVER_ERROR} for {component_name}"
342383
)),
343384
},
344-
CheckUpdates::Once => Ok(local),
385+
CheckUpdates::Once => {
386+
// If we have a local installation, use it
387+
if let Some(path) = local {
388+
return Ok(Some(path));
389+
}
390+
391+
// If we've already checked once, don't check again
392+
if has_checked_once(component_name) {
393+
return Err(format!(
394+
"{NO_LOCAL_INSTALL_ONCE_ERROR} for {component_name}"
395+
));
396+
}
397+
398+
// First time checking - allow download
399+
Ok(None)
400+
}
345401
CheckUpdates::Always => Ok(None),
346402
}
347403
}

0 commit comments

Comments
 (0)