Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(utils): Remove allow(dead_code) from update utils #2216

Merged
merged 2 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ impl Api {

/// Convenience method that downloads a file into the given file object
/// and show a progress bar
#[cfg(not(feature = "managed"))]
pub fn download_with_progress(&self, url: &str, dst: &mut File) -> ApiResult<ApiResponse> {
self.request(Method::Get, url, None)?
.follow_location(true)?
Expand Down Expand Up @@ -329,12 +330,13 @@ impl Api {

if resp.status() == 200 {
let info: RegistryRelease = resp.convert()?;
for (filename, download_url) in info.file_urls {
for (filename, _download_url) in info.file_urls {
info!("Found asset {}", filename);
if filename == ref_name {
return Ok(Some(SentryCliRelease {
version: info.version,
download_url,
#[cfg(not(feature = "managed"))]
download_url: _download_url,
}));
}
}
Expand Down Expand Up @@ -2164,6 +2166,7 @@ struct RegistryRelease {
/// Information about sentry CLI releases
pub struct SentryCliRelease {
pub version: String,
#[cfg(not(feature = "managed"))]
pub download_url: String,
}

Expand Down
2 changes: 2 additions & 0 deletions src/utils/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ impl Drop for TempFile {
}

/// Checks if a path is writable.
#[cfg(not(feature = "managed"))]
pub fn is_writable<P: AsRef<Path>>(path: P) -> bool {
fs::OpenOptions::new()
.write(true)
Expand All @@ -134,6 +135,7 @@ pub fn is_writable<P: AsRef<Path>>(path: P) -> bool {

/// Set the mode of a path to 755 if we're on a Unix machine, otherwise
/// don't do anything with the given path.
#[cfg(not(feature = "managed"))]
pub fn set_executable_mode<P: AsRef<Path>>(path: P) -> Result<()> {
#[cfg(not(windows))]
fn exec<P: AsRef<Path>>(path: P) -> io::Result<()> {
Expand Down
9 changes: 8 additions & 1 deletion src/utils/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ impl Deref for ProgressBar {
pub enum ProgressBarMode {
Disabled,
Request,
#[cfg(not(feature = "managed"))]
Response,
Shared((Arc<ProgressBar>, u64, usize, Arc<RwLock<Vec<u64>>>)),
}
Expand All @@ -95,6 +96,12 @@ impl ProgressBarMode {

/// Returns whether a progress bar should be displayed during download.
pub fn response(&self) -> bool {
matches!(*self, ProgressBarMode::Response)
#[cfg(not(feature = "managed"))]
let rv = matches!(*self, ProgressBarMode::Response);

#[cfg(feature = "managed")]
let rv = false;

rv
}
}
20 changes: 15 additions & 5 deletions src/utils/update.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
#![allow(dead_code)] // a ton of functions/fields might be unused based on the `managed` feature

#[cfg(not(feature = "managed"))]
use std::env;
use std::fs;
use std::io;
use std::io::Write;
#[cfg(not(feature = "managed"))]
use std::path::Path;

use anyhow::{bail, format_err, Result};
#[cfg(not(feature = "managed"))]
use anyhow::bail;
use anyhow::{format_err, Result};
use chrono::{DateTime, Duration, Utc};
use console::{style, user_attended};
use if_chain::if_chain;
Expand All @@ -17,8 +19,11 @@ use serde::{Deserialize, Serialize};
use crate::api::{Api, SentryCliRelease};
use crate::config::Config;
use crate::constants::{APP_NAME, VERSION};
#[cfg(not(feature = "managed"))]
use crate::utils::fs::{is_writable, set_executable_mode};
use crate::utils::system::{is_homebrew_install, is_npm_install, QuietExit};
#[cfg(not(feature = "managed"))]
use crate::utils::system::QuietExit;
use crate::utils::system::{is_homebrew_install, is_npm_install};

#[cfg(windows)]
fn rename_exe(exe: &Path, downloaded_path: &Path, elevate: bool) -> Result<()> {
Expand Down Expand Up @@ -52,7 +57,7 @@ fn rename_exe(exe: &Path, downloaded_path: &Path, elevate: bool) -> Result<()> {
Ok(())
}

#[cfg(not(windows))]
#[cfg(not(any(feature = "managed", windows)))]
fn rename_exe(exe: &Path, downloaded_path: &Path, elevate: bool) -> Result<()> {
if elevate {
println!("Need to sudo to overwrite {}", exe.display());
Expand Down Expand Up @@ -119,6 +124,7 @@ impl SentryCliUpdateInfo {
self.latest_release.is_some()
}

#[cfg(not(feature = "managed"))]
pub fn is_latest_version(&self) -> bool {
self.latest_version() == VERSION
}
Expand All @@ -135,6 +141,7 @@ impl SentryCliUpdateInfo {
}
}

#[cfg(not(feature = "managed"))]
pub fn download_url(&self) -> Result<&str> {
if let Some(ref rel) = self.latest_release {
Ok(rel.download_url.as_str())
Expand All @@ -143,6 +150,7 @@ impl SentryCliUpdateInfo {
}
}

#[cfg(not(feature = "managed"))]
pub fn download(&self) -> Result<()> {
let exe = env::current_exe()?;
let elevate = !is_writable(&exe);
Expand Down Expand Up @@ -175,10 +183,12 @@ pub fn get_latest_sentrycli_release() -> Result<SentryCliUpdateInfo> {
})
}

#[cfg(not(feature = "managed"))]
pub fn can_update_sentrycli() -> bool {
!is_homebrew_install() && !is_npm_install()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether this function is even necessary. Ideally, we would distribute to package managers the managed build, so that this function would always return true.

}

#[cfg(not(feature = "managed"))]
pub fn assert_updatable() -> Result<()> {
if is_homebrew_install() {
println!("This installation of sentry-cli is managed through homebrew");
Expand Down
Loading