darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.44k stars 1.12k forks source link

Removing film-roll removes all images recursively #16949

Open har0ke opened 2 months ago

har0ke commented 2 months ago

Describe the bug

If film-rolls are placed recursively on the file system, deleting a parent film-roll, will remove all child film-rolls recursively. This is counter-Intuitive, as in darktable film-rolls never show images from child film-rolls because file system hierarchy is typically not visible.

A basic example for a problematic file system layout:

My worst case example:

I have all my images under /home/<user>/Pictures/Darktable. If I temporarily open (and therefore implicitly import) a file from /home/<user>/ a new film-roll <user> (1) is created, with a single image in that film-roll. Removing that film-roll from the library (right-click -> remove...) will now remove all images recursively, including all my primary film-rolls. This basically clears my whole database. The "remove images?" dialog that pops up actually tells you it is removing a huge amount of images, but some people (including me) might not read carefully enough.

Steps to reproduce

  1. Place two images under <base_path>/A/a.jpeg and <base_path>/A/B/b.jpeg
  2. Import both images into darktable
  3. Remove film-roll "A (1)"

Film-roll "B (1)" is now also removed from darktable.

Expected behavior

Not remove film-rolls recursively.

Logfile | Screenshot | Screencast

No response

Commit

No response

Where did you obtain darktable from?

distro packaging

darktable version

4.6.1

What OS are you using?

Linux

What is the version of your OS?

Arch Linux

Describe your system?

No response

Are you using OpenCL GPU in darktable?

None

If yes, what is the GPU card and driver?

No response

Please provide additional context if applicable. You can attach files too, but might need to rename to .txt or .zip

No response

pehar1 commented 2 months ago

Duplicate of "Film rolls are removed recursively" #16921

wpferguson commented 2 months ago

I'll ask the same questions I asked in #16921

When you try and remove a nested filmroll, how should it work?

Fail silently?
Refuse to give you the option to remove?
Fail with a message?
???

What should darktable do if you want to remove a nested filmroll?

It doesn't matter which solution you check, because it won't work for someone. The answer is there is no solution that can protect the user from themselves.

However, darktable does backup the database, so you can go to your ~/.config/darktable folder and rename the library.db and data.db files then choose a backup set and rename them to data.db and library.db then restart darktable and have your filmrolls back minus whatever differences happened between the backup and the problem.

Or, if you write sidecar files, you can just re-import each folder or recursive import the whole thing.

The backups and sidecars are how we try and protect the users from themselves.

har0ke commented 2 months ago

Sorry for the duplicate, when searching for duplicates I did have "collections" and not "film-rolls" in my head...

Or, if you write sidecar files, you can just re-import each folder or recursive import the whole thing.

For me, the re-import work-around is currently not sufficient because of #16827.

When you try and remove a nested filmroll, how should it work?

Fail silently?
Refuse to give you the option to remove?
Fail with a message?
???

I still think this is counter-intuitive behavior, again, because in darktable the file system hierarchy is not really visible.

A minimum would be to warn the user that if more than the right-clicked film-roll will be affected. Then the dialog changes in formatting and it really is visible that something extraordinary is happening. A number that does not match the expected number of files, is hard to catch.

Also, having two separate options "remove..." and "remove recursively..." (or alternative wording) could give the users more flexibility and tells the user directly that more than files in that film-roll might be affected if he hits "remove recursively...".

wpferguson commented 2 months ago

For me, the re-import work-around is currently not sufficient because of https://github.com/darktable-org/darktable/issues/16827.

Then use the database backup.

har0ke commented 2 months ago

Then use the database backup.

I already have.

This ticket was not intended as a support ticket but to suggest changing the unexpected behavior (in my opinion) or alternatively introducing another fail-safe or more explicit warnings to protect future users from making the same mistakes.

har0ke commented 2 months ago

But if you are giving support here: Is it possible to re-import duplicate edits? The ones saved as "_01.xmp".

ralfbrown commented 2 months ago

When you import, darktable looks for all existing sidecar files for the image. You should be getting the duplicate edits automatically.

wpferguson commented 2 months ago

Just imported a fresh film roll, duplicated the first image twirce. Stopped darktable and removed all the database files. Opened darktable, reimported, and the duplicates showed up fine

har0ke commented 2 months ago

Yes, I have not re-imported files that are in the last backup, but that I created duplicates for, after the backup. Thank you.

har0ke commented 2 months ago

If you, unlike me, think this is the behavior as expected by the general user, you can close this issue.

Just so my suggestions do not get lost in the support discussion:

A minimum would be to warn the user that if more than the right-clicked film-roll will be affected. Then the dialog changes in formatting and it really is visible that something extraordinary is happening. A number that does not match the expected number of files, is hard to catch.

Also, having two separate options "remove..." and "remove recursively..." (or alternative wording) could give the users more flexibility and tells the user directly that more than files in that film-roll might be affected if he hits "remove recursively...".

wpferguson commented 2 months ago

If you, unlike me, think this is the behavior as expected by the general user

I would only not like you if you were rude. If you were rude, I would just leave the conversation. I'm still here :smile:

I understand your issue and don't disagree with your solution.

However, there are some facts you should be aware of:

  1. The newest release, 4.8, is going to be built in the next week or so and released in a couple of weeks.
  2. The earliest you could see a solution to this would be the end of December, unless you download master and compile it.
  3. The person that is most familiar with this piece of code is currently busy with life and job and doesn't have much time to work on code.
  4. Anyone deciding to take this on can review the conversation and be able to evaluate how critical it is so they can prioritize the work.
  5. All the workarounds/recovery methods listed above will help anyone else that encounters this issue before any fix is released.
har0ke commented 2 months ago

I think that was a miscommunication. I meant: "If you, as opposed to me, think this is the behavior as expected by the general user, [....]". I was not intending to say that someone did not like me, nor trying to be rude myself. 😄

Also, I do not think this is issue is time-sensitive in nature. Thank you for your help.

github-actions[bot] commented 2 weeks ago

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.