-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
[BUG] ComicInfo.xml is not found when not placed at archive root #222
Comments
The problem with this is that Manga Manager is following the Spec. A ComicInfo.xml should only be at the root of the document. Different reading softwares have different fallbacks. Like Kavita will fallback and search the whole directory. Komga I believe does not and only supports at root. So in order for this to work, it would have to find the nested ComicInfo then delete it and write it at root to be successful for reading software to pick it up. |
Thank you for taking time to respond to my issue. I understand your statement about the spec. I actually wondered why ComicInfo.xml shouldn't always be at the root myself. My confusion came from seeing that Manga Manager's LoadedFileMetadata class differentiates between ComicInfo.xml at the root vs in a subdirectory and specifically contains the following comment at line 64: "# If Comicinfo is not at root try to grab any ComicInfo.xml in the file". I figured it might be written this way to support chapter separation by folder within the archive so that each chapter could contain its own metadata without being removed from the volume and stored as separate chapters; possibly as an extension of the spec or preparation for a future spec implementation. To ensure I understand your second statement, you're saying that if Manga Manager did load the nested ComicInfo.xml file, saved a new version at the root, and then deleted the previous nested one from the archive, this would be in spec? If so, I have already created a pull request for code that does this. As far as I could tell Manga Manager already only saves to the root, so I replaced the code to find ComicInfo.xml at the root with code that finds the first ComicInfo.xml anywhere in the archive (as the line 64 comment suggests). Manga Manager already removes other nested ComicInfo.xml files from the archive during processing, so everything should work and fit spec after processing. When I tested this locally with a limited number of tests it always appears to work. I advocate for Manga Manager implementing this since all archives processed by Manga Manager will then be properly packaged and have a root ComicInfo.xml and no nested ones. Additionally, users of Kavita or similar programs that have fallback scans (who might have improperly packaged archives) can use Manga Manager to manage their libraries without worrying whether improperly stored metadata is being ignored and needing to re-enter all of it from scratch. In the end, Manga Manager will ensure their library is properly in spec. Alternatively, if this is not how Manga Manager should function, I propose modifying the LoadedFileMetadata class to remove the portion that attempts to distinguish between ComicInfo.xml at the root and nested ComicInfo.xml since anytime ComicInfo.xml is not found at the root, the code runs an unnecessary list comprehension over filepaths in the entire archive, which will always return false and then default to ComicInfo.xml at the root anyway. |
The idea to scan for any comic info in the file should exist and only grab series level metadata from the first file found. |
To be certain we're talking about the same code, I'm looking at the develop branch and the code is still there. I proposed this issue and the related pull request because I believe that there is a bug in the code and it only ever grabs the value of the list at index 0 which will always be "False" since the list comprehension over the filepaths in the archive are never filtered. This is the original code as is it appears in the LoadedFileMetadata class: # If Comicinfo is not at root try to grab any ComicInfo.xml in the file
if COMICINFO_FILE not in self.archive.namelist():
cinfo_file = [filename.endswith(COMICINFO_FILE) for filename in self.archive.namelist()][
0] or COMICINFO_FILE The list comprehension is making a list of boolean values whose value is based on whether the filepath ends with "ComicInfo.xml" and since the ComicInfo.xml file will never be the first file (since it's not at the root), the value at index 0 is "False". Therefore, Manga Manager defaults to the looking for metadata at the hardcoded location of ComicInfo.xml at the root even though it might have found it elsewhere in the archive. |
Describe the bug
In develop branch, Manga-Manager does not actually load ComicInfo.xml files that are not at the archive root.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
At a superficial level, it was expected that Manga-Manager should load the meta data from any ComicInfo.xml present within an archive even if not at the root of the archive.
At the code level, Manga-Manager's LoadedFileMetadata class logic appears to written so that ComicInfo.xml metadata should be loaded from anywhere within a file archive since it searches for a filename ending with "ComicInfo.xml."
Platforms
This error occurs in following two tested platforms:
Additional Platform Notes
The bug occurs on the windows exe binary and also occurs with cloned code from this github repository for both Windows and Linux.
Additional Context
Code has been added to a fork and a pull request will be created and linked to this issue for review.
The text was updated successfully, but these errors were encountered: