pull: fix hang caused by blocking git2 call and redundant Launchpad checks
This commit is contained in:
+99
-26
@@ -19,7 +19,7 @@ pub fn ppa_to_base_url(user: &str, name: &str) -> String {
|
|||||||
format!("http://ppa.launchpadcontent.net/{}/{}/ubuntu", user, name)
|
format!("http://ppa.launchpadcontent.net/{}/{}/ubuntu", user, name)
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn check_launchpad_repo(package: &str) -> Result<Option<String>, Box<dyn Error>> {
|
fn check_launchpad_repo_sync(package: &str) -> Result<Option<String>, String> {
|
||||||
let url = format!("https://git.launchpad.net/ubuntu/+source/{}", package);
|
let url = format!("https://git.launchpad.net/ubuntu/+source/{}", package);
|
||||||
|
|
||||||
// Use libgit2 to check if the remote repository exists
|
// Use libgit2 to check if the remote repository exists
|
||||||
@@ -33,6 +33,31 @@ async fn check_launchpad_repo(package: &str) -> Result<Option<String>, Box<dyn E
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Check if a Launchpad git repository exists for the given package.
|
||||||
|
///
|
||||||
|
/// This runs the blocking git2 connection check on a dedicated thread with a
|
||||||
|
/// timeout so that it never stalls the tokio runtime (the git2 connect call
|
||||||
|
/// has no built-in timeout and can hang for minutes on unreachable hosts).
|
||||||
|
async fn check_launchpad_repo(
|
||||||
|
package: &str,
|
||||||
|
) -> Result<Option<String>, Box<dyn Error + Send + Sync>> {
|
||||||
|
let package_owned = package.to_string();
|
||||||
|
let package_for_err = package_owned.clone();
|
||||||
|
let result = tokio::time::timeout(
|
||||||
|
std::time::Duration::from_secs(15),
|
||||||
|
tokio::task::spawn_blocking(move || check_launchpad_repo_sync(&package_owned)),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map_err(|_| {
|
||||||
|
format!(
|
||||||
|
"Timed out checking Launchpad repository for '{}'",
|
||||||
|
package_for_err
|
||||||
|
)
|
||||||
|
})?
|
||||||
|
.map_err(|e| format!("Launchpad check task failed: {}", e))?;
|
||||||
|
result.map_err(|e| -> Box<dyn Error + Send + Sync> { e.into() })
|
||||||
|
}
|
||||||
|
|
||||||
/// A File used in a source package
|
/// A File used in a source package
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
pub struct FileEntry {
|
pub struct FileEntry {
|
||||||
@@ -245,24 +270,24 @@ fn parse_sources(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Get package information from a package, distribution series, and pocket
|
/// Get package information from a package, distribution series, and pocket
|
||||||
|
///
|
||||||
|
/// `launchpad_vcs` is a pre-computed Launchpad VCS URL (if any), checked once
|
||||||
|
/// by the caller so that the blocking git2 connection test is not repeated for
|
||||||
|
/// every series/pocket combination.
|
||||||
async fn get(
|
async fn get(
|
||||||
package_name: &str,
|
package_name: &str,
|
||||||
series: &str,
|
series: &str,
|
||||||
pocket: &str,
|
pocket: &str,
|
||||||
version: Option<&str>,
|
version: Option<&str>,
|
||||||
base_url: Option<&str>,
|
base_url: Option<&str>,
|
||||||
|
launchpad_vcs: Option<String>,
|
||||||
) -> Result<PackageInfo, Box<dyn Error>> {
|
) -> Result<PackageInfo, Box<dyn Error>> {
|
||||||
let dist = crate::distro_info::get_dist_from_series(series).await?;
|
let dist = crate::distro_info::get_dist_from_series(series).await?;
|
||||||
|
|
||||||
// Handle Ubuntu case: Vcs-Git does not usually point to Launchpad but Salsa
|
// Use the pre-computed Launchpad VCS URL (if any).
|
||||||
// We need to check manually if there is a launchpad repository for the package
|
// This is checked once in the caller rather than per-series to avoid
|
||||||
let mut preferred_vcs = None;
|
// repeated blocking git2 connections.
|
||||||
if dist == "ubuntu"
|
let mut preferred_vcs = launchpad_vcs;
|
||||||
&& let Some(lp_url) = check_launchpad_repo(package_name).await?
|
|
||||||
{
|
|
||||||
debug!("Found Launchpad URL: {}", lp_url);
|
|
||||||
preferred_vcs = Some(lp_url);
|
|
||||||
}
|
|
||||||
|
|
||||||
// Determine the base URL to use (either provided PPA URL or default archive)
|
// Determine the base URL to use (either provided PPA URL or default archive)
|
||||||
let distro_base_url = crate::distro_info::get_base_url(&dist)?;
|
let distro_base_url = crate::distro_info::get_base_url(&dist)?;
|
||||||
@@ -340,6 +365,7 @@ async fn find_package(
|
|||||||
pocket: &str,
|
pocket: &str,
|
||||||
version: Option<&str>,
|
version: Option<&str>,
|
||||||
base_url: Option<&str>,
|
base_url: Option<&str>,
|
||||||
|
launchpad_vcs: Option<String>,
|
||||||
progress: ProgressCallback<'_>,
|
progress: ProgressCallback<'_>,
|
||||||
) -> Result<PackageInfo, Box<dyn Error>> {
|
) -> Result<PackageInfo, Box<dyn Error>> {
|
||||||
let series_list = crate::distro_info::get_ordered_series_name(dist).await?;
|
let series_list = crate::distro_info::get_ordered_series_name(dist).await?;
|
||||||
@@ -356,7 +382,16 @@ async fn find_package(
|
|||||||
};
|
};
|
||||||
|
|
||||||
for p in pockets {
|
for p in pockets {
|
||||||
match get(package_name, series, &p, version, base_url).await {
|
match get(
|
||||||
|
package_name,
|
||||||
|
series,
|
||||||
|
&p,
|
||||||
|
version,
|
||||||
|
base_url,
|
||||||
|
launchpad_vcs.clone(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
{
|
||||||
Ok(info) => {
|
Ok(info) => {
|
||||||
if i > 0 {
|
if i > 0 {
|
||||||
let location = if p.is_empty() {
|
let location = if p.is_empty() {
|
||||||
@@ -410,6 +445,41 @@ pub async fn lookup(
|
|||||||
base_url: Option<&str>,
|
base_url: Option<&str>,
|
||||||
progress: ProgressCallback<'_>,
|
progress: ProgressCallback<'_>,
|
||||||
) -> Result<PackageInfo, Box<dyn Error>> {
|
) -> Result<PackageInfo, Box<dyn Error>> {
|
||||||
|
// Resolve the distribution early so we can check Launchpad once
|
||||||
|
let resolved_dist = dist.unwrap_or_else(||
|
||||||
|
// Use auto-detection to see if current distro is ubuntu, or fallback to debian by default
|
||||||
|
if std::process::Command::new("lsb_release").arg("-i").arg("-s").output()
|
||||||
|
.map(|o| String::from_utf8_lossy(&o.stdout).trim().to_lowercase()).unwrap_or_default() == "ubuntu" {
|
||||||
|
"ubuntu"
|
||||||
|
} else {
|
||||||
|
"debian"
|
||||||
|
}
|
||||||
|
);
|
||||||
|
|
||||||
|
// Check Launchpad repository once for Ubuntu packages, rather than
|
||||||
|
// once per series/pocket combination inside get().
|
||||||
|
let launchpad_vcs = if resolved_dist == "ubuntu" && base_url.is_none() {
|
||||||
|
if let Some(cb) = progress {
|
||||||
|
cb("", &format!("Checking Launchpad for {}...", package), 0, 0);
|
||||||
|
}
|
||||||
|
match check_launchpad_repo(package).await {
|
||||||
|
Ok(Some(lp_url)) => {
|
||||||
|
debug!("Found Launchpad URL: {}", lp_url);
|
||||||
|
Some(lp_url)
|
||||||
|
}
|
||||||
|
Ok(None) => None,
|
||||||
|
Err(e) => {
|
||||||
|
debug!(
|
||||||
|
"Launchpad check failed (will use stanza Vcs-Git if available): {}",
|
||||||
|
e
|
||||||
|
);
|
||||||
|
None
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
|
||||||
// Obtain the package information, either directly in a series or with a search in all series
|
// Obtain the package information, either directly in a series or with a search in all series
|
||||||
let package_info = if let Some(s) = series {
|
let package_info = if let Some(s) = series {
|
||||||
if let Some(cb) = progress {
|
if let Some(cb) = progress {
|
||||||
@@ -422,24 +492,18 @@ pub async fn lookup(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Get the package information from that series and pocket
|
// Get the package information from that series and pocket
|
||||||
get(package, s, pocket, version, base_url).await?
|
get(package, s, pocket, version, base_url, launchpad_vcs).await?
|
||||||
} else {
|
} else {
|
||||||
let dist = dist.unwrap_or_else(||
|
|
||||||
// Use auto-detection to see if current distro is ubuntu, or fallback to debian by default
|
|
||||||
if std::process::Command::new("lsb_release").arg("-i").arg("-s").output()
|
|
||||||
.map(|o| String::from_utf8_lossy(&o.stdout).trim().to_lowercase()).unwrap_or_default() == "ubuntu" {
|
|
||||||
"ubuntu"
|
|
||||||
} else {
|
|
||||||
"debian"
|
|
||||||
}
|
|
||||||
);
|
|
||||||
|
|
||||||
if let Some(cb) = progress {
|
if let Some(cb) = progress {
|
||||||
cb(
|
cb(
|
||||||
&format!(
|
&format!(
|
||||||
"Searching for package {} in {}...",
|
"Searching for package {} in {}...",
|
||||||
package,
|
package,
|
||||||
if base_url.is_none() { dist } else { "ppa" }
|
if base_url.is_none() {
|
||||||
|
resolved_dist
|
||||||
|
} else {
|
||||||
|
"ppa"
|
||||||
|
}
|
||||||
),
|
),
|
||||||
"",
|
"",
|
||||||
0,
|
0,
|
||||||
@@ -448,7 +512,16 @@ pub async fn lookup(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Try to find the package in all series from that dist
|
// Try to find the package in all series from that dist
|
||||||
find_package(package, dist, pocket, version, base_url, progress).await?
|
find_package(
|
||||||
|
package,
|
||||||
|
resolved_dist,
|
||||||
|
pocket,
|
||||||
|
version,
|
||||||
|
base_url,
|
||||||
|
launchpad_vcs,
|
||||||
|
progress,
|
||||||
|
)
|
||||||
|
.await?
|
||||||
};
|
};
|
||||||
|
|
||||||
Ok(package_info)
|
Ok(package_info)
|
||||||
@@ -512,7 +585,7 @@ Version: 1.0
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_find_package_fallback() {
|
async fn test_find_package_fallback() {
|
||||||
// python2.7 is in bullseye but not above
|
// python2.7 is in bullseye but not above
|
||||||
let info = find_package("python2.7", "debian", "", None, None, None)
|
let info = find_package("python2.7", "debian", "", None, None, None, None)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(info.stanza.package, "python2.7");
|
assert_eq!(info.stanza.package, "python2.7");
|
||||||
@@ -522,7 +595,7 @@ Version: 1.0
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_find_package_devel() {
|
async fn test_find_package_devel() {
|
||||||
// hello is in sid
|
// hello is in sid
|
||||||
let info = find_package("hello", "debian", "", None, None, None)
|
let info = find_package("hello", "debian", "", None, None, None, None)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(info.stanza.package, "hello");
|
assert_eq!(info.stanza.package, "hello");
|
||||||
|
|||||||
Reference in New Issue
Block a user