Garmelon / PFERD

Programm zum Flotten, Einfachen Runterladen von Dateien
Other
129 stars 24 forks source link

Suggestion: PFERD should only delete files it previously added #82

Closed brodmo closed 10 months ago

brodmo commented 10 months ago

If I set on_conflict=remote_first, PFERD also deletes files which I added manually (e.g. exercise solutions, supplementary material not on Ilias). I think this is neither intuitive nor practical. From what I can gather, this should be relatively easy to implement by checking the previous report and only deleting known files.

I-Al-Istannen commented 10 months ago

The reason is that PFERD tries to synchronize a folder with ILIAS. I do not work on any assignments in that folder, and I want the folder to mirror ILIAS. If I explicitly tell PFERD that I want my local state to match ILIAS (on_conflict=remote_first), I expect PFERD to ensure that exactly this happens. If I want my local state to win, I can use on_conflict=local_first. If I want a mix of both, you need to use either no_delete (i.e. update files iff changed on the remote, but do not delete any) or prompt and decide manually.

What exactly is your problem with no_delete or prompt? You seem to add files locally, so I am not sure why you would want to blindly follow the remote state.

Maybe your use case is covered by #65? I do not think the change suggested in this issue makes sense for PFERD currently. It seems rather surprising.

brodmo commented 10 months ago

I find my use case quite intuitive and I’m surprised you consider it unusual. I’m not sure why I would want the solution to an exercise in a different folder than the description. Or the text which the lecture is about in a different folder than the lecture slides. To me it really doesn’t make sense to separate the materials to a course based on whether or not they are on Ilias. Moreover, changing PFERD this way would not make a difference to those who don’t create local files, while making it usable for those who do. I don’t see why anyone who manually created a local file would want PFERD to delete it. I really don’t see the downside. As for your suggestions, I looked at the options before creating this issue, and they don’t work for me. no_delete causes a lot of clutter because the names of the materials on Ilias are often renamed (at least in the courses I’m in). Using it would mean I have lots of duplicates which I have to manually clean up. With prompt I also need to do a lot of manual and tedious work every time I use PFERD. So both are, to me, unviable. I also don't think that this would break the concept of synchronization. Sure, I want my local state to mirror the remote state, but if I create files locally and they stay around, I don't think that breaks the synchronization.

Garmelon commented 10 months ago

If a file exists locally but not remotely, PFERD can't distinguish between

  1. the file was added by the user
  2. the file previously existed on ILIAS but was renamed or deleted In both cases, a file exists locally that has no corresponding remote file.

In this scenario,

Distinguishing manually created local files from PFERD-created local files correctly is not easy to do robustly, and makes the mental model required to use PFERD more complex as well (e.g. "Why is PFERD not updating this file? It comes from ILIAS. Ooh, I opened it in LibreOffice and pressed ctrl+s accidentally").

Another way to view PFERD is as a read-only mounted ILIAS. I wouldn't expect to be able to place files in a remote file system without write access to said file system.

I recommend letting one directory be managed by PFERD, and having a separate directory for any files you create or edit manually.

brodmo commented 10 months ago

Could you not look at the known files from the previous run to solve this elegantly? Only delete a file if it is known, otherwise, leave it alone. It's unknown, so to say, PFERD wouldn't even consider it. I think this makes a ton of sense. I'm not sure why seeing PFERD as a utility that pulls everything from Ilias but doesn't interfere with my files is less natural than viewing it as read-only mount. It's obvious that the syncing only happens one way, so no one would expect creating a file to have any impact on the remote. Thus PFERD may as well leave it be. I see that I could have a separate folder structure and manage my own files there, but to me this seems way less practical, always having to navigate in two separate windows. Maybe a separate flag could be a compromise? Something like ignore_unknown_files. But then again, I don't see any practical reason why someone would want it to be false.

Garmelon commented 10 months ago

You seem to think about PFERD as syncing files, while I think about it syncing directories.

Of course you could keep the files from the previous run, but this is not trivial and a lot can go wrong.

For a start, you'd need to keep not just the remote file path and the local file path it was transformed into, you also need to keep a hash of the file contents to ensure that a file that was deleted on ILIAS but modified locally is not just deleted by PFERD.

Then, there's the entire topic of aborting a run. You will need some way to merge the partial results of the aborted run with the files from the previous run, or things will go wrong on the next run. This will require infrastructure in PFERD for tracking file info in a threadsafe way as well as infrastructure for saving state information when a run is aborted.

This new state is also turning conflict resolution in a three-way problem (current ILIAS state, current local state, previous ILIAS+local state) instead of the two-way problem it is currently (current ILIAS state, current local state), not to mention all the additional possible race conditions something like this might introduce.

I don't think this complexity and all the additional possibilities of introducing bugs is worth it when you can just as well store non-ILIAS files in a separate directory. If you must keep both ILIAS and your own files in the same directory, use no_delete and occasionally clean up the files that were deleted/renamed on ILIAS manually or using prompt.

brodmo commented 10 months ago

I would not consider file contents, which seems to me like it would solve a lot of these headaches. If you modify a file from Ilias, it may still be deleted later, I think this matches expectations. Would you still consider it to be a major hassle in this case? Obviously I don't know all that much about the inner workings of PFERD and what edge cases need to be considered. To me it seems as though keeping track of known files and only touching those would be relatively trivial (as known files are already kept track of).

Garmelon commented 10 months ago

All the other parts still apply even without file hashes.

brodmo commented 10 months ago

I don't really understand why aborting a run would be a problem if PFERD never touches unknown files. These files would be outside PFERD's universe, so they don't present a problem and don't require any other changes in logic, also with conflict resolution -- where am I going wrong with this logic? I suppose a problem I can identify is that the previous report may be bogus due to an aborted run, do you think that's the core issue here? To me it just seems unsatisfying to have to manage my files in a way I would consider much less convenient, when PFERD is all about ease of use.

Garmelon commented 10 months ago

During a run, PFERD updates and creates files which need to be added to the state. If that does not happen correctly, on next runs, PFERD would view those files as user-created, and different conflict resolution rules would apply. If PFERD never touches unknown files, this would mean that you would always have to clean up any new files downloaded during a partial run, or - if only the files from the partial run were stored in the state - all files not downloaded during a partial run.