tldr-pages / tldr-c-client

C command-line client for tldr pages
MIT License
296 stars 49 forks source link

Error when try to update local database #64

Closed robsonpeixoto closed 2 years ago

robsonpeixoto commented 2 years ago
❯ tldr --version
tldr v1.3.0 (v1.3.0)
Copyright (C) 2016 Arvid Gerstmann
Source available at https://github.com/tldr-pages/tldr-c-client

❯ tldr -v -u
main.zip [========================================] 100%
Error: Could Not Rename: /tmp/tldrVRo8lf/tldr to /Users/robinho/.tldrc/tldr/

❯ sw_vers
ProductName:    macOS
ProductVersion: 12.0.1
BuildVersion:   21A559
lwpie commented 2 years ago

21

CleanMachine1 commented 2 years ago

This is almost definitely #59

Did the fix work from #21

glawrence commented 2 years ago

I tried the fix at https://github.com/tldr-pages/tldr-c-client/pull/21#issuecomment-292096705 which did not work for me. However I note that, as above tldr -v -u shows main.zip and the instructions on #21 I just linked to refer to master.zip

marchersimon commented 2 years ago

This is really strange, because I made #59 to fix exactly this issue and it worked for me. Now, I'm getting the same error.

glawrence commented 2 years ago

Argh, how annoying @marchersimon ! I guess on the plus side, from our point of view, it is useful you can reproduce the issue.... Happy to try and help by testing stuff on macOS

marchersimon commented 2 years ago

The main reason this is failing is, because the archive we download is called main.zip, because it's coming from the main branch. The archive contains the directory tldr-main/, so when we extract main.zip, we get tldr-main, which the program doesn't expect. I'll keep you up to date.

marchersimon commented 2 years ago

Update: I figured out the issue. In Linux (and probably macOS too) /tmp has it's own filesystem called tmpfs. We now try to rename the tldr directory in /tmp/tldrXXXX/tldr-main to ~/.tldrc/tldr, which doesn't work, because the rename() command doesn't support that. It set's the errno code 18 - Invalid cross-device link. This issue even existed in the code more than 5 years ago. Honestly, I'm wondering how this worked at all...

I'm currently making a PR to replace the rename() function to a copy() and delete().

glawrence commented 2 years ago

Will it remove ~/.tldrc/tldr-master as part of the process? I am guessing that's not needed, although removing it might stop tldr from working, I guess

marchersimon commented 2 years ago

No, we didn't take care of this yet. Here's more info. It will just stay there until you manually delete it. But it has nothing to do with this issue.

marchersimon commented 2 years ago

@glawrence the fix for macOS is merged now. Could you please verify that it's working by updating your client using the --HEAD opption?

glawrence commented 2 years ago

I started by checking I was still seeing the issue:

Geoffs-MacBook-Pro:~ $ tldr -u
Error: Could Not Rename: /tmp/tldrYmdBuU/tldr to /Users/xxxxx/.tldrc/tldr/
Geoffs-MacBook-Pro:~ $ brew install tldr --HEAD
Updating Homebrew...
==> Auto-updated Homebrew!Updated 3 taps (homebrew/cask-versions, homebrew/core and homebrew/cask).

Error: tldr 1.4.0 is already installed
To install HEAD, first run:
  brew unlink tldr
Geoffs-MacBook-Pro:~ $ brew unlink tldr
Unlinking /usr/local/Cellar/tldr/1.4.0... 5 symlinks removed.
Geoffs-MacBook-Pro:~ $ brew install tldr --HEAD
==> Cloning https://github.com/tldr-pages/tldr-c-client.git
Cloning into '/Users/xxxxx/Library/Caches/Homebrew/tldr--git'...
==> Checking out branch master
Already on 'master'
Your branch is up to date with 'origin/master'.
==> make PREFIX=/usr/local/Cellar/tldr/HEAD-2faf09e install
==> Caveats
Bash completion has been installed to:
  /usr/local/etc/bash_completion.d
==> Summary
🍺  /usr/local/Cellar/tldr/HEAD-2faf09e: 10 files, 82.1KB, built in 11 seconds
Removing: /Users/xxxxx/Library/Caches/Homebrew/tldr--1.4.0... (14.7KB)
Geoffs-MacBook-Pro:~ $ tldr -u
Successfully updated local database
Geoffs-MacBook-Pro:~ $ 

I needed to unlink, but after that it worked, which is great! Thank you very much.

Not critical but tldr -v returns tldr v1.3.0 (v1.4.0-2-g2faf09e). Oh and I checked in the .tldrc directory and found both tldr-mainand tldr-masterwhich I believe is as expected.

If that was not what you wanted, or I did something wrong, please just ask.

marchersimon commented 2 years ago

Thanks, that was exactly what I wanted to know. We will create a new release with the fix soon, so you can get the new version without the --HEAD flag.

marchersimon commented 2 years ago

I just released the fix. Thanks for your help @glawrence