Skip to content

Commit 6388329

Browse files
committed
fix(update): Implement proper "once" behavior for check_updates setting
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 9929ca3 commit 6388329

4 files changed

Lines changed: 153 additions & 15 deletions

File tree

src/debugger.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ use zed_extension_api::{
1010
};
1111

1212
use crate::{
13-
config::get_java_debug_jar,
13+
config::{CheckUpdates, get_check_updates, get_java_debug_jar},
1414
lsp::LspWrapper,
1515
util::{
16-
create_path_if_not_exists, get_curr_dir, path_to_quoted_string,
16+
create_path_if_not_exists, get_curr_dir, mark_checked_once, path_to_quoted_string,
1717
should_use_local_or_download,
1818
},
1919
};
@@ -131,7 +131,14 @@ impl Debugger {
131131
return Ok(path);
132132
}
133133

134-
self.get_or_download_fork(language_server_id)
134+
let result = self.get_or_download_fork(language_server_id);
135+
136+
// Mark as checked once if in Once mode and download was successful
137+
if result.is_ok() && get_check_updates(configuration) == CheckUpdates::Once {
138+
let _ = mark_checked_once(DEBUGGER_INSTALL_PATH, "0.53.2");
139+
}
140+
141+
result
135142
}
136143

137144
fn get_or_download_fork(

src/jdk.rs

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@ 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_quoted_string, remove_all_files_except};
9+
use crate::{
10+
config::{CheckUpdates, get_check_updates},
11+
util::{
12+
get_curr_dir, has_checked_once, mark_checked_once, path_to_quoted_string,
13+
remove_all_files_except,
14+
},
15+
};
1016

1117
// Errors
1218
const JDK_DIR_ERROR: &str = "Failed to read into JDK install directory";
@@ -15,6 +21,7 @@ const NO_JDK_DIR_ERROR: &str = "No match for jdk or corretto in the extracted di
1521
const CORRETTO_REPO: &str = "corretto/corretto-25";
1622
const CORRETTO_UNIX_URL_TEMPLATE: &str = "https://corretto.aws/downloads/resources/{version}/amazon-corretto-{version}-{platform}-{arch}.tar.gz";
1723
const CORRETTO_WINDOWS_URL_TEMPLATE: &str = "https://corretto.aws/downloads/resources/{version}/amazon-corretto-{version}-{platform}-{arch}-jdk.zip";
24+
const JDK_INSTALL_PATH: &str = "jdk";
1825

1926
fn build_corretto_url(version: &str, platform: &str, arch: &str) -> String {
2027
let template = match zed::current_platform().0 {
@@ -46,9 +53,58 @@ fn get_platform() -> zed::Result<String> {
4653
}
4754
}
4855

56+
fn find_latest_local_jdk() -> Option<PathBuf> {
57+
let jdk_path = get_curr_dir().ok()?.join(JDK_INSTALL_PATH);
58+
std::fs::read_dir(&jdk_path)
59+
.ok()?
60+
.filter_map(Result::ok)
61+
.map(|entry| entry.path())
62+
.filter(|path| path.is_dir())
63+
.filter_map(|path| {
64+
let created_time = std::fs::metadata(&path)
65+
.and_then(|meta| meta.created())
66+
.ok()?;
67+
Some((path, created_time))
68+
})
69+
.max_by_key(|&(_, time)| time)
70+
.map(|(path, _)| path)
71+
}
72+
4973
pub fn try_to_fetch_and_install_latest_jdk(
5074
language_server_id: &LanguageServerId,
75+
configuration: &Option<Value>,
5176
) -> zed::Result<PathBuf> {
77+
let jdk_path = get_curr_dir()?.join(JDK_INSTALL_PATH);
78+
79+
// Check if we should use local installation based on update mode
80+
match get_check_updates(configuration) {
81+
CheckUpdates::Never => {
82+
if let Some(local_path) = find_latest_local_jdk() {
83+
return get_jdk_bin_path(&local_path);
84+
}
85+
return Err(
86+
"Update checks disabled (never) and no local JDK installation found".to_string(),
87+
);
88+
}
89+
CheckUpdates::Once => {
90+
// If we have a local installation, use it
91+
if let Some(local_path) = find_latest_local_jdk() {
92+
return get_jdk_bin_path(&local_path);
93+
}
94+
95+
// If we've already checked once, don't check again
96+
if has_checked_once(JDK_INSTALL_PATH) {
97+
return Err(
98+
"Update check already performed once for JDK. No local installation found."
99+
.to_string(),
100+
);
101+
}
102+
}
103+
CheckUpdates::Always => {
104+
// Continue to check for updates
105+
}
106+
}
107+
52108
let version = zed::latest_github_release(
53109
CORRETTO_REPO,
54110
zed_extension_api::GithubReleaseOptions {
@@ -58,7 +114,6 @@ pub fn try_to_fetch_and_install_latest_jdk(
58114
)?
59115
.version;
60116

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

64119
// Check for updates, if same version is already downloaded skip download
@@ -87,12 +142,21 @@ pub fn try_to_fetch_and_install_latest_jdk(
87142
)?;
88143

89144
// Remove older versions
90-
let _ = remove_all_files_except(jdk_path, version.as_str());
145+
let _ = remove_all_files_except(&jdk_path, version.as_str());
146+
}
147+
148+
// Mark as checked once if in Once mode
149+
if get_check_updates(configuration) == CheckUpdates::Once {
150+
let _ = mark_checked_once(JDK_INSTALL_PATH, &version);
91151
}
92152

153+
get_jdk_bin_path(&install_path)
154+
}
155+
156+
fn get_jdk_bin_path(install_path: &Path) -> zed::Result<PathBuf> {
93157
// Depending on the platform the name of the extracted dir might differ
94158
// Rather than hard coding, extract it dynamically
95-
let extracted_dir = get_extracted_dir(&install_path)?;
159+
let extracted_dir = get_extracted_dir(install_path)?;
96160

97161
Ok(install_path
98162
.join(extracted_dir)

src/jdtls.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ use zed_extension_api::{
1515
};
1616

1717
use crate::{
18-
config::is_java_autodownload,
18+
config::{CheckUpdates, get_check_updates, is_java_autodownload},
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_quoted_string,
23-
remove_all_files_except, should_use_local_or_download,
22+
get_java_major_version, get_latest_versions_from_tag, mark_checked_once,
23+
path_to_quoted_string, remove_all_files_except, should_use_local_or_download,
2424
},
2525
};
2626

@@ -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
}
@@ -204,6 +205,11 @@ pub fn try_to_fetch_and_install_latest_jdtls(
204205
let _ = remove_all_files_except(prefix, build_directory.as_str());
205206
}
206207

208+
// Mark as checked once if in Once mode
209+
if get_check_updates(configuration) == CheckUpdates::Once {
210+
let _ = mark_checked_once(JDTLS_INSTALL_PATH, &latest_version);
211+
}
212+
207213
// return jdtls base path
208214
Ok(build_path)
209215
}
@@ -250,6 +256,11 @@ pub fn try_to_fetch_and_install_latest_lombok(
250256
let _ = remove_all_files_except(prefix, jar_name.as_str());
251257
}
252258

259+
// Mark as checked once if in Once mode
260+
if get_check_updates(configuration) == CheckUpdates::Once {
261+
let _ = mark_checked_once(LOMBOK_INSTALL_PATH, &latest_version);
262+
}
263+
253264
// else use it
254265
Ok(jar_path)
255266
}

src/util.rs

Lines changed: 59 additions & 3 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

@@ -253,7 +293,7 @@ fn get_tag_at(github_tags: &Value, index: usize) -> Option<&str> {
253293
/// On Unix, returns the path unquoted since spawn() treats quotes as literals.
254294
fn format_path_for_os(path_str: String, os: Os) -> String {
255295
if os == Os::Windows {
256-
format!("\"{}\"", path_str)
296+
format!("\"{path_str}\"")
257297
} else {
258298
path_str
259299
}
@@ -350,6 +390,7 @@ pub fn remove_all_files_except<P: AsRef<Path>>(prefix: P, filename: &str) -> zed
350390
///
351391
/// # Errors
352392
/// - Update mode is Never but no local installation found
393+
/// - Update mode is Once and already checked but no local installation found
353394
pub fn should_use_local_or_download(
354395
configuration: &Option<Value>,
355396
local: Option<PathBuf>,
@@ -362,7 +403,22 @@ pub fn should_use_local_or_download(
362403
"{NO_LOCAL_INSTALL_NEVER_ERROR} for {component_name}"
363404
)),
364405
},
365-
CheckUpdates::Once => Ok(local),
406+
CheckUpdates::Once => {
407+
// If we have a local installation, use it
408+
if let Some(path) = local {
409+
return Ok(Some(path));
410+
}
411+
412+
// If we've already checked once, don't check again
413+
if has_checked_once(component_name) {
414+
return Err(format!(
415+
"{NO_LOCAL_INSTALL_ONCE_ERROR} for {component_name}"
416+
));
417+
}
418+
419+
// First time checking - allow download
420+
Ok(None)
421+
}
366422
CheckUpdates::Always => Ok(None),
367423
}
368424
}

0 commit comments

Comments
 (0)