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

Fix ppc64 chrp bootinfo generation. #2641

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Fix ppc64 chrp bootinfo generation. #2641

merged 1 commit into from
Sep 10, 2024

Conversation

hramrach
Copy link
Contributor

Fixes bsc#1210888

Changes proposed in this pull request:

  • Correctly escape the file content
  • In test compare against text file to avoid wrong escaping in both test and code to cancel each other

Copy link

@zumbi zumbi left a comment

Choose a reason for hiding this comment

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

This looks good time, just one remark, do we want to add ppc/os-chooser file in this commit as well (not sure if that's needed)?

@schaefi
Copy link
Collaborator

schaefi commented Sep 10, 2024

/home/runner/work/kiwi/kiwi/test/unit/bootloader/config/grub2_test.py:1753:14: E999 SyntaxError: closing parenthesis ']' does not match opening parenthesis '(' on line 1749
18
1     E999 SyntaxError: closing parenthesis ']' does not match opening parenthesis '(' on line 1749
19
1

@wfeldt
Copy link
Collaborator

wfeldt commented Sep 10, 2024

No, that's not needed. os-chooser was used on old Apple machines (iirc).

@schaefi
Copy link
Collaborator

schaefi commented Sep 10, 2024

@hramrach thanks for fixing this, this was my fault on the first PR, sorry

For this one I propose a fix on the test though.

  1. Please move the example test file grub2_test_chrp_boot.txt into the test/data path. We have all data files for the tests there
  2. I propose the following change in test/unit/bootloader/config/grub2_test.py
diff --git a/test/unit/bootloader/config/grub2_test.py b/test/unit/bootloader/config/grub2_test.py
index 25a4a6473..01008e672 100644
--- a/test/unit/bootloader/config/grub2_test.py
+++ b/test/unit/bootloader/config/grub2_test.py
@@ -1727,6 +1727,9 @@ class TestBootLoaderConfigGrub2:
 
         mock_exists.side_effect = side_effect
 
+        with open('../data/grub2_test_chrp_boot.txt') as chrp:
+            grub2_test_chrp_boot = chrp.read()
+
         with patch('builtins.open', create=True) as mock_open:
             mock_open.return_value = MagicMock(spec=io.IOBase)
             file_handle = mock_open.return_value.__enter__.return_value
@@ -1745,12 +1748,7 @@ class TestBootLoaderConfigGrub2:
                 call('search --file --set=root /boot/0xffffffff\n'),
                 call('set prefix=($root)/boot/grub2\n'),
                 call('source ($root)/boot/grub2/grub.cfg\n'),
-                call(
-                    '\n<chrp-boot>\n<description>Bob</description>\n'
-                    '<os-name>Bob</os-name>\n<boot-script>'
-                    'boot &device;:1,\boot\grub2\powerpc-ieee1275\grub.elf'
-                    '</boot-script>\n</chrp-boot>\n'
-                ),
+                call(grub2_test_chrp_boot),
                 call('source /boot/grub2/grub.cfg\n')
             ]

Thanks

Copy link
Collaborator

@schaefi schaefi left a comment

Choose a reason for hiding this comment

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

Looks good 👍

I'll merge as soon as the test runs are green

@schaefi schaefi merged commit 30c9eaf into OSInside:main Sep 10, 2024
12 checks passed
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.

4 participants