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

Add an option to provide path to uefi-ntfs.img #17

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tukusejssirs
Copy link

@tukusejssirs
Copy link
Author

I’ve just created this PR. I tried to test it (manually) if it works correctly (it should), however, @Lin-Buo-Ren, plese review and test it thoroughly.

Just a question: should we also remove L1079?

Copy link
Member

@brlin-tw brlin-tw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: should we also remove L1079?

Keep it as WoeUSB still requires internet access after the merge in some cases.

sbin/woeusb Outdated Show resolved Hide resolved
sbin/woeusb Outdated Show resolved Hide resolved
sbin/woeusb Show resolved Hide resolved
sbin/woeusb Outdated Show resolved Hide resolved
@tukusejssirs
Copy link
Author

tukusejssirs commented Nov 19, 2020

Changes to the code:

  1. I added $uefi_ntfs_img_download variable that should contain Boolean string. If set to false, it will use the supplied image (if it is a file), else it will download the UEFI NTFS image. By default, it is set to true on line 65.

  2. Also, I used the name refs where necessary, as you required.

  3. I move the $uefi_ntfs_img test to the check_runtime_dependencies() function, as you required. Note that I have simplified the condition (a bit) using grep (line 702), however, I could not find a better solution to check the -u/--uefi-ntfs-img then using a for cycle, because $uefi_ntfs_img might contain spaces. I don’t like the for cycle here, because it increases the overhead and depends on seq. If you know of a better way, let me know please.

@tukusejssirs
Copy link
Author

@Lin-Buo-Ren, is there anything that needs to be resolved (besides rebasing) before merging this PR? 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an option to provide path to uefi-ntfs.img
2 participants