Skip to content

Commit 4325206

Browse files
QaidVoidQaidVoid
authored
fix(packages): skip version fetching when installed version matches (#143)
1 parent 099f96c commit 4325206

6 files changed

Lines changed: 354 additions & 277 deletions

File tree

‎crates/soar-cli/src/apply.rs‎

Lines changed: 222 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ pub struct ApplyDiff {
106106
pub in_sync: Vec<String>,
107107
/// Packages not found in metadata
108108
pub not_found: Vec<String>,
109+
/// Pending version updates for packages.toml (package_name, version)
110+
pub pending_version_updates: Vec<(String, String)>,
109111
}
110112

111113
/// Main entry point for the apply command
@@ -131,12 +133,27 @@ pub async fn apply_packages(
131133

132134
display_diff(&diff, prune);
133135

134-
if diff.to_install.is_empty() && diff.to_update.is_empty() && diff.to_remove.is_empty() {
136+
let has_package_changes =
137+
!diff.to_install.is_empty() || !diff.to_update.is_empty() || !diff.to_remove.is_empty();
138+
let has_toml_updates = !diff.pending_version_updates.is_empty();
139+
140+
if !has_package_changes && !has_toml_updates {
135141
info!("\nAll packages are in sync!");
136142
return Ok(());
137143
}
138144

139145
if dry_run {
146+
if has_toml_updates {
147+
info!("\nWould update packages.toml:");
148+
for (pkg_name, version) in &diff.pending_version_updates {
149+
info!(
150+
" {} {} -> {}",
151+
Colored(Blue, pkg_name),
152+
Colored(Yellow, "version"),
153+
Colored(Green, version)
154+
);
155+
}
156+
}
140157
info!("\n{} Dry run - no changes made", icon_or("", "[DRY RUN]"));
141158
return Ok(());
142159
}
@@ -171,82 +188,186 @@ async fn compute_diff(
171188
// Track declared package
172189
declared_keys.insert((pkg.name.clone(), pkg.pkg_id.clone(), pkg.repo.clone()));
173190

174-
// Handle GitHub/GitLab release sources
175-
if pkg.github.is_some() || pkg.gitlab.is_some() {
176-
let source = match ReleaseSource::from_resolved(pkg) {
177-
Some(s) => s,
178-
None => {
179-
diff.not_found.push(format!(
180-
"{} (missing asset_pattern for github/gitlab source)",
181-
pkg.name
182-
));
183-
continue;
184-
}
191+
let is_github_or_gitlab = pkg.github.is_some() || pkg.gitlab.is_some();
192+
if is_github_or_gitlab || pkg.url.is_some() {
193+
let local_pkg_id = if is_github_or_gitlab {
194+
pkg.pkg_id.clone().or_else(|| {
195+
pkg.github
196+
.as_ref()
197+
.or(pkg.gitlab.as_ref())
198+
.map(|repo| repo.replace('/', "."))
199+
})
200+
} else {
201+
pkg.pkg_id.clone()
185202
};
186203

187-
// If version is specified, fetch that specific tag; otherwise fetch latest
188-
let release = match source.resolve_version(pkg.version.as_deref()) {
189-
Ok(r) => r,
190-
Err(e) => {
191-
warn!("Failed to resolve release for {}: {}", pkg.name, e);
192-
diff.not_found.push(format!("{} ({})", pkg.name, e));
193-
continue;
204+
let installed: Option<InstalledPackage> = diesel_db
205+
.with_conn(|conn| {
206+
CoreRepository::list_filtered(
207+
conn,
208+
Some("local"),
209+
Some(&pkg.name),
210+
local_pkg_id.as_deref(),
211+
None,
212+
Some(true),
213+
None,
214+
Some(1),
215+
None,
216+
)
217+
})?
218+
.into_iter()
219+
.next()
220+
.map(Into::into);
221+
222+
if let Some(ref cmd) = pkg.version_command {
223+
if let Some(ref declared) = pkg.version {
224+
let normalized = declared.strip_prefix('v').unwrap_or(declared);
225+
if let Some(ref existing) = installed {
226+
if existing.version == normalized {
227+
diff.in_sync.push(format!("{} (local)", pkg.name));
228+
continue;
229+
}
230+
}
194231
}
195-
};
196232

197-
// Use version_command if specified, otherwise use release version
198-
let version = if let Some(ref cmd) = pkg.version_command {
199-
match run_version_command(cmd) {
200-
Ok(v) => v,
233+
let result = match run_version_command(cmd) {
234+
Ok(r) => r,
201235
Err(e) => {
202236
warn!("Failed to run version_command for {}: {}", pkg.name, e);
203-
release.version.clone()
237+
diff.not_found
238+
.push(format!("{} (version_command failed: {})", pkg.name, e));
239+
continue;
240+
}
241+
};
242+
243+
let version = result
244+
.version
245+
.strip_prefix('v')
246+
.unwrap_or(&result.version)
247+
.to_string();
248+
249+
if let Some(ref existing) = installed {
250+
if existing.version == version {
251+
let declared = pkg
252+
.version
253+
.as_ref()
254+
.map(|s| s.strip_prefix('v').unwrap_or(s));
255+
if declared != Some(version.as_str()) {
256+
// Queue version update for after user confirmation
257+
diff.pending_version_updates
258+
.push((pkg.name.clone(), version.clone()));
259+
}
260+
diff.in_sync.push(format!("{} (local)", pkg.name));
261+
continue;
204262
}
205263
}
206-
} else {
207-
release.version.clone()
208-
};
209264

210-
let version = version.strip_prefix('v').unwrap_or(&version).to_string();
265+
let mut url_pkg = UrlPackage::from_remote(
266+
&result.download_url,
267+
Some(&pkg.name),
268+
Some(&version),
269+
pkg.pkg_type.as_deref(),
270+
local_pkg_id.as_deref(),
271+
)?;
272+
url_pkg.size = result.size;
211273

212-
let derived_pkg_id = pkg.pkg_id.clone().or_else(|| {
213-
pkg.github
214-
.as_ref()
215-
.or(pkg.gitlab.as_ref())
216-
.map(|repo| repo.replace('/', "."))
217-
});
274+
match check_url_package_status(&url_pkg, pkg, "local", &diesel_db)? {
275+
UrlPackageStatus::ToInstall(target) => {
276+
diff.to_install.push((pkg.clone(), target))
277+
}
278+
UrlPackageStatus::ToUpdate(target) => {
279+
diff.to_update.push((pkg.clone(), target))
280+
}
281+
UrlPackageStatus::InSync(label) => diff.in_sync.push(label),
282+
}
283+
continue;
284+
}
285+
286+
if is_github_or_gitlab {
287+
if let Some(ref declared) = pkg.version {
288+
let normalized = declared.strip_prefix('v').unwrap_or(declared);
289+
if let Some(ref existing) = installed {
290+
if existing.version == normalized {
291+
diff.in_sync.push(format!("{} (local)", pkg.name));
292+
continue;
293+
}
294+
}
295+
}
296+
297+
let source = match ReleaseSource::from_resolved(pkg) {
298+
Some(s) => s,
299+
None => {
300+
diff.not_found.push(format!(
301+
"{} (missing asset_pattern for github/gitlab source)",
302+
pkg.name
303+
));
304+
continue;
305+
}
306+
};
307+
let release = match source.resolve_version(pkg.version.as_deref()) {
308+
Ok(r) => r,
309+
Err(e) => {
310+
warn!("Failed to resolve release for {}: {}", pkg.name, e);
311+
diff.not_found.push(format!("{} ({})", pkg.name, e));
312+
continue;
313+
}
314+
};
315+
let version = release
316+
.version
317+
.strip_prefix('v')
318+
.unwrap_or(&release.version)
319+
.to_string();
320+
321+
let url_pkg = UrlPackage::from_remote(
322+
&release.download_url,
323+
Some(&pkg.name),
324+
Some(&version),
325+
pkg.pkg_type.as_deref(),
326+
local_pkg_id.as_deref(),
327+
)?;
218328

219-
let url_pkg = UrlPackage::from_remote(
220-
&release.download_url,
221-
Some(&pkg.name),
222-
Some(&version),
223-
pkg.pkg_type.as_deref(),
224-
derived_pkg_id.as_deref(),
225-
)?;
226-
227-
match check_url_package_status(&url_pkg, pkg, "local", &diesel_db)? {
228-
UrlPackageStatus::ToInstall(target) => diff.to_install.push((pkg.clone(), target)),
229-
UrlPackageStatus::ToUpdate(target) => diff.to_update.push((pkg.clone(), target)),
230-
UrlPackageStatus::InSync(label) => diff.in_sync.push(label),
329+
match check_url_package_status(&url_pkg, pkg, "local", &diesel_db)? {
330+
UrlPackageStatus::ToInstall(target) => {
331+
diff.to_install.push((pkg.clone(), target))
332+
}
333+
UrlPackageStatus::ToUpdate(target) => {
334+
diff.to_update.push((pkg.clone(), target))
335+
}
336+
UrlPackageStatus::InSync(label) => diff.in_sync.push(label),
337+
}
338+
continue;
231339
}
232-
continue;
233-
}
234340

235-
if let Some(ref url) = pkg.url {
236-
let url_pkg = UrlPackage::from_remote(
237-
url,
238-
Some(&pkg.name),
239-
pkg.version.as_deref(),
240-
pkg.pkg_type.as_deref(),
241-
pkg.pkg_id.as_deref(),
242-
)?;
243-
244-
match check_url_package_status(&url_pkg, pkg, "local", &diesel_db)? {
245-
UrlPackageStatus::ToInstall(target) => diff.to_install.push((pkg.clone(), target)),
246-
UrlPackageStatus::ToUpdate(target) => diff.to_update.push((pkg.clone(), target)),
247-
UrlPackageStatus::InSync(label) => diff.in_sync.push(label),
341+
if let Some(ref url) = pkg.url {
342+
if let Some(ref declared) = pkg.version {
343+
let normalized = declared.strip_prefix('v').unwrap_or(declared);
344+
if let Some(ref existing) = installed {
345+
if existing.version == normalized {
346+
diff.in_sync.push(format!("{} (local)", pkg.name));
347+
continue;
348+
}
349+
}
350+
}
351+
352+
let url_pkg = UrlPackage::from_remote(
353+
url,
354+
Some(&pkg.name),
355+
pkg.version.as_deref(),
356+
pkg.pkg_type.as_deref(),
357+
pkg.pkg_id.as_deref(),
358+
)?;
359+
360+
match check_url_package_status(&url_pkg, pkg, "local", &diesel_db)? {
361+
UrlPackageStatus::ToInstall(target) => {
362+
diff.to_install.push((pkg.clone(), target))
363+
}
364+
UrlPackageStatus::ToUpdate(target) => {
365+
diff.to_update.push((pkg.clone(), target))
366+
}
367+
UrlPackageStatus::InSync(label) => diff.in_sync.push(label),
368+
}
369+
continue;
248370
}
249-
continue;
250371
}
251372

252373
// Find package in metadata
@@ -586,11 +707,25 @@ async fn execute_apply(state: &AppState, diff: ApplyDiff, no_verify: bool) -> So
586707

587708
let mut version_updates: Vec<(String, String)> = Vec::new();
588709

710+
// Apply pending version updates for in-sync packages
711+
for (pkg_name, version) in &diff.pending_version_updates {
712+
if let Err(e) = PackagesConfig::update_package(pkg_name, None, Some(version), None) {
713+
warn!(
714+
"Failed to update version for '{}' in packages.toml: {}",
715+
pkg_name, e
716+
);
717+
}
718+
}
719+
589720
if !diff.to_install.is_empty() {
590721
info!("\nInstalling {} package(s)...", diff.to_install.len());
591722

592723
for (pkg, target) in &diff.to_install {
593-
if (pkg.github.is_some() || pkg.gitlab.is_some()) && pkg.version.is_none() {
724+
let declared_version = pkg
725+
.version
726+
.as_ref()
727+
.map(|v| v.strip_prefix('v').unwrap_or(v));
728+
if declared_version != Some(target.package.version.as_str()) {
594729
version_updates.push((pkg.name.clone(), target.package.version.clone()));
595730
}
596731
}
@@ -633,6 +768,17 @@ async fn execute_apply(state: &AppState, diff: ApplyDiff, no_verify: bool) -> So
633768
if !diff.to_update.is_empty() {
634769
info!("\nUpdating {} package(s)...", diff.to_update.len());
635770

771+
let mut update_version_updates: Vec<(String, String)> = Vec::new();
772+
for (pkg, target) in &diff.to_update {
773+
let declared_version = pkg
774+
.version
775+
.as_ref()
776+
.map(|v| v.strip_prefix('v').unwrap_or(v));
777+
if declared_version != Some(target.package.version.as_str()) {
778+
update_version_updates.push((pkg.name.clone(), target.package.version.clone()));
779+
}
780+
}
781+
636782
let targets: Vec<InstallTarget> = diff
637783
.to_update
638784
.into_iter()
@@ -654,6 +800,18 @@ async fn execute_apply(state: &AppState, diff: ApplyDiff, no_verify: bool) -> So
654800
perform_update(ctx.clone(), targets, diesel_db.clone(), false).await?;
655801
updated_count = ctx.installed_count.load(Ordering::Relaxed) as usize;
656802
failed_count += ctx.failed.load(Ordering::Relaxed) as usize;
803+
804+
if updated_count > 0 {
805+
for (pkg_name, version) in &update_version_updates {
806+
if let Err(e) = PackagesConfig::update_package(pkg_name, None, Some(version), None)
807+
{
808+
warn!(
809+
"Failed to update version for '{}' in packages.toml: {}",
810+
pkg_name, e
811+
);
812+
}
813+
}
814+
}
657815
}
658816

659817
if !diff.to_remove.is_empty() {

0 commit comments

Comments
 (0)