Skip to content

Commit

Permalink
fix: download failure when remote file is missing and add support for…
Browse files Browse the repository at this point in the history
… tar.gz archives (#1195)

* fix: download failure when remote file is missing

If the .tar.xz download fails, it will try again with the .tar.gz file.

Fixes #1088

* Update changeset

* Update changeset

* fix clippy warnings and add test

* add snapshot

* fix types

* skip on windows

* don't have obsolete snapshots

---------

Co-authored-by: Gal Schlezinger <[email protected]>
  • Loading branch information
mattmess1221 and Schniz authored Oct 6, 2024
1 parent d0ed740 commit 74d7c33
Show file tree
Hide file tree
Showing 12 changed files with 209 additions and 81 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-falcons-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fnm": patch
---

When downloading from node-dist, Fallback to .tar.gz download when the .tar.xz download fails
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ thiserror = "1.0.61"
clap_complete = "4.5.2"
anyhow = "1.0.86"
indicatif = { version = "0.17.8", features = ["improved_unicode"] }
flate2 = "1.0.30"

[dev-dependencies]
pretty_assertions = "1.4.0"
Expand Down
42 changes: 42 additions & 0 deletions e2e/__snapshots__/old-versions.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Bash download old Node.js 0.10.x: Bash 1`] = `
"set -e
eval "$(fnm env)"
fnm install v0.10.36
fnm use v0.10.36
if [ "$(node --version)" != "v0.10.36" ]; then
echo "Expected node version to be v0.10.36. Got $(node --version)"
exit 1
fi"
`;

exports[`Fish download old Node.js 0.10.x: Fish 1`] = `
"fnm env | source
fnm install v0.10.36
fnm use v0.10.36
set ____test____ (node --version)
if test "$____test____" != "v0.10.36"
echo "Expected node version to be v0.10.36. Got $____test____"
exit 1
end"
`;

exports[`PowerShell download old Node.js 0.10.x: PowerShell 1`] = `
"$ErrorActionPreference = "Stop"
fnm env | Out-String | Invoke-Expression
fnm install v0.10.36
fnm use v0.10.36
if ( "$(node --version)" -ne "v0.10.36" ) { echo "Expected node version to be v0.10.36. Got $(node --version)"; exit 1 }"
`;

exports[`Zsh download old Node.js 0.10.x: Zsh 1`] = `
"set -e
eval "$(fnm env)"
fnm install v0.10.36
fnm use v0.10.36
if [ "$(node --version)" != "v0.10.36" ]; then
echo "Expected node version to be v0.10.36. Got $(node --version)"
exit 1
fi"
`;
24 changes: 24 additions & 0 deletions e2e/old-versions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { script } from "./shellcode/script.js"
import { Bash, Fish, PowerShell, WinCmd, Zsh } from "./shellcode/shells.js"
import testNodeVersion from "./shellcode/test-node-version.js"
import describe from "./describe.js"
import os from "node:os"

for (const shell of [Bash, Zsh, Fish, PowerShell, WinCmd]) {
describe(shell, () => {
test(`download old Node.js 0.10.x`, async () => {
const testCase = script(shell)
.then(shell.env({}))
.then(shell.call("fnm", ["install", "v0.10.36"]))
.then(shell.call("fnm", ["use", "v0.10.36"]))
.then(testNodeVersion(shell, "v0.10.36"))
.takeSnapshot(shell)

if (os.platform() === "win32") {
console.warn(`test skipped as 0.10.x isn't prebuilt for windows`)
} else {
await testCase.execute(shell)
}
})
})
}
2 changes: 1 addition & 1 deletion src/archive/extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,5 @@ impl From<crate::http::Error> for Error {
}

pub trait Extract {
fn extract_into<P: AsRef<Path>>(self, path: P) -> Result<(), Error>;
fn extract_into(self: Box<Self>, path: &Path) -> Result<(), Error>;
}
55 changes: 51 additions & 4 deletions src/archive/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,58 @@
pub mod extract;
pub mod tar_xz;
pub mod tar;
pub mod zip;

pub use self::extract::{Error, Extract};
use std::io::Read;
use std::path::Path;

pub use self::extract::{Error, Extract};
#[cfg(unix)]
pub use self::tar_xz::TarXz;
use self::tar::Tar;

#[cfg(windows)]
pub use self::zip::Zip;
use self::zip::Zip;

pub enum Archive {
#[cfg(windows)]
Zip,
#[cfg(unix)]
TarXz,
#[cfg(unix)]
TarGz,
}

impl Archive {
pub fn extract_archive_into(&self, path: &Path, response: impl Read) -> Result<(), Error> {
let extractor: Box<dyn Extract> = match self {
#[cfg(windows)]
Self::Zip => Box::new(Zip::new(response)),
#[cfg(unix)]
Self::TarXz => Box::new(Tar::Xz(response)),
#[cfg(unix)]
Self::TarGz => Box::new(Tar::Gz(response)),
};
extractor.extract_into(path)?;
Ok(())
}

pub fn file_extension(&self) -> &'static str {
match self {
#[cfg(windows)]
Self::Zip => "zip",
#[cfg(unix)]
Self::TarXz => "tar.xz",
#[cfg(unix)]
Self::TarGz => "tar.gz",
}
}

#[cfg(windows)]
pub fn supported() -> &'static [Self] {
&[Self::Zip]
}

#[cfg(unix)]
pub fn supported() -> &'static [Self] {
&[Self::TarXz, Self::TarGz]
}
}
27 changes: 27 additions & 0 deletions src/archive/tar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
use super::{extract::Error, Extract};
use std::{io::Read, path::Path};

pub enum Tar<R: Read> {
/// Tar archive with XZ compression
Xz(R),

Check warning on line 6 in src/archive/tar.rs

View workflow job for this annotation

GitHub Actions / Release build for Windows

variants `Xz` and `Gz` are never constructed

Check warning on line 6 in src/archive/tar.rs

View workflow job for this annotation

GitHub Actions / unit_tests (windows-latest)

variants `Xz` and `Gz` are never constructed
/// Tar archive with Gzip compression
Gz(R),
}

impl<R: Read> Tar<R> {
fn extract_into_impl<P: AsRef<Path>>(self, path: P) -> Result<(), Error> {
let stream: Box<dyn Read> = match self {
Self::Xz(response) => Box::new(xz2::read::XzDecoder::new(response)),
Self::Gz(response) => Box::new(flate2::read::GzDecoder::new(response)),
};
let mut tar_archive = tar::Archive::new(stream);
tar_archive.unpack(&path)?;
Ok(())
}
}

impl<R: Read> Extract for Tar<R> {
fn extract_into(self: Box<Self>, path: &Path) -> Result<(), Error> {
self.extract_into_impl(path)
}
}
22 changes: 0 additions & 22 deletions src/archive/tar_xz.rs

This file was deleted.

9 changes: 4 additions & 5 deletions src/archive/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ impl<R: Read> Zip<R> {
}

impl<R: Read> Extract for Zip<R> {
fn extract_into<P: AsRef<Path>>(mut self, path: P) -> Result<(), Error> {
let path = path.as_ref();
fn extract_into(mut self: Box<Self>, path: &Path) -> Result<(), Error> {
let mut tmp_zip_file = tempfile().expect("Can't get a temporary file");

debug!("Created a temporary zip file");
Expand Down Expand Up @@ -86,11 +85,11 @@ mod tests {

#[test_log::test]
fn test_zip_extraction() {
let temp_dir = tempfile::tempdir().expect("Can't create a temp directory");
let temp_dir = &tempfile::tempdir().expect("Can't create a temp directory");
let response = crate::http::get("https://nodejs.org/dist/v12.0.0/node-v12.0.0-win-x64.zip")
.expect("Can't make request to Node v12.0.0 zip file");
Zip::new(response)
.extract_into(&temp_dir)
Box::new(Zip::new(response))
.extract_into(temp_dir.as_ref())
.expect("Can't unzip files");
let node_file = temp_dir
.as_ref()
Expand Down
90 changes: 45 additions & 45 deletions src/downloader.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
use crate::arch::Arch;
use crate::archive;
use crate::archive::{Error as ExtractError, Extract};
use crate::archive::{Archive, Error as ExtractError};
use crate::directory_portal::DirectoryPortal;
use crate::progress::ResponseProgress;
use crate::version::Version;
use indicatif::ProgressDrawTarget;
use log::debug;
use std::io::Read;
use std::path::Path;
use std::path::PathBuf;
use thiserror::Error;
Expand Down Expand Up @@ -38,39 +36,36 @@ pub enum Error {
}

#[cfg(unix)]
fn filename_for_version(version: &Version, arch: Arch) -> String {
fn filename_for_version(version: &Version, arch: Arch, ext: &str) -> String {
format!(
"node-{node_ver}-{platform}-{arch}.tar.xz",
"node-{node_ver}-{platform}-{arch}.{ext}",
node_ver = &version,
platform = crate::system_info::platform_name(),
arch = arch,
ext = ext
)
}

#[cfg(windows)]
fn filename_for_version(version: &Version, arch: Arch) -> String {
format!("node-{node_ver}-win-{arch}.zip", node_ver = &version)
fn filename_for_version(version: &Version, arch: Arch, ext: &str) -> String {
format!(
"node-{node_ver}-win-{arch}.{ext}",
node_ver = &version,
arch = arch,
ext = ext,
)
}

fn download_url(base_url: &Url, version: &Version, arch: Arch) -> Url {
fn download_url(base_url: &Url, version: &Version, arch: Arch, ext: &str) -> Url {
Url::parse(&format!(
"{}/{}/{}",
base_url.as_str().trim_end_matches('/'),
version,
filename_for_version(version, arch)
filename_for_version(version, arch, ext)
))
.unwrap()
}

fn extract_archive_into(path: impl AsRef<Path>, response: impl Read) -> Result<(), Error> {
#[cfg(unix)]
let extractor = archive::TarXz::new(response);
#[cfg(windows)]
let extractor = archive::Zip::new(response);
extractor.extract_into(path)?;
Ok(())
}

/// Install a Node package
pub fn install_node_dist<P: AsRef<Path>>(
version: &Version,
Expand All @@ -94,39 +89,44 @@ pub fn install_node_dist<P: AsRef<Path>>(

let portal = DirectoryPortal::new_in(&temp_installations_dir, installation_dir);

let url = download_url(node_dist_mirror, version, arch);
debug!("Going to call for {}", &url);
let response = crate::http::get(url.as_str())?;
for extract in Archive::supported() {
let ext = extract.file_extension();
let url = download_url(node_dist_mirror, version, arch, ext);
debug!("Going to call for {}", &url);
let response = crate::http::get(url.as_str())?;

if response.status() == 404 {
return Err(Error::VersionNotFound {
version: version.clone(),
arch,
});
}
if !response.status().is_success() {
continue;
}

debug!("Extracting response...");
if show_progress {
extract_archive_into(
&portal,
ResponseProgress::new(response, ProgressDrawTarget::stderr()),
)?;
} else {
extract_archive_into(&portal, response)?;
}
debug!("Extraction completed");
debug!("Extracting response...");
if show_progress {
extract.extract_archive_into(
portal.as_ref(),
ResponseProgress::new(response, ProgressDrawTarget::stderr()),
)?;
} else {
extract.extract_archive_into(portal.as_ref(), response)?;
}
debug!("Extraction completed");

let installed_directory = std::fs::read_dir(&portal)?
.next()
.ok_or(Error::TarIsEmpty)??;
let installed_directory = installed_directory.path();

let installed_directory = std::fs::read_dir(&portal)?
.next()
.ok_or(Error::TarIsEmpty)??;
let installed_directory = installed_directory.path();
let renamed_installation_dir = portal.join("installation");
std::fs::rename(installed_directory, renamed_installation_dir)?;

let renamed_installation_dir = portal.join("installation");
std::fs::rename(installed_directory, renamed_installation_dir)?;
portal.teleport()?;

portal.teleport()?;
return Ok(());
}

Ok(())
Err(Error::VersionNotFound {
version: version.clone(),
arch,
})
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 74d7c33

Please sign in to comment.