Support path 'asset_name/bin/<exe_name>' inside assets

[chshersh]

This is needed for some tools. Example:

• https://github.com/cli/cli/releases/tag/v2.14.7

All the search paths are specified here:

tool-sync/src/sync/archive.rs

Lines 144 to 153 in cb339b0

	// List of potential paths where an executable can be inside the archive
	fn exe_paths(exe_name: &str, asset_name: &str) -> Vec<PathBuf> {
	let exe_name = mk_exe_name(exe_name);
	vec![
	[asset_name, &exe_name].iter().collect(),
	[&exe_name].iter().collect(),
	["bin", &exe_name].iter().collect(),
	]
	}

To fix this problem:

• Add one more path search entry
• Add tests for gh

[hgrahamcs]

Where are the current paths tested?

[chshersh]

@hgrahamcs Hmm, good question 🤔

There're no tests for checking this function specifically. This issue came up when I tried downloading the GitHub CLI tool using tool-sync. So, the test would be:

• Add gh to the hardcoded database:

tool-sync/src/sync/db.rs

Line 5 in eeed4bb

pub fn lookup_tool(tool_name: &str) -> Option<ToolInfo> {

• Add gh to the full-database.toml test

tool-sync/tests/full-database.toml

Lines 4 to 10 in eeed4bb

	# the following tools will be installed in 'store_directory'
	[bat]
	[difftastic]
	[exa]
	[fd]
	[ripgrep]
	[tool-sync]

• Add tests on the CI

• tool-sync/.github/workflows/ci.yml

  Lines 77 to 82 in eeed4bb

  	if [[ ! -x $SYNC_DIR/bat ]]; then echo "error on: bat"; false; fi
  	if [[ ! -x $SYNC_DIR/difft ]]; then echo "error on: difft"; false; fi
  	if [[ ! -x $SYNC_DIR/exa ]]; then echo "error on: exa"; false; fi
  	if [[ ! -x $SYNC_DIR/fd ]]; then echo "error on: fd"; false; fi
  	if [[ ! -x $SYNC_DIR/rg ]]; then echo "error on: rg"; false; fi
  	if [[ ! -x $SYNC_DIR/tool ]]; then echo "error on: tool"; false; fi

• tool-sync/.github/workflows/ci.yml

  Lines 98 to 112 in eeed4bb

  	if (!(Test-Path $env:SYNC_DIR\bat.exe)) {
  	throw 'error on bat.exe'
  	}
  	if (!(Test-Path $env:SYNC_DIR\difft.exe)) {
  	throw 'error on difft.exe'
  	}
  	if (!(Test-Path $env:SYNC_DIR\fd.exe)) {
  	throw 'error on fd.exe'
  	}
  	if (!(Test-Path $env:SYNC_DIR\rg.exe)) {
  	throw 'error on rg.exe'
  	}
  	if (!(Test-Path $env:SYNC_DIR\tool.exe)) {
  	throw 'error on tool.exe'
  	}

Adding gh to the database is something I wanted to do anyway as it's a quite popular tool so makes sense to add it 👍🏻

[MitchellBerend]

@hgrahamcs Are you still working on this?

[MitchellBerend]

@chshersh are the tasks in this issue still current or do I need to revise this fully?

[chshersh]

@MitchellBerend The tasks in the issue description and my previous comment are still current 👍🏻

Feel free to take this issue. I'd like to release tool-sync v0.2.0 this week and there're only a few issues left in the v0.2.0 milestone.

[MitchellBerend]

@chshersh What is actually required here? Simply adding this path does not work, it tries to download the .deb blob.
There might be a bug in the prefetch function. I'm not sure why it chooses this .deb file. Maybe because its the first hit in the list?
Do you have some insight here?

$ cargo run -- --config tools.toml sync 
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/tool --config tools.toml sync`
🔄  All done!                                                                                                                                                             📦  Estimated total download size: 7.71 MiB
⛔  github v2.14.7  [error] Unsupported asset type: gh_2.14.7_linux_amd64.deb                                                                                             ✨  Successfully installed 0 tools!
📁  Installation directory: /home/mitchell/rust/tool-sync/bin

$ cat tools.toml
# # tool-sync default configuration file
# https://github.com/chshersh/tool-sync
# This file was automatically generated by tool-sync
#####################################################
#
#
store_directory = "$HOME/rust/tool-sync/bin"
#
# tool-sync provides native support for some of the tools without the need to configure them
# Uncomment the tools you want to have them
#
#[bat]
#[difftastic]
#[fd]
#[ripgrep]
#
# To add configuration for other tools these are the config options:
[github]
    owner     = "cli"
    repo      = "cli"
    exe_name  = "gh"
#
#        # Uncomment to download a specific version or tag.
#        # Without this tag latest will be used
    tag       = "v2.14.7"
#
#
# Asset name to download on linux OSes
    asset_name.linux = "linux_amd64"
#
# uncomment if you want to install on macOS as well
# asset_name.macos = "apple-darwin"
#
# uncomment if you want to install on Windows as well
# asset_name.windows = "x86_64-pc-windows-msvc"

$ git diff
diff --git a/src/sync/archive.rs b/src/sync/archive.rs
index b4e78a4..2fdc2e4 100644
--- a/src/sync/archive.rs
+++ b/src/sync/archive.rs
@@ -150,5 +150,6 @@ fn exe_paths(exe_name: &str, asset_name: &str) -> Vec<PathBuf> {
         [asset_name, &exe_name].iter().collect(),
         [&exe_name].iter().collect(),
         ["bin", &exe_name].iter().collect(),
+        [asset_name, "bin", &exe_name].iter().collect(),
     ]
 }

[chshersh]

@MitchellBerend Unfortunately, gh has several assets matching the string linux_amd64:

• gh_2.14.7_linux_amd64.deb
• gh_2.14.7_linux_amd64.rpm
• gh_2.14.7_linux_amd64.tar.gz

So it's not enough to specify only this string. Try the following instead:

asset_name.linux = "linux_amd64.tar.gz"

The error message [error] Unsupported asset type: gh_2.14.7_linux_amd64.deb is probably not clear as well so it could be improved, I guess. It means that tool-sync supports files only with the .zip, .tar.gz and .exe extensions and not .deb.

---

The logic for finding the asset is here:

tool-sync/src/model/tool.rs

Lines 79 to 84 in 7700be2

	let asset = assets.iter().find(|&asset| asset.name.contains(asset_name));
	match asset {
	None => Err(AssetError::NotFound(asset_name.to_owned())),
	Some(asset) => Ok(asset.clone()),
	}

So yes, it does find the first matching asset. It should be improved to find all and report an error early when several assets match the query string (or be smarter and actually select the one that matches). The only difficulty here is that in this mode all errors should be single-line to be present at the same line as the tool name. But proper error-reporting would require spanning an error message on multiple lines. So error-reporting improvement needs some thinking.

[MitchellBerend]

Okay I added the new path option but I also want to expand the scope of this issue to include that error message.