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

Handle 0-byte (moved/deleted) albums differently on import dupe detection #2486

Closed
RollingStar opened this issue Mar 21, 2017 · 16 comments
Closed
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature."

Comments

@RollingStar
Copy link
Contributor

Problem

Same setup and similar problem as #2485. Does the user need to care about a phantom 0-byte nonexistent album in their library? That sounds like it is more useful as a message in a log file, or an error at the end of an import. See also #116.

Suggested behavior: beet imports the new album and defaults to "remove old" in the case of 0 byte albums that it cannot find the files for. Doesn't it end up pruning such removed files upon a "beet update" anyway?

@sampsyo sampsyo added the needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." label Mar 21, 2017
@sampsyo
Copy link
Member

sampsyo commented Mar 21, 2017

That's odd! Just to check, you've somehow ended up with a bunch of empty files in your collection? Maybe something like

find . -size 0 | xargs rm

would help clean those up?

@RollingStar
Copy link
Contributor Author

The way it happens on Windows is I import an album, then find a flaw with my config. So I change the config and move the imported album out of the library and send it to the importer again. There's probably a better way to do this. One would think beet update searchquery but I get stuff like this:

skipping \'Weird Al' Yankovic - 1985 - Dare to Be Stupid (CD) [192kbps MP3]\11. Hooked on Polkas.mp3 because mtime is up to date (1488817281.0)

I want to update the file folders and filenames based on how my config.yaml is set up, without going through the time-intensive process of beet re-calculating file paths for each file (most of which are unchanged). I can do that with beet move but it takes a while.

@sampsyo
Copy link
Member

sampsyo commented Mar 24, 2017

Take a look at the FAQ—the right solution is to run beet move. (beet update does something else.)

And for what it's worth, you can remove things from your library with beet rm, which also tells beets that you're removing them.

@stale
Copy link

stale bot commented Jul 12, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 12, 2020
@RollingStar
Copy link
Contributor Author

I can't guarantee I manage my library through beets 100% of the time. Beets should not get caught up in files it can't find. I don't know if this behavior still happens.

@stale stale bot removed the stale label Jul 12, 2020
@stale
Copy link

stale bot commented Sep 10, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 10, 2020
@RollingStar
Copy link
Contributor Author

yes

@stale stale bot removed the stale label Sep 10, 2020
@sampsyo
Copy link
Member

sampsyo commented Sep 10, 2020

Hey, @RollingStar—is it possible to summarize what the actionable next steps are? It would be useful to crystallize what actually needs to change here. Then we can turn this into a proper feature request.

@RollingStar
Copy link
Contributor Author

RollingStar commented Sep 10, 2020

The problem traces back to this:

I can't guarantee I manage my library through beets 100% of the time.

If we accept this as acceptable (but annoying) behavior for Beets users (especially novice users), then some bugs are bound to crop up. Going back to it now, the bug I reported is a subset of a general bug: Beets behaves oddly when it encounters files that have been deleted. Perhaps it should manage those errors more invisibly. Or should a filesystem error trigger a hard stop and suggest the user (1) look into the problem and (2) update when they are satisfied they addressed it? File errors could come up with USB, SD (alternatives/convert plugin?), or network drives, on top of normal user deletes and moves.

Steps to reproduce:

  1. Download the ogg of Directionless EP

  2. beet import "C:\Users\rollingstar\Downloads\Broke For Free - Directionless EP.zip" (Windows x64 cmder if it matters)

Sidenote: An example normal import which triggers the dupe detection.

Correcting tags from:
    Broke For Free - Directionless EP
To:
    Broke For Free - Directionless
URL:
    https://musicbrainz.org/release/e81e934b-1c7e-44b0-849c-de3c5311fb19
(Similarity: 95.9%) (country, media) (Digital Media, 2011, XW)
This album is already in the library!
Old: 6 items, OGG, 192kbps, 21:25, 24.8 MiB
New: 6 items, OGG, 192kbps, 21:25, 24.7 MiB
[S]kip new, Keep both, Remove old, Merge all?
  1. Move the Directionless folder out of the beets library folder.

  2. Run the import on the same file. (Again, this could be a new master that the user might actually want)

Correcting tags from:
    Broke For Free - Directionless EP
To:
    Broke For Free - Directionless
URL:
    https://musicbrainz.org/release/e81e934b-1c7e-44b0-849c-de3c5311fb19
(Similarity: 95.9%) (country, media) (Digital Media, 2011, XW)
This album is already in the library!
could not get filesize: [WinError 3] The system cannot find the path specified: '\\\\?\\e:\\Music\\Broke For Free - 2011 - Directionless\\01. Night Owl.ogg'
could not get filesize: [WinError 3] The system cannot find the path specified: '\\\\?\\e:\\Music\\Broke For Free - 2011 - Directionless\\02. My Always Mood.ogg'
could not get filesize: [WinError 3] The system cannot find the path specified: '\\\\?\\e:\\Music\\Broke For Free - 2011 - Directionless\\03. Day Bird.ogg'
could not get filesize: [WinError 3] The system cannot find the path specified: '\\\\?\\e:\\Music\\Broke For Free - 2011 - Directionless\\04. My Luck.ogg'
could not get filesize: [WinError 3] The system cannot find the path specified: "\\\\?\\e:\\Music\\Broke For Free - 2011 - Directionless\\05. Mell's Parade.ogg"
could not get filesize: [WinError 3] The system cannot find the path specified: '\\\\?\\e:\\Music\\Broke For Free - 2011 - Directionless\\06. Only Instrumental.ogg'
Old: 6 items, OGG, 192kbps, 21:25, 0.0 B
New: 6 items, OGG, 192kbps, 21:25, 24.7 MiB
[S]kip new, Keep both, Remove old, Merge all?

The old album shows up as 0 bytes, meaning it's been deleted.

I feel that Beets should/could detect that the old album doesn't actually exist.

  • If there's one "old" (deleted) album and one "new" album being imported, I want this to kick out of the dupe detection workflow and go into a normal import workflow. Should the empty album be auto removed at this point with update?
  • If there are multiple "old" albums, and only a subset are deleted (0 bytes), they should be listed differently in some way. Or deleted outright with something like the normal "beet update".

@sampsyo
Copy link
Member

sampsyo commented Sep 11, 2020

I see; thanks for clarifying!

I guess the thing that is still making me uncertain here is that I am unsure what we can do that would not create new, ancillary confusion.

  • Ignore the "old" but file-less album and proceed as normal? This could be confusing because now there are two duplicate albums in the library and the user never saw a prompt that this was going to happen.
  • Automatically delete the "old" album from the library because its files have been removed? This is potentially a surprising data-loss trap waiting to happen—we are not in the habit of automatically deleting user data without their consent.

We could potentially clarify the message to say that there are zero files associated with the album? But I'm not sure that would help much.

@RollingStar
Copy link
Contributor Author

After sleeping on it, I think the best course of action is this:

should a filesystem error trigger a hard stop and suggest the user (1) look into the problem and (2) update when they are satisfied they addressed it? File errors could come up with USB, SD (alternatives/convert plugin?), or network drives, on top of normal user deletes and moves.

It strikes a balance between hard-to-predict edge cases and informing/protecting the user of those edge cases. The reason I made this bug report in the first place was because I didn't know I was incorrectly using beets, and there are simple remedies to prevent it and fix the missing albums. The error that comes up could be reasonably clear in just a few lines, and link to a documentation page too. Something like:

One of the files in your beets library is missing. Please ensure that any external or networked drives are connected and working properly in your OS. If you deleted or moved files that were tracked by beets, this message is normal. Consult the following FAQ and run beet update when you are aware of the risks.

I would print that message and simultaneously run beet update -pretend in the background to get a figure for how many files are missing. When that query is done (seems pretty quick even on my 100GB+ library) it could print the number of albums / files affected.

@sampsyo
Copy link
Member

sampsyo commented Sep 11, 2020

Sounds like a good place to start! What if we just enhanced the message you get there and expanded the docs for now? (I still think automatically removing stuff in the background is problematic, but we can tell people to run the command themselves if they want.)

@RollingStar
Copy link
Contributor Author

I'm with you on not doing an auto-remove. It's too hard to do right.

@RollingStar
Copy link
Contributor Author

This issue got far away from my original post and a lot of the info is irrelevant. Should I make a new issue for a more informative error message and close this one?

@sampsyo
Copy link
Member

sampsyo commented Sep 13, 2020

Sure, sounds good!

@RollingStar
Copy link
Contributor Author

Moving discussion to #3750

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature."
Projects
None yet
Development

No branches or pull requests

2 participants