-
Notifications
You must be signed in to change notification settings - Fork 147
Remove superfluous directory creation #67
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
Conversation
|
As @jrspruitt commented:
What about keeping the original logic if not specified? but use the specified one if specified. Maybe even a new option |
|
I failed to notice that this did not change ubireader_extract_files. I totally agree the directory is not needed. My only issue is that if you work on more than one UBI image, perhaps to compare contents, the UBIFS file names can be ambiguous. So what about something like this, prefixing the image file name, with the UBI filename? ubifs-root/fruits.ubi_img-1180426539_vol-apple.ubifs |
|
I'll talk with the unblob dev team, I think we can live without this change applied to ubi-reader. We'll just need to adapt our integration tests for UBI. Will get back to you. |
ubireader_extract_images creates an output directory named after the input file basename _within_ the output path. We introduce the --no-basename-subdir option for users who don't want this extra directory to be created.
|
@jrspruitt I modified the initial pull request to only touch ubireader_extract_images. The default behavior stays the same, but if users don't want the extra directory they can use the We can change the option name / add a short name if required. What do you think ? |
|
Hi @qkaiser, You said in your other post you didn't need this. I'm actually in the middle of a big UI update, that would essentially just wipe out this code when it's finally done as the code behind it is completely changing #63 . I can add this at that point, if it's not crucial now. I am more inclined to just skip the directory creation all together, for extract_images and utils_info, and just make the file name include the UBI image name. Is there an issue with just putting it in the file name instead of creating a directory? -Jason |
We cannot use git hosted dependencies if we want to publish unblob to PyPi. We therefore moved from our fork of ubi-reader to the official package of ubi-reader. Our fork was synced with upstream except for one UI change we did to get rid of superfluous directories created by ubireader_extract_images. Since the maintainer of ubi-reader does not want to include it upstream, we moved that logic into the UBIExtractor class. See onekey-sec/ubi_reader#67 for more details.
|
That's ok, we've moved our custom logic to the UBI handler (onekey-sec/unblob#504). Closing this PR. |
We cannot use git hosted dependencies if we want to publish unblob to PyPi. We therefore moved from our fork of ubi-reader to the official package of ubi-reader. Our fork was synced with upstream except for one UI change we did to get rid of superfluous directories created by ubireader_extract_images. Since the maintainer of ubi-reader does not want to include it upstream, we moved that logic into the UBIExtractor class. See onekey-sec/ubi_reader#67 for more details.
ubi-reader scripts creates an output directory named after the input file basename within the extraction directory.
We don't think these directories are required and writing directly to the extraction directory saves some space.
This is what it looks like before:
This is what it looks like with our fix:
We initially applied this change in our fork of ubi-reader to make the output of unblob cleaner (see onekey-sec/unblob#345). I understand this is kind of an opinionated API change but our team would love to see it land upstream so that we can publish unblob to PyPi (see onekey-sec/unblob#501).