WebDav: Simplify path manipulations to fix wonky heuristics#12038
Conversation
|
All folders have empty |
|
It's being lost here: |
|
Narrowed down to |
|
(Don't mind underscores in the LOGG var names - I did it for pure alignment :D) |
|
|
|
It appears to behave differently than the coreutils |
|
|
|
dirname and basename are two different things |
|
A pure C (POSIX) version behaves correctly, so at least it's not a weird POSIX vs. GNU implementation quirk (as we should be pulling the POSIX one, too). |
|
So, this |
|
No, you just quoted the wrong bit of FFIUtil ;). |
|
Yep, my bad :D Was looking for |
|
Fun fact, KOReader's Terminal Emulator plugin can do |
|
If you mean on a device that's probably from busybox; otherwise probably GNU coreutils or equivalent. |
|
Yes, this was on emulator. |
|
It looks all OK until the last operation (this is for Last operation being |
Except we don't, not since koreader/koreader-base#804, because Android. And... Raaaah. |
|
Time for |
diff --git a/ffi/util.lua b/ffi/util.lua
index 093fbed7..d4d0a984 100644
--- a/ffi/util.lua
+++ b/ffi/util.lua
@@ -193,10 +193,15 @@ if ffi.os == "Windows" then
end
else
function util.basename(in_path)
- -- We have no guarantee of getting a GNU implementation of basename;
- -- POSIX compliant implementations *will* modify input, so, always make a copy.
- local path = ffi.new("char[?]", #in_path + 1, in_path)
- return ffi.string(C.basename(path))
+ -- NOTE: While we attempt to prefer the GNU implementation, because it's the only one Android provides,
+ -- we have no guarantee of getting it (e.g., on macOS, or non-glibc Linux systems);
+ -- POSIX compliant implementations *will* modify input, so, always make a copy.
+ -- NOTE: Moreover, the GNU implementation has a few potentially annoying quirks:
+ -- it returns an empty string when path has a trailing slash (as well as for "/").
+ -- So, we'll want to strip trailing slashes, too...
+ local stripped_path = in_path:match(".*[^/]") or "/" -- Ensure basename("/") leads to "/" as per SUSv2
+ local c_path = ffi.new("char[?]", #stripped_path + 1, stripped_path)
+ return ffi.string(C.basename(c_path))
end
end |
|
@NiLuJe |
|
@NiLuJe |
|
Review and testing quite welcome, as I never tested this myself, and I wrote this last "night" while heavily sleep-deprived because insomnia, so, err, here be dragons ;p. |
|
I hope you get better! :D Anything else? |
|
I'm not too worried about the basename stuff (there are only a few call sites); I was mostly thinking of the actual WebDav stuff ;). |
|
ALright, in that case Yandex.Disk webdav works. Anyone else using other WebDav clouds? |
|
@NiLuJe |
|
With a git client? ;o). No idea how that's doable without a CLI. |
|
Yes, I am using |
|
Essentially, yeah. e.g., |
|
Thanks! |
NiLuJe
left a comment
There was a problem hiding this comment.
Let's break all the things \o/
Reviewed 2 of 3 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @mergen3107)
mergen3107
left a comment
There was a problem hiding this comment.
Reviewable status:
|
(why does it still say it is waiting for me?) |
It doesn't; you're probably looking at my own review report, which is frozen in time ;). |
|
Someone please merge :D |
…#12038) This prevents the current directory from appearing in the listing.
…#12038) This prevents the current directory from appearing in the listing.
Supersedes #12034
Based on this diff: #12034 (comment)
Closes #12025
@NiLuJe
This diff on top of -191 master caused all folders to disappear from WebDav. I can only see files only.
This change is