codecheck: reformat with black, version 23.1.0 Drop empty newline as expected by black >=23.1.0 See https://github.com/psf/black/blob/main/CHANGES.md Closes: #1031466
Fix --grub and --syslinux handling With commit 3664e4ae we removed an argument from the grub_option() and syslinux_warning() callback functions, which was reported as false positives by vulture tool. Fixes regression from 3664e4ae, failing grml2usb execution with --grub (and deprecated --syslinux) command line option with: | TypeError: grub_option() takes 3 positional arguments but 4 were given Thanks: Ralf Moll for the bugreport
Fix race condition with blockdev/BLKRRPART due to lack of fsync Quoting dann frazier from Debian's #975015: | grml2usb autopkgtest frequently fail in Ubuntu's CI. | [...] | 2020-11-17 14:14:49,433 Probing device via 'blockdev --rereadpt /dev/loop0' | blockdev: ioctl error on BLKRRPART: Device or resource busy | 2020-11-17 14:14:49,452 Execution failed: ("Couldn't execute blockdev on '%s' (install util-linux?)", '/dev/loop0') | | I am able to reproduce this on an OpenStack instance with a failure rate of | 33% (36 failures, 110 passes). My theory is that the sync is not sufficient, | and that we really need to do a targeted fsync of the file. With the | attached patch, I've yet to see a failure in 42 iterations. Thanks: dann frazier <dannf@dannf.org> for bug report and patch Closes: #975015
codecheck: reformat with black, version 20.8b1 The formatting of grml2usb that was done with black, version 18.9b0 triggers changes with black 20.8b1-3 as present in current Debian/unstable: | black --check grml2usb | would reformat grml2usb | Oh no! 💥 💔 💥 | 1 file would be reformatted. | make[2]: *** [Makefile:44: codecheck] Error 1 Applying the change via modern black though doesn't lead to any changes with black, version 18.9b0, so let's use this. Gbp-Dch: Ignore
Use "GRML" as FAT label when creating the file system Closes: grml/grml2usb#32
Add unit testing capabilities and basic tests for check_for_usbdevice * Add a test directory and create file 'test/grml2usb_test.py' * Provide basic test for extract_device_name() * Extract regex match into separate function for better readability and testing options * Provide pytest.ini configuration to be able to use custom markers * Add symlink in test directory pointing to grml2usb for usage with importlib
Code cleanups * Drop unused (*arg) usage from generate_main_syslinux_config() * Drop global variable DATESTAMP, leftover from commit 100193b24, also remove corresponding `import datetime` * Drop global variable GPT_HEADER, leftover from commit 705a96b84 * Drop unused `import time` * Drop unused value variable from syslinux_warning() + grub_option() * Drop unused function update_grml_versions(), leftover from commit 35feaad92af2e * Drop unused array2string + string2array functions (introduced in commit 68df4a8c90), also remove corresponding `import struct` * Drop unused is_writeable() function * Drop unused generate_isolinux_splash() function * Mark unused variables in search_file() as such (Vulture will ignore these variables if they start with an underscore) * Mark unused function and excinfo parameters in del_failed() as unused The path and exception information is reported as unused by vulture, though shutil.rmtree needs to invoked with three parameters, quoting from https://docs.python.org/3/library/shutil.html: | If onerror is provided, it must be a callable that accepts three parameters: function, path, and excinfo. * The 'warn' function is deprecated, using 'warning' instead Thanks to vulture tool (https://pypi.org/project/vulture/). Thanks: Chris Hofstaedtler for review + feedback
Do not fail if ISO doesn't have grub.cfg inside efi.img If an ISO doesn't support Secure Boot then the grub.cfg file doesn't exist inside the efi.img, so we shouldn't continue with its execution. Use search_file() instead which is unset if the file doesn't exist and therefore execution gets skipped. Follow up fix for commit 92ffc08bb28f73c
Support Grml's new Secure Boot approach Secure Boot support was kind of broken and in grml-live commit 518eb395d we reworked the layout and handling of the configuration. The main change is the new GRUB prefix /boot/grub/grub.cfg instead of /EFI/ubuntu. We need to adopt this accordingly, though it's probably not worth being backwards compatible (given that we never released official Grml ISOs with Secure Boot). NOTE: the configuration file /boot/grub/grub.cfg *inside* the efi.img doesn't get adjusted via handle_grub_config() yet, so if we should ever add custom boot entries directly into this grub configuration file (which is known as the grml-live template file templates/secureboot/grub.cfg), we'd have to adjust handle_grub_config() or invoke handle_grub_config() from inside handle_secure_boot(). Also we install the grub.cfg from inside EFI as /boot/grub/x86_64-efi/grub.cfg. Looking at GRUB's default configuration file (see `cat (memdisk)/grub.cfg`) shows that if /boot/grub/x86_64-efi/grub.cfg exists it's getting sourced before /boot/grub/grub.cfg. Since our *actual* GRUB configuration of the Grml ISO is residing as /boot/grub/grub.cfg, we can use /boot/grub/x86_64-efi/grub.cfg to control behavior in Secure Boot mode. Also ensure we take over file /conf/bootfile_*, which we rely on from with grml-live's templates/secureboot/grub.cfg. This work was funded by Grml-Forensic.
codecheck: fix flake8 issues with versions >=3.8.3 Fixes: | grml2usb:920:17: F523 '...'.format(...) has unused arguments at position(s): 0 | grml2usb:1954:57: E741 ambiguous variable name 'l' Thanks: Florian Apolloner for feedback
Skip boot flag check when installing to directory In commit 8b59cb0b5c0cfa the check_boot_flag was moved from install_grml() to main(), though inside install_grml() we had the special casing whether the target is a directory or not. To avoid having to add this check before any possible check_boot_flag() invocation (and possibly forget about it), add this check to the implementation of check_boot_flag(). Fixes: | % sudo grml2usb --tmpdir=/tmp/grml2iso.tmp grml64-small_2020.06-rc1.iso grml32-small_2020.06-rc1.iso /tmp/grml2iso.tmp/cddir | Executing grml2usb version 0.18.1 | Checking for boot flag | Fatal: /tmp/grml2iso.tmp/cddir: unrecognised disk label
Merge remote-tracking branch 'origin/pr/35'
Check for boot flag before possibly creating FAT16 on the partition Checks should be performed before actually modifying the USB device, if the user specified `--fat16` then the boot flag should be checked *before* the FAT filesystem gets created. check_for_fat(device) is called in handle_vfat(device) (if device is present and not a directory and if option 'syslinux' (default) is set) so it should be safe to remove check_for_fat(device) call in install_grml(). The same applies to check_boot_flag() which should happen before actually creating the FAT filesystem (in handle_vfat()). Closes: grml/grml2usb#30
Reread partition table after installing MBR to ensure devices exist We need to execute blockdev after installing the MBR, otherwise the devices might not exist yet as expected and executing syslinux can fail therefore. While at it remove duplicate mbrtemplate python docstring in install_mbr Closes: grml/grml2usb#33 Thanks: Darshaka Pathirana for debugging
Use "boot flag" instead of "bootflag" also in docs + exceptions Closes: grml/grml2usb#31
Drop old bootflag detection + be more verbose about its execution Our custom boot flag detection isn't reliable, so there's no point in using it, instead let's rely on python3-parted only. While at it, rename "bootflag" into "boot flag" in human readable strings. While at it, also be more verbose about the execution (esp. the "sync" step takes a while and might look like something is hanging). Thanks: Darshaka Pathirana for bug report and debugging Closes: grml/grml2usb#19
Coding style: execute black + isort and fix undefined 'path' Avoid discussions around pep8, pylint etc, but instead use what black(1) (see https://github.com/psf/black) provides. Fix: | grml2usb:299:39: F821 undefined name 'path'
Coding style: fix pep8 + flake8 issues Fix: | W605 invalid escape sequence '\s' by using re.compile(r'...') instead of re.compile("...") Fix: | E261 at least two spaces before inline comment introduced in commit ed5b633be961ef6b3 Fix flake8 issues: | grml2usb:409:5: F841 local variable 'e' is assigned to but never used | grml2usb:557:34: W504 line break after binary operator | grml2usb:559:34: W504 line break after binary operator | grml2usb:737:37: W504 line break after binary operator | grml2usb:1615:13: F841 local variable 'error' is assigned to but never used | grml2usb:1641:9: F841 local variable 'error' is assigned to but never used | grml2usb:1665:5: F841 local variable 'error' is assigned to but never used | grml2usb:1826:30: W504 line break after binary operator | grml2usb:1832:30: W504 line break after binary operator The https://www.flake8rules.com/rules/W503.html vs https://www.flake8rules.com/rules/W504.html is strange, though let's use what black(1) (see https://github.com/psf/black) uses.
Fix duplicate Syslinux entries (2) Commit f0e3c936 changed open mode to "r+", but this mode does not create missing files. Revert open mode back to "a+" and add seek to beginning of the file.
Fix duplicate Syslinux entries Documentation of open()[1] says: Other common values ... and 'a' for appending (which on some Unix systems, means that all writes append to the end of the file regardless of the current seek position). On my system (Linux 5.4), writes to a file opened as 'a+' go at the end. [1] https://docs.python.org/3/library/functions.html#open