-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add dissect.disc #1
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial few comments, let's first discuss these before I go more in depth and you have to go grab the.. well you know. miss you buddy
Two overall comments to think about:
dissect.disc
makes sense to support different disc formats in the long run. Currently you support ISO, but call it DISC. Maybe just put all the ISO stuff indissect.disc.iso
to make the separation clear.- Maybe think of a way to separate the extensions into different files. Circular dependency will be an issue though, so you may have to introduce a base file for the base directory record implementation.
Can you also include these two changes? |
I've addressed your initial few comments in the first three commits, and the broader remarks in the last one. Locally, I've started working on UDF, also to further understand what might be a nice structuring for this PR that keeps future extension in mind. I ended up implementing some 'base' logic in I do have some reservations about the new module structure, but am curious to hear what you think. Perhaps it's acceptable enough that we go more in depth, no holding back. like we always used to do |
- Throw the project-specific exceptions rather than the base python ones - Handle non-ISO9660 input files correctly - Move file finding logic to base.py - Bugfix current/parent directory for Joliet - Bugfix filesize - New tests to test the bugfixes
Hi @Schamper, I know you're upset with me for missing your birthday, but I hope this small present makes up for it. I know your favourite activity is to do code review, so I extended this PR with UDF support so there are more lines in this PR for you to play around with. To make it even more challenging, I only briefly looked at consistent spacing and that stuff, because I know how excited you get clicking that sweet 'add suggestion' button. I think this more than makes up for all those "How could you miss my birthday", "It was really important to me" and "Why are you so obsessed with an optical medium of the 90s" comments/jokes you were making. You're welcome! |
This PR adds support for filesystems typically encountered on optical discs. It currently supports ISO 9660 and its extensions 'Rock Ridge' and 'Joliet'. It currently does not support UDF and Apple extensions to ISO9660.
For example, the following usage:
Would yield: