Update the in-place modification APIs#172
Open
clalancette wants to merge 9 commits into
Open
Conversation
Directory records depend on seeing a zero entry to stop parsing. But in the case that we *shrunk* a directory record, we were failing to zero out old entries. That means we could have invalid or "ghost" directory entries. Fix this by zero-padding directory records, and add tests for this problem. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
When shrinking a directory record, it is possible that we use fewer extents now, and thus we need to shrink the grandparent. That can cascade all the way up the tree. Fix this by walking back up the whole tree, rewriting extents. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
modify_file_in_place was correctly updating the dot entry data length, but not the subdirectory dotdot entries. Fix that here by walking into all subdirectories. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Six PyCdlib methods carried a "(deprecated)" note in their docstrings but had no runtime signal: get_and_write, get_and_write_fp, add_joliet_directory, rm_joliet_directory, list_dir, and get_entry. Have each emit a DeprecationWarning pointing at its replacement so callers can find and migrate them. Also hide their warnings in the tests; once we remove these deprecated methods, we can remove these tests as well. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
These are a convenience that allow users to update the file contents in an existing file. It is pretty much syntactic sugar; you could achieve the same effect via rm_file/add_file, but this is more convenient. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Doing modify_file_in_place inside of the main PyCdlib class was a mistake; it is too easy to misuse it and mix it together with the other APIs which modify things in memory and only write out at the end. Instead, we add in a new InPlaceEditor contextmanager, and users have to use it from within that. While doing this we also end up deprecating modify_file_in_place with an eye towards eventually removing it. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After heavy debugging on #171 , I've come to the conclusion that the APIs around updating data are incorrect. In particular, it is too easy to misuse
modify_file_in_placewhen using the other APIs. Thus this PR does the following:modify_file_in_place. Again a bug fix to make sure dotdot length is always correct.DeprecationWarningfrom actually deprecated APIs.update_file_contents. This allows the user to update file contents, ready for mastering. This was technically possible before by calling arm_filefollowed by anadd_filewith the same filename and new contents, but this is more ergonomic.InPlaceEditor, which allows modifying things in place. This is the new API users should use if they want to modify in place. Also in here, I've deprecatedmodify_file_in_placefrom the mainPyCdlibclass.