tldr-pages / tldr-c-client

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

Fix: Fallback manual copy for update_localdb() #21

Closed iguessthislldo closed 6 years ago

iguessthislldo commented 7 years ago

I originally started writing this as an issue I was going to open, but decided to look into it and found that I could fix it.

When using tldr --update, it exits with error code 1 and prints something like Error: Could Not Rename: /tmp/tldrqquWXv/tldr-master to /home/fred/.tldrc/tldr-master/. I compiled from latest commit on Arch Linux. It was shows the message when querying a command for the first time, but it also shows the tldr page, and no error code. After .tldrc has been created it only shows it for tldr --update.

This happens because rename() has an EXDEV (cross device) error. It can't copy the updated pages from /tmp to home because Arch mounts /tmp using tmpfs by default. I changed it so that only when rename() has a EXDEV error, it falls back to a recursive copy instead.

iguessthislldo commented 7 years ago

I think I addressed all your comments, also I hope you didn't mind that I copy and pasted the fts loop from the rm function. It was tempting and I'm not super familiar with unix apis, but I hope to be.

LukasRypl commented 7 years ago

Would be nice if this PR is accepted. Is there anything more what needs to be done?

Maybe a better solution would be to create a temporary directory inside ~/.tldrc/ instead of /tmp so that there is no need to handle renaming between different file systems.

iguessthislldo commented 7 years ago

Yeah, that seems like a more elegant solution than my attempt to recreate cp -r. Maybe I should have brought this up as an issue first after all, but I was a little excited to make my first contribution to a open source project.

I went ahead and changed it to how you described.

iguessthislldo commented 7 years ago

I just realized that @LukasRypl isn't @Leandros... I'm sorry I keep jumping the gun on what I'm doing here. @Leandros when you get back to this, I'm happy to go with whatever, as long as the problem is fixed, because I like program.

Leandros commented 7 years ago

Don't worry. I'll take a look as soon as I can.

12345ieee commented 7 years ago

I hit this error, as well.

While I wait for the fix to be released, what is the workaround?

12345ieee commented 7 years ago

For anybody landing here, this fixes the problem:

mkdir ~/.tldrc
cd ~/.tldrc
wget https://github.com/tldr-pages/tldr/archive/master.zip
unzip master.zip
rm master.zip

Script based on a cursory look at the code.

gingerbeardman commented 3 years ago

Above gets me working but does not fix the core issue. Eventually the app thinks the data is too old and I need to do the manual fix again.

devhell commented 3 years ago

@Leandros can this be merged please?

For anyone else looking for a good alternative, I've found https://github.com/dbrgn/tealdeer to be pretty sweet.

bl-ue commented 3 years ago

I haven't seen this PR. I wonder if we can merge it. 🤔

bl-ue commented 3 years ago

Hey @iguessthislldo! I realize that it's been a long time, but would you be interested in working with me to merge this PR? I'm a tldr-pages maintainer so I can do that ;)

iguessthislldo commented 3 years ago

@bl-ue I moved on to a different tldr client a long time ago, but it would be nice if my first ever PR was merged. Looks like I deleted my fork at some point though, so I can't just simply reopen it.... I will see if I can get it set back up if I have some time.

lwpie commented 2 years ago

Any updates?