Open slyshykO opened 1 year ago
MitchellBerend
commented
1 year ago Hi @slyshykO thanks for creating this issue, can you supply your tools.toml config file?
I think most (if not all) current contributers work on unix, so feedback on windows issues is very helpful. They can be incorporated in ci so they are caught earlier in the development process.
slyshykO
commented
1 year ago This is my config.
The store_directory is set to $HOME/bin. The tool is also located here in the $HOME/bin.
MitchellBerend
commented
1 year ago Are you on WSL? I did not know windows also had a $HOME environment variable.
For future reference, this is their config file
# This configuration is automatically generated by tool-sync 0.2.0
# https://github.com/chshersh/tool-sync
#######################################
#
# Installation directory for all the tools:
store_directory = "$HOME/bin"
#
# tool-sync provides native support for some of the tools without the need to
# configure them. Uncomment all the tools you want to install with a single
# 'tool sync' command:
#
# [bat]
# [difftastic]
# [exa]
[fd]
# [github]
[ripgrep]
[tool-sync]
#
# You can configure the installation of any tool by specifying corresponding options:
#
# [ripgrep] # Name of the tool (new or one of the hardcoded to override default settings)
# owner = "BurntSushi" # GitHub repository owner
# repo = "ripgrep" # GitHub repository name
# exe_name = "rg" # Executable name inside the asset
# Uncomment to download a specific version or tag.
# Without this tag latest will be used
# tag = "13.0.0"
# Asset name to download on linux OSes
# asset_name.linux = "x86_64-unknown-linux-musl"
# 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"@slyshykO Can you please try it with this config, there might be an issue with the way the windows config is hardcoded. This overwrites the hardcoded version.
# # tool-sync default configuration file
# https://github.com/chshersh/tool-sync
# This file was automatically generated by tool-sync
#####################################################
#
#
store_directory = "$HOME/bin"
#
# tool-sync provides native support for some of the tools without the need to
# configure them. Uncomment all the tools you want to install with a single
# 'tool sync' command:
#
# [bat]
# [difftastic]
# [exa]
[fd]
# [github]
[ripgrep]
[tool-sync]
owner = "chshersh" # GitHub repository owner
repo = "tool-sync" # GitHub repository name
exe_name = "tool" # Executable name inside the asset
# Uncomment to download a specific version or tag.
# Without this tag latest will be used
# tag = "13.0.0"
# Asset name to download on linux OSes
asset_name.windows = "x86_64-pc-windows-msvc.zip"
#
# You can configure the installation of any tool by specifying corresponding options:
#
# [ripgrep] # Name of the tool (new or one of the hardcoded to override default settings)
# owner = "BurntSushi" # GitHub repository owner
# repo = "ripgrep" # GitHub repository name
# exe_name = "rg" # Executable name inside the asset
# Uncomment to download a specific version or tag.
# Without this tag latest will be used
# tag = "13.0.0"
# Asset name to download on linux OSes
# asset_name.linux = "x86_64-unknown-linux-musl"
# 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"@MitchellBerend Try to run with changed config - same error. I think the problem is that it is impossible to rewrite the file that is used by the system. So the tool can't rewrite itself cause it running at that time.
MitchellBerend
commented
1 year ago Mhh okay I don't know enough about windows to help with that, is there a common way to do self update on windows?
Edit: a quick google search tells me others get around this by renaming their currently running program so there is no name collision. Is this something that is easy to do?
chshersh
commented
1 year ago I wasn't aware Windows has such a problem 😮
I don't know how to fix it, this requires some investigation. Any help is appreciated! 🙏🏻 But it would be nice to include the fix for this in the next version.
FrancisMurillo
commented
1 year ago Searching for answers, I think trying to support this on Windows might not be feasible without increasing complexity. My suggestion is to warn users on Windows that tool-sync cannot be self-updated and should go thru other means by:
tool-sync is in the configuration then issue an error message
tool-sync installtool-sync is about to be downloaded, output a warning message and NOOP insteadchshersh
commented
1 year ago So, I've asked about the potential solution on Twitter and, apparently, there's a simple way. Windows doesn't allow updating the executable while it's running. However (and this is weird) it allows renaming it (by moving to a different directory or just changing a name).
So, the simple solution to this problem seems to be following:
tool executable into a temporary directory
std::env::current_exe for getting path to the currently running tool executablestd::env::temp_dir to get the path to the system temporary directoryfs::rename to store it in the temp dir. This way it'll be automatically deleted by the system after reboot.
So, yes, this logic should be only on Windows (conditional compilation) and only for tool-sync because it's a special case.
binyomen
commented
1 year ago For what it's worth, here's my understanding of why this happens:
Linux/unix has a concept of "unlinking" rather than "deleting" files. This basically removes the file from the directory tree, but it doesn't delete the file object (inode^inode) in the kernel or free the file's data on disk until all open file descriptors are closed and hard links are unlinked. Basically each inode has a reference count consisting of all open file descriptors and all hard links (including the hard link you might consider the original path to the file). This means you can unlink the executable file for a running process on unix without the actual inode being deleted yet.
Windows doesn't really have this concept of unlinking. There is FILE_SHARE_DELETE^create-file, which is an attribute you can set on an opened file that does something sort of similar to unix's ref-counted inodes, although I'm not entirely sure of the specifics. It doesn't seem like the loader for .exe and .dll files sets this attribute, and even if it did older versions of NTFS would keep a tombstone around^ntfs, preventing you from replacing the file with one with the same name. I'm not entirely sure why the loader doesn't specify FILE_SHARE_DELETE. I know windows uses the actual PE images (the format of .exe, .dll, and .sys files) as page files for text/static data at runtime, but assuming it ref-counts the actual file object I don't think this should provide a reason not to delete them? Unsure.
The reason why renaming works is because you're not actually deleting the underlying file data, you're just moving it to a new location in the directory tree. This doesn't require special permissions since it does not remove the file from disk.
My current workaround is this powershell script:
# tool-sync.ps1
$local:ErrorActionPreference = 'Stop'
$currentToolPath = (Get-Command tool).Source
$newToolDir = "$env:TEMP/$((New-Guid).Guid)"
$newToolPath = "$newToolDir/tool.exe"
mkdir $newToolDir > $null
Copy-Item $currentToolPath $newToolDir
& $newToolPath sync
Remove-Item -Force -Recurse $newToolDirIs anybody currently working on this? If not, I'd be happy to take it on.
binyomen
commented
1 year ago Actually, I just set this up on Linux for the first time, and self update doesn't seem to be working there either. I'm getting [error] Text file busy (os error 26). My guess is that we're copying files using std::fs::copy, which opens the destination file as writable and writes bytes to it, whereas what we should be doing (I think) is actually removing the destination file and then copying the new version to that location. Is anyone else seeing this? It seems we have a test in CI to validate this, but it runs using cargo run which means it's not actually overwriting the same file that's running.
MitchellBerend
commented
1 year ago I'll try and test this on my machine, I haven't actually self updated yet so I'm not sure.
MitchellBerend
commented
1 year ago @binyomen It doesn't actually work for me either. If what you are saying is the case the ci needs to be changed as well so it does not give a false positive.
~/rust/tool-sync on main [$?] is 📦 v0.2.0 via 🦀 v1.65.0
➜ ./bin/tool --config tool.toml
tool-sync 0.2.0
Dmitrii Kovanikov <kovanikov@gmail.com>
A CLI tool to manage other CLI tools
USAGE:
tool [OPTIONS] <SUBCOMMAND>
OPTIONS:
-c, --config <FILE> Sets a path to a configuration file (default: $HOME/.tool.toml)
-h, --help Print help information
-V, --version Print version information
SUBCOMMANDS:
default-config Generate a default .tool.toml file and prints it to std out
help Print this message or the help of the given subcommand(s)
install Install a tool if it is hardcoded into internal database
sync Sync all tools specified in configuration file
~/rust/tool-sync on main [$?] is 📦 v0.2.0 via 🦀 v1.65.0
❯ ./bin/tool --config tool.toml sync
🔄 All done! 📦 Estimated total download size: 1.44 MiB
⛔ tool-sync v0.2.0 [error] Text file busy (os error 26) ✨ Successfully installed 0 tools!
📁 Installation directory: /home/mitchell/rust/tool-sync/binSorry for the radio silence on this. I can work on both self update in unix and windows then.
FrancisMurillo
commented
1 year ago A self_replace crate for this task might be able to resolve this now.
Is it possible to sync the tool from the tool under windows? I am getting the error: