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

FR: Centralized Sidecar directories #9265

Closed
uroybd opened this issue Jun 28, 2022 · 54 comments · Fixed by #10074
Closed

FR: Centralized Sidecar directories #9265

uroybd opened this issue Jun 28, 2022 · 54 comments · Fixed by #10074
Milestone

Comments

@uroybd
Copy link
Contributor

uroybd commented Jun 28, 2022

Currently, KOReader creates a sidecar folder whenever we open a book in the same directory the book is in. The best thing about the sidecar-based approach is its easy discoverability and portability. On the other hand, It can create a lot of directories and kinda look bad in file explorers.

I'm proposing it to centralize in a way which I believe will be a win-win.

Firstly, there will be a sidecar directory in KOReader's config directory, similar to the clipboard directory. Let's call it $SIDECAR_DIR for now.
Instead of creating a sidecar in the same directory of the book, it will be created at: $SIDECAR_DIR/absolute/path/of/the/file.sdr/

In this way, sidecars will be easily discoverable, and the directories outside will be clean too.

@mergen3107
Copy link
Contributor

mergen3107 commented Jun 28, 2022

Try this:

In the koreader directory create a directory called sdr.
In the koreader\frontend\docsettings.lua file replace the function DocSettings:getSidecarDir to this one:

local DATA_SDR_DIR = DataStorage:getDataDir() .. "/sdr/"
function DocSettings:getSidecarDir(doc_path)
if doc_path == nil or doc_path == '' then return '' end
local file_name = doc_path:match(".*%/(.*)%.")
if file_name then
return DATA_SDR_DIR .. file_name .. ".sdr"
end
file_name = doc_path:match(".*%/(.*)")
if file_name then
return DATA_SDR_DIR .. file_name .. ".sdr"
end
return doc_path .. ".sdr"
end

@mergen3107
Copy link
Contributor

However you will need to do this after every update

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

I'm not planning to do this as a one-time thing. My intention is to implement this in the KOReader.

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

I'm proposing it to centralize in a way which I believe will be a win-win.

Definitely not a win-win. Imagine trying to sync between two devices.

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

I'm proposing it to centralize in a way which I believe will be a win-win.

Definitely not a win-win. Imagine trying to sync between two devices.

Syncing via plugin? That only syncs progress now, right?

Or, syncing via third-party methods?

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

By syncing I mean any action that copies the file and its metadata, no specific method. You can do it by hand. (But no, not the progress sync.)

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

It also just means organizing your own files using your computer.

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

I see. Can't we make a setting to handle this? People can choose where they want there sidecars to be.

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

That setting already exists, doesn't it?

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

Or something like #8577

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

That setting already exists, doesn't it?

Okay. Couldn't find it though.

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

See #4831 (comment)

@poire-z
Copy link
Contributor

poire-z commented Jun 28, 2022

Even more links in #8281 (comment)

There is no setting yet. Everybody gets .sdr/ folders alongside the books.
Only on read-only filesystems the metadata.lua is saved in koreader/history/somehashortruncatedbutsinglefile.lua - so people may already have these (that you would need to migrate/consider when looking for past docsetting).

It should be a setting, and I think it should default to what we have currently.
(A setting for how to save - but when reading, you should look in all the possible old & new places.)

Having subdirectories making the full path of the file is obviously better than the somehashortruncatedbutsinglefile idea, which is not reversible (you can't guess the fullpath from this somehashortruncatedbutsinglefile) - and you would still have all the .old backup and ffiutil.fsyncOpenedFile() stuff working as-is.
Just need to be sure that everything working with sidecar dir/file works (history to show opened books, moving/copying books from one folder to another, and some other ones I assume :)

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

That's a patch, not a setting. And a very bad patch I guess. I'll loose data, right?

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

That's a patch, not a setting.

Well, I didn't remember the exact details about this thing I don't care for one iota. ;-)

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

Even more links in #8281 (comment)

There is no setting yet. Everybody gets .sdr/ folders alongside the books.
Only on read-only filesystems the metadata.lua is saved in koreader/history/somehashortruncatedbutsinglefile.lua - so people may already have these (that you would need to migrate/consider when looking for past docsetting).

It should be a setting, and I think it should default to what we have currently.
(A setting for how to save - but when reading, you should look in all the possible old & new places.)

Having subdirectories making the full path of the file is obviously better than the somehashortruncatedbutsinglefile idea, which is not reversible (you can't guess the fullpath from this somehashortruncatedbutsinglefile) - and you would still have all the .old backup and ffiutil.fsyncOpenedFile() stuff working as-is.
Just need to be sure that everything working with sidecar dir/file works (history to show opened books, moving/copying books from one folder to another, and some other ones I assume :)

Or, instead of centralizing, which means much work, we can hide the folders (at least in unix-like systems) by prepending a dot like another FR suggested.

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

What's the current situation on Kindle @NiLuJe?

@uroybd
Copy link
Contributor Author

uroybd commented Jun 28, 2022

@poire-z

Having subdirectories making the full path of the file is obviously better than the somehashortruncatedbutsinglefile idea, which is not reversible (you can't guess the fullpath from this somehashortruncatedbutsinglefile) - and you would still have all the .old backup and ffiutil.fsyncOpenedFile() stuff working as-is.

Here's a confusion, and I'm responsible for that. In the original post I wanted to move the directory, not making it a .sdr file instead. I should've added a slash at the end. For example, for a book in /sdcard/emulated/0/books/example.epub

Currently the sidecar is: /sdcard/emulated/0/books/example.sdr/

I wanted it to be: $SIDECAR_DIR/sdcard/emulated/0/books/example.sdr/

@poire-z
Copy link
Contributor

poire-z commented Jun 28, 2022

^ That's how I understood it - so my bad if you thought I wasn't on track :)

@zwim
Copy link
Contributor

zwim commented Jun 28, 2022

However you will need to do this after every update.

Not necessarily. You can use #9104 to apply user patches.. Put you code into /koreader/patches/n-xxx.lua.

n=0 means to execute very early on startup (once after an update).

n=1 means to execute very early on every startup.

n=2 means a bit later on startup

xxx may be choosen as you like.

I think you can use /kodreader/patches/2-sdr-location-patch.lua to modify DocSettings:getSidecarDir.

You can find an example for patching in https://gist.github.com/zwim/1edcd34ef8a59166f203d5ee8c08f7e3

@NiLuJe
Copy link
Member

NiLuJe commented Jun 28, 2022

What's the current situation on Kindle @NiLuJe?

Same as everywhere (that isn't Android ;p) else (in fact, the whole sidecar folder thing was designed after how the stock reader does this) ;).

@NiLuJe
Copy link
Member

NiLuJe commented Jun 28, 2022

FWIW, I am completely unbothered by the current situation, but at least what @uroybd proposes is much less fugly than the current ro fallback ;).

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

in fact, the whole sidecar folder thing was designed after how the stock reader does this

My bad, I didn't express myself clearly. I meant, does the stock reader still do that? :-)

@poire-z
Copy link
Contributor

poire-z commented Jun 28, 2022

does the stock reader still do that?

@hius07 did answer a similar question of mine at #8281 (comment)

@NiLuJe
Copy link
Member

NiLuJe commented Jun 28, 2022

TL;DR: Yes, and there's even more stuff in there than it used to ;p.

@uroybd
Copy link
Contributor Author

uroybd commented Jun 30, 2022

Now... Should we centralize sidecars or make them hidden with dots? Of course configurably.

@poire-z
Copy link
Contributor

poire-z commented Jun 30, 2022

The fact is that KOReader will have to find any sidecar dir/file in any past, present or future possible locations.
(You can't do a one-time migration because of removable SD cards, and you may not know about all the places where some books can be - and I think this is just fine.)
The current code already does that (from the sidecar dir as we know them, to the koreader/history/ dir when the library filesystem is read-only), and I think this idea should be kept:

  • scan all posible locations (past or present), sort the found metadata.lua by date
  • read and use the most recent metadata.lua found
  • on autoflush or when leaving the book, save to the "prefered location" (currently, the .sdr folder as we know it) or, if not possible because read-only, to the "next prefered location" (koreader/history/) - and (optionally disableable) delete the other locations

So, if you want to keep this nice idea which wouldn't need any migration, you could just add new locations:

  • A current sdr without dot alongside the book (that you have to keep as you have to read them)
  • B new sdr with dot alongside the book (could be or not the new default)
  • C new sdr without dot in koreader/sidecars/ or $SIDECAR_DIR
  • D current-old a file in koreader/history/

And if you can do that all abstracted in docsettings.lua (+ may be some adjustments in filemanager.lua for the move/copy/delete a book), it wouldn't (shouldn't) need any changes anywhere else, except for using possible added docsettings.lua API.

And may be, if we add support for C, we can get rid of proposing/creating D (although we should still scan/read D) as D is the most ugly system as we can't go back from a setting to the book (and it doesn't handle .old and using .old when the current is corrupted).
And a user could choose between A B and C for its future new & updated sidecars.

(Proposing this choice to the user is not really a feature we would need, but the system as it is currently and as it can still be, could make it easy - as we need the various locations for technical reasons.)

@pazos
Copy link
Member

pazos commented Jun 30, 2022

I have no strong opinion on this topic, or in most topics :p.

But the B proposal (sdr with dots) looks to me as the worst of both worlds since we still have the filesystem cluttered with one folder per file, is not sorted near the file in any FM and it doesn't play nice with some sync daemons.

So 👍 to include it as an option if it is useful for somebody. -1000 to make it default. I guess I would be ok even if it is the new default if there's a way to switch back to current behaviour.

And a user could choose between A B and C for its future new & updated sidecars.

That would be awesome. Even A and C alone would be enough for most users, I think

@Frenzie
Copy link
Member

Frenzie commented Jun 30, 2022

looks to me as the worst of both worlds

That's also my opinion for similar reasons in case I hadn't said so yet. It's basically the clutter without the positive aspects.

I think the better form of that would probably look like a .sdr folder that simply has the relevant filename.lua metadata inside for all the files. To be clear, I'm not suggesting we do that. ;-)

@Ketrel
Copy link

Ketrel commented Nov 9, 2022

I had said this in another issue and was told this was the active discussion on it.

I have the same problem/annoyance with this.

It's be far better if it used the koreader folder as a base, so if I have a book at say /storage/emulated/0/Books/a/b/c/d.epub the metadata folder would be /storage/emulated/0/koreader/metadata/a/b/c/d.sdr/

That seems like it'd be the best option to avoid filesystem clutter while keeping code changes to the minimum.

@liskin
Copy link
Contributor

liskin commented Jan 14, 2023

If anyone needs this, I've implemented a solution similar to the one posted above as a patch, thus it should survive an update.

I have this in /applications/koreader/patches/1-sidecar.lua:

-- move sidecar directories, they mess up the DropBox sync

local DocSettings = require("docsettings")
local DataStorage = require("datastorage")
local lfs = require("libs/libkoreader-lfs")

local function getSidecarDir()
    local d = DataStorage:getDataDir() .. "/sidecars"
    if lfs.attributes(d, "mode") ~= "directory" then
        lfs.mkdir(d)
    end
    return d
end

local SIDECAR_DIR = getSidecarDir()

function DocSettings:getSidecarDir(doc_path)
    if doc_path == nil or doc_path == '' then
        return ''
    end

    local file_without_suffix = doc_path:match("(.*)%.")
    if not file_without_suffix then
        file_without_suffix = doc_path
    end

    return SIDECAR_DIR .. "/[" .. file_without_suffix:gsub("(.*/)([^/]+)", "%1] %2"):gsub("/", "#") .. ".sdr"
end

(The gsub is taken from DocSettings:getHistoryPath nearly verbatim. I tried using a simpler patch that only did DocSettings.ensureSidecar = function() end, forcing a fallback to getHistoryPath, for a while, but not seeing reading status in File Manager and History was annoying.)

The reason this is needed is there is a weird bug in PocketBook's DropBox client. Every single time it syncs, it duplicates the metadata.epub.lua and metadata.epub.lua.old files, so after a couple days of reading, my DropBox is full of metadata.epub.lua (1) and metadata.epub.lua (2) and so on. That's on a PocketBook InkPad 3 with the latest firmware. So on this device, the sidecars cannot be alongside the books…

@hius07
Copy link
Member

hius07 commented Jan 25, 2023

Please note the centralized sidecar folder is not supported in copy/move filebrowser operations.

@hius07
Copy link
Member

hius07 commented Jan 28, 2023

D is the most ugly system as we can't go back from a setting to the book (and it doesn't handle .old and using .old when the current is corrupted)

@poire-z, can you please elaborate on this. I'm afraid I got it wrong.
I see that we can go back from a setting to the book (and are doing this in readhistory), and .old backups are handled well.

@poire-z
Copy link
Contributor

poire-z commented Jan 28, 2023

Well, I don't really remember what I had in mind :/
Just tried opening a book from a readonly directory, and I got:
koreader/history/[#koreader_stuff#test#MyReadableDirectory#MyNonWritableDirectory2#] file2.txt.lua
so it seems like we can go back from this to where the file is: /koreader_stuff/test/MyReadableDirectory/MyNonWritableDirectory/file2.txt

May be when I wrote this, I had wrongly in mind that it was saved as koreader/history/ab45d6817.lua, like with a hash of the full path ? (and then we could indeed not get back the fullpath from that hash). May be I was confusing it with how it's done in koreader/cache/cr3cache/.

So, well, that's all I can think of today :/ Either I was confused that day, or I was more clever than I am today :)

(I'm also not sure why it would be important to go back from the setting to the book, except when "rebuilding the history", which feels non-essential.)

@hius07
Copy link
Member

hius07 commented Jan 28, 2023

Good, thanks.
So I believe that having a choice in UX between A and D would be enough.

@poire-z
Copy link
Contributor

poire-z commented Jan 28, 2023

Yes, probably.
The above patches/1-sidecar.lua is doing it more like D than C, right ?

I'm not sure, but I guess that with C, I was thinking about replicating the whole directory tree under sidecar/, ie:
koreader/sidecar/koreader_stuff/test/MyReadableDirectory/MyNonWritableDirectory/file2.sdr/metadata.txt.lua
which would make the existing code (moving/renaming directories) reusable, instead of having to just rename koreader/history/[#koreader_stuff#test#MyReadableDirectory#MyNonWritableDirectory2#] file2.txt.lua.

So, dunno which of C or D is easier. But given you would still have to handle D (or migrate them to C), if C does not bring anything new to D, we could go with just D.

Also beware with D: on FAT32, the max filename length is 255, so users with deeply organized book trees could meet this limit.

(In the past, we had also thought about storing other things in the sidecar directory, like book cover image - even if we never ended up doing anything like that - C would allow it, while D would not.)

@hius07
Copy link
Member

hius07 commented Jan 30, 2023

Another bad thing with D (doc-settings files in koreader/history) is that removing a book from History kills its history-file (that contains doc-settings).

@poire-z
Copy link
Contributor

poire-z commented Feb 3, 2023

So, it looks like with #10074, you went with C - and not with D that seemed to have your preference above.
Can you elaborate why this change of mind :) (I'm fine with it :)
I didn't really understand your above comment "Another bad thing with D..." - and dunno if it is the only reason for forgetting about D.

@hius07
Copy link
Member

hius07 commented Feb 3, 2023

The reasons of chioice are yours: limit of 256 chars in the full name and future impossibility of having more files related to a book in its sdr folder.
Plus my one (line 268):

function ReadHistory:removeItem(item, idx, no_flush)
local index = idx or self:getIndexByTime(item.time, item.file:gsub(".*/", ""))
table.remove(self.hist, index)
os.remove(DocSettings:getHistoryPath(item.file))
if not no_flush then
self:_flush()
end
end

While removing an item from history, its (legacy) history file is deleted. We cannot store anything in it.
If we keep it, the book will reappear in History.

@mergen3107
Copy link
Contributor

On the other hand, It can create a lot of directories and kinda look bad in file explorers.

Temporary quasi-solution I am using is color filters (in WinSCP, Total Commander on PC):
I created a filter to make *.sdr/ folders to be shown in blue color when looking through files, and hidden when bulk copying books without settings.

@poire-z poire-z added this to the 2023.02 milestone Feb 17, 2023
@ryanwwest
Copy link
Contributor

ryanwwest commented Apr 30, 2023

I see a lot of mentions of $SIDECAR_DIR as the alternative centralized root directory instead of storing .sdr folders alongside the original files. But on the 2023.04 update, it looks like the new book metadata location setting only allows $SIDECAR_DIR to be koreader/docsettings/. Am I missing an option to be able to set this path string myself? As I still want to avoid cluttered .sdr folders everywhere but would like to put the folder in a place that I can easily sync between KOreader devices (using Syncthing).

I can make a new FR issue for this, assuming I'm not just missing something. I can't see many downsides to this - perhaps it would be relatively easy to add, given the work is already there for this?

@NiLuJe
Copy link
Member

NiLuJe commented Apr 30, 2023

We have no existing settings to relocate the similar-in-design settings and cache folders (and others), so I'm not sure I understand the interest/use-case?

@ryanwwest
Copy link
Contributor

@NiLuJe Thanks, I didn't realize that $SIDECAR_DIR was also used for other things besides storing the .sdr files. I was thinking it was a new directory for storing just the .sdr files and thus could reside anywhere. My use case is being able to place a '.sdr folder only' version of this folder in a place where I can easily sync it using Syncthing so that I can sync reading progress, annotations, and reading statistics (if they're stored in there) between all devices using KOReader. This already seems to work pretty well keeping the .sdr files in their original places (next to their corresponding document files) and I was thinking it wouldn't be too hard to keep them in a local folder instead and just point to that folder in the settings. But if you use that folder for more things too, it's a different story.

On the other hand, it would be nice to even sync app settings and other things in this same way (for example, obsidian.md puts ALL app settings in a .obsidian/ folder that resides int the same folder as the rest of your 'vault' or collection of Markdown files which makes syncing the whole vault, settings, plugins etc. trivial. But I know that's a large charge from the current setup so not expecting something similar.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 30, 2023

I didn't realize that $SIDECAR_DIR was also used for other things besides storing the .sdr files.

It's not. That wasn't my point. My point was other "user data" folders exist, and they're not relocatable either.

My actual question being: what's stopping you from pointing your sync stuff to the existing (or not, since it's opt-in) folder?

@NiLuJe
Copy link
Member

NiLuJe commented Apr 30, 2023

Settings are another kettle of fish, as they're not guaranteed to be cross-device compatible (e.g., when something is platform-specific, it's checked in the UI, not in the place the setting is read [mostly]).

So, while you can simply two-way sync 'em (again, it's a fixed location, but a stable one), it's probably a terrible idea, especially if you cross a platform-gap (e.g., Android to anything !Android; Android <-> Android would be mostly fine, probably. But still not guaranteed to not blow up in fun and interesting ways).

@ryanwwest
Copy link
Contributor

ryanwwest commented Apr 30, 2023

Re settings sync, I agree, you may want settings to be different for different devices with different properties and use cases. This is somewhat of a negative for Obsidian. VS Code has a neat solution of this with some defaults that can be overridden for specific devices and profiles, but that's far too complicated for something like this or most apps.

My actual question being: what's stopping you from pointing your sync stuff to the existing (or not, since it's opt-in) folder?

That is an option - it's just that this default absolute path changes from OS type (on Linux it's a different root path). I have a consistent folder setup for Syncthing across devices and OSes and would prefer to keep everything under them. I could always try symlinking or rsync, but that sometimes causes random issues. You have a good point, but it also makes me wonder why if the Screenshots folder path can currently be specified, why it wouldn't make sense to allow (and reuse) the same customizability here (maybe I'm missing a key reason). I think there are a lot of unforeseen reasons why people may want path customizability which is why so many apps allow it.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 30, 2023

It would feel like gratuitous complexity, and a very fine way of wondering why shit suddenly doesn't work as expected ;).

Also, on Android, we have to deal with the sandboxing fs access shenanigans, so the less hoop jumping we have to go through, the better.

TL;DR: Unlikely to change, ever.

@ryanwwest
Copy link
Contributor

Unrelated but could I ask how 'stable' the .sdr folder contents are across different versions of KOReader (I presume they are the same across OSes)? I'm thinking about ways to use them as a basic format for storing highlights that can also be written and read in other applications. For example, Zotero offers a built-in PDF reader and stores its own note/highlight annotations in its own database outside of the PDF file - I'm thinking about building a sync translation layer that would look at those annotations and write an equivalent version into the .sdr file Lua data structure so the highlights show up in KOReader. And vice versa when making annotation changes in KOReader so the Zotero PDF highlights database is updated.

Yeah, there's a ton of issues with sync conflicts that I'd have to figure out - I'm just wondering if I should invest time at all if the .sdr structure changes so frequently that it would break every other update. Thanks.

@Frenzie
Copy link
Member

Frenzie commented May 1, 2023

Here's a link to KOHighlights if you haven't come across it yet: https://www.mobileread.com/forums/showthread.php?t=265433

Stable depends a bit on what you mean by the word. There are some changes occasionally but on average that's (almost) as annoying for us as it'd be for another app.

@NiLuJe
Copy link
Member

NiLuJe commented May 1, 2023

The main pain point to deal with it externally is probably going to be that it's actually Lua code. You'll notice KoHighlights uses a Python module that basically massages Lua tables into Python dictionaries ;). I haven't really looked all that much, but that may very well be the only example of that sort of cross-language compatbility.

@ryanwwest
Copy link
Contributor

Maybe KoHighlights has already done a lot of the translation then and it could be a good thing to contribute to or build off of. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.