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

Support executables that aren't in an archive #138

Open
binyomen opened this issue Oct 14, 2022 · 6 comments · May be fixed by #157
Open

Support executables that aren't in an archive #138

binyomen opened this issue Oct 14, 2022 · 6 comments · May be fixed by #157
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers hacktoberfest https://hacktoberfest.com/
Milestone

Comments

@binyomen
Copy link

I'm thinking this would be useful for repos like marksman which store the executables as top-level assets rather than in archives.

Thanks! I was about to make this tool myself when I saw someone had just recently started it!

@chshersh chshersh added enhancement New feature or request good first issue Good for newcomers hacktoberfest https://hacktoberfest.com/ labels Oct 14, 2022
@chshersh
Copy link
Owner

@binyomen That sounds like a good idea! tool-sync already supports .exe executables on Windows that aren't in the archive.

  • let exe_file = asset_name.strip_suffix(".exe");
    exe_file.map(|_| Archive {
    archive_path,
    tmp_dir,
    exe_name,
    archive_type: ArchiveType::Exe(asset_name),
    })
    }

But it makes sense to support them from other platforms as well 🙂

@binyomen
Copy link
Author

binyomen commented Oct 14, 2022

Huh, weird, that doesn't seem to work for me. With the config:

store_directory = '~/bin'

[marksman]
owner = 'artempyanykh'
repo = 'marksman'
asset_name.windows = 'marksman.exe'

I get the output:

🔄  All done!
📦  Estimated total download size: 16.93 MiB
⛔  marksman 2022-09-13 [error] The system cannot find the file specified. (os error 2)                                 ✨  Successfully installed 0 tools!
📁  Installation directory: C:\Users\beweedon/bin

This is on Windows 11 and tool-sync version 0.2.0.

@chshersh
Copy link
Owner

@binyomen Huh, that is weird. I'd expect this work. I don't have access to Windows so I would start debugging this issue by adding a test for downloading marksman similar to the ripgrep one:

- if: matrix.os == 'windows-latest'
name: "Integration test: [windows] [sync-ripgrep]"
env:
SYNC_DIR: "sync-ripgrep"
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
mkdir $env:SYNC_DIR
cargo run -- --config="tests\$env:SYNC_DIR.toml" sync
ls -l $env:SYNC_DIR
if (!(Test-Path $env:SYNC_DIR\rg.exe)) {
throw 'error on rg.exe'
}

At this point, I'm not even sure where exactly this is coming from so a better investigation is required 🕵🏻

@chshersh chshersh added the bug Something isn't working label Oct 15, 2022
@chshersh chshersh added this to the v0.3.0: Auto milestone Oct 15, 2022
@binyomen
Copy link
Author

Thanks for the tip! I'll have a look at this when I get the chance!

@hdhoang
Copy link
Contributor

hdhoang commented Oct 19, 2022

somehow changing copy_file argument copies the exe correctly:

                Ok(_tool_path) => {
                    copy_file(
                        download_info.archive_path,
                        self.store_directory,
                        &tool_asset.exe_name,
                    )?;

and it still extracts+copies the native-support executables. we need to branch this based on ArchiveType, i think.

Some types in sync modules can implement Debug to ease printing them out for investigating.

@binyomen
Copy link
Author

binyomen commented Nov 5, 2022

It looks like the issue is that for marksman for example, the path passed to copy_file is just marksman.exe rather than the whole path (for tool it's C:\Users\binyomen\AppData\Local\Temp\tool-sync.AeusgFEbnvL2\tool.exe).

I'm working on a fix now.

@binyomen binyomen linked a pull request Nov 5, 2022 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers hacktoberfest https://hacktoberfest.com/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants